-
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
Improve mpeg parsing / corruption issues #8867
Conversation
Fixes #5266 ,have not found anything broken, |
Regression: Xyanide Resurrection stucks before the logo video by f7cbe54 |
Hmm, I'll have to figure out the error codes better then. I assume it's broken only by the most recent commit, right? Is #5266 still fixed without the most recent commit? That's the one that really helps/fixes Valkyrie Profile, but it might not be bad to get the other commits in for now. It's gonna take some time to figure out how exactly the firmware splits aus... -[Unknown] |
Make sure we don't read garbage.
In cases where we did not have a full 64k at first, we would potentially send FFmpeg garbage if it asked for it.
Without doing this, FFmpeg will try to probe the streams to detect them instead. When it does this, sometimes it tries to read beyond the data that's available - and then gets confused by EOFs. Parsing this way allows us to control the situation. An example is Valkyrie Profile, corruption in the first frames of the second video during the intro. Thi doesn't fix it yet, but now it's just a matter of buffering.
Helps the ending video in Valkyrie Profile. See hrydgard#6008.
Okay, rebased without that - I think reducing corruption is still a good thing and I'll need it for whatever the ultimate fix is no matter what, I think. That last commit was the most dangerous one anyway. @hrydgard I think this should be somewhat safe now. -[Unknown] |
@hrydgard how about this ? |
Ah, missed that it got rebased. Thanks! |
+ if (dstWidth > dstBuffer->width || dstHeight > dstBuffer->height) {
+ // The buffer isn't big enough, and we have a clear hint of size. Resize.
+ // This happens in Valkyrie Profile when uploading video at the ending.
+ ResizeFramebufFBO(dstBuffer, dstWidth, dstHeight, false, true);
+ } It looks like this is causing performance problems on God of War. -[Unknown] |
This improves #6008.I've tested this with some other games, but it could probably use more testing. The things this fixes were all found in Valkyrie Profile:Near the intro, there is a video where the main character transforms. Previously, the first frame was gray, causing 1-2 seconds of corrupted delta frames.In the ending, the video would play at the wrong size for a while (cropped with edge repeat to the screen), before eventually snapping to place.In the ending, after the video played, the game would crash. This was because the first frame was decoded to early compared to correct behavior.Errors were emitted when some videos were played, because FFmpeg was partially reading garbage.Also worth noting: this game uses short video sequences for some animations, such as small doors opening. I haven't entirely figured out the story on these - they don't seem to contain AU split markers in the data they expect to be decoded. Ultimately, I think the sceMpeg_Au_ funcs are supposed to fail on partial data in more complicated ways (see #6008 for more detail.)f7cbe54 fixed those things, but needed the prev commits, and has been removed since it broke other things. However, areas of corruption are still improved by this and it fixes #5266. Will have to continue to work on the fix for Valkyrie Profile.
There is some danger of breaking videos that work, but I've tested several games so far and things are working...
-[Unknown]