Repository navigation
Correct size and YUV order for jpeg decoding - #16179
Merged
Merged
Conversation
Seems to be reused even after Delete, strangely.
This also fixes a crash on invalid output pointer.
Previously, it was assumed that the stride was the nearest power of two, but it's actually the Create width.
For non-mjpeg, the height and width are halved, and more sampling modes are supported. This also checks for invalid pointers and notifies the debugger. The exact YCbCr->RGBA conversion is not accurate, but it writes in the correct places now. MJpeg is still a bit off.
This was referenced Oct 8, 2022
Owner
|
Very cool! I'll look through it tomorrow. |
Verified this is what's output from a PSP's DecodeMJpegYCbCr, and games directly use its output in MJpegCsc - so change to match. This makes the colors in Gods Eater Burst character portraits look better.
Collaborator
Author
|
I went ahead and changed MJpegCsc to match what I'm roughly getting. I get some garbage in my tests, but visually it has to be this way. My tests do show it uses different coefficients, though, so I kept the loop separate. Slightly worried this may impact #8521, but I think it was just getting messed up colors before? For me, this improves the character image (on save data) from Gods Eater Burst. -[Unknown] |
hrydgard
approved these changes
Oct 9, 2022
ghost
mentioned this pull request
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9931, it turns out the stride is part of the
sceJpegCreateMJpeg()function call, which should keep #9760 working (does for another play view.) See tests in hrydgard/pspautotests#229 which validate this functionality.I think this may help #16157 as some logs indicated it uses sceJpeg (and sceJpeg was not logging read/write in the debugger.) It would make more sense than a download to VRAM using the wrong stride.
This also corrects the swizzle for sceJpegCsc, which I'm hoping will help #9457 as well.
Other changes:
Notably, the jpeg decoding and colorspace conversions are not bit accurate to the PSP, so the colors are still slightly off. Didn't really touch the RGBA/YCbCr conversion. Currently our JPEG decoder goes to RGB, and then we go back to YCbCr, which adds error.
There's some weird behavior with
sceJpegMJpegCsc(), which is no less broken than before. Changing Cb or Cr values doesn't affect the pixels I'd expect, I think it might be upside-down and swizzled or something.sceJpegDecodeMJpegYCbCr()is probably using a similarly wrong order. Since they match, it shouldn't cause real problems, but it's not currently accurate.-[Unknown]