-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix jpeg's with width not matching psp buffer size. #9760
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is probably fine, but I'd recommend writing some pspautotests
code to validate the behavior of these functions - shouldn't be too hard to throw different jpegs at them.
Also, what about the dht and other parameters for the MJpeg functions?
-[Unknown]
Core/HLE/sceJpeg.cpp
Outdated
@@ -139,12 +139,19 @@ static int __DecodeJpeg(u32 jpegAddr, int jpegSize, u32 imageAddr) { | |||
if (actual_components == 3) { | |||
u24_be *imageBuffer = (u24_be*)jpegBuf; | |||
u32 *abgr = (u32*)Memory::GetPointer(imageAddr); | |||
int pspWidth; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this isn't initialized, what if the width and height are crazy values, like 4096? Not that it should happen, but good to handle error scenarios.
-[Unknown]
DHT I guess stands for Define Huffman Table(edited: found what it really stands for;p ~ this site has lots of info about it ) and it should be handled by the jpeg decoder we're using, don't think we have to do anything with it unless we want to write a new decoder from 0. There are no other parameters in two affected syscalls? JpegAddr and size just tells where and how big jpeg is and image address is where we will send the decoded texture. The syscall also returns resolution of the jpeg, so I think everything is handled here. Other jpeg functions there probably need more work like the mentioned __JpegConvertRGBToYCbCr might need similar change, not sure about that, but it could work by luck like my initial implementation which was based on game with nice sized jpegs matching psp textures. |
Well, maybe the parameter is wrong. Not all of these guessed function names are correct. Maybe the "dht" parameter actually controls whether textures must be square or something. -[Unknown] |
Hard to tell if it has anything to do with square, edge of the zoomed-in book page in that playview app was the only place I saw a problematic tall rectangle which emulated app really expected to see in a square texture, that parameter varies from app to app, but seems to be same for everything within the app doesn't matter the shape of the jpeg's. I checked dumped jpeg's using "JPEGsnoop" that I found on the site I linked earlier and DHT was same in jpeg's which set this parameter to 1 and those that set it to 0, the only thing which clearly was different was "DQT"(Define Quantization Tables) which is basically used for lossy compression, so I guess it could be that and maybe it was used to skip some work when decoding images on the psp. |
I got one of those playView apps and turns out jpeg needed a bit more work when jpeg width does not match psp buffer sizes:].
Currently:
With fix:
I think such change might also be required in __JpegConvertRGBToYCbCr, but I don't know what games use sceJpegDecodeMJpegYCbCr to test. Should I just blindly add it?:P It makes sense, but I don't know anything bugged by it.
This should hopefully fix #1504, but if it plays movies like the one I got, those still have some unimplemented stuff.
Edit: Heh noticed one problem still even with this ~
Don't understand why it happens, doesn't seem to be decode problem, that texture just get's wrong buffer width?:| ~ this should be 128x256 looking like that:
Edit: pspWidth definitely get's 128 for this, checked with hash just to be sure I get info from correct jpeg, don't understand why it's happening:|, is there any code outside of sceJpeg that could be causing this?
I think the only way for this to make sense if texture made from jpeg would have to be square as that's how all other textures end up, I'll see how that works.:]
Last edit: :P Yeah it needs to be square, fixed it: