Skip to content
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

Merged
merged 5 commits into from
Aug 6, 2016

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Jul 24, 2016

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]

@daniel229
Copy link
Collaborator

Fixes #5266 ,have not found anything broken,

@daniel229
Copy link
Collaborator

daniel229 commented Jul 24, 2016

Regression: Xyanide Resurrection stucks before the logo video by f7cbe54

@unknownbrackets
Copy link
Collaborator Author

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]

@daniel229
Copy link
Collaborator

Yes,still helps #5266 without f7cbe54

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.
@unknownbrackets
Copy link
Collaborator Author

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]

@unknownbrackets unknownbrackets changed the title Improve mpeg parsing / corruption / delay issues Improve mpeg parsing / corruption issues Jul 24, 2016
@sum2012
Copy link
Collaborator

sum2012 commented Aug 6, 2016

@hrydgard how about this ?

@hrydgard
Copy link
Owner

hrydgard commented Aug 6, 2016

Ah, missed that it got rebased. Thanks!

@unknownbrackets
Copy link
Collaborator Author

+				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]

hrydgard added a commit that referenced this pull request Sep 11, 2024
This reverts to the old behavior before we started parsing mpeg headers,
that is, in 558b462 which is part of #8867.

LunaMoo has this under "HackFixVideo" in his build.

See #8991. This doesn't really "fix" that, but works around it.
hrydgard added a commit that referenced this pull request Sep 11, 2024
This reverts to the old behavior before we started parsing mpeg headers,
that is, in 558b462 which is part of #8867.

LunaMoo has this under "HackFixVideo" in his build.

See #8991. This doesn't really "fix" that, but works around it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression:Bakemonogatari corrupted frames in video.
4 participants