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

Reject invalid MPEG puts for certain lib versions #8803

Merged
merged 8 commits into from
Jun 5, 2016

Conversation

unknownbrackets
Copy link
Collaborator

@unknownbrackets unknownbrackets commented Jun 5, 2016

The good news: this reportedly fixes #3318, and fixes #6657.

This change is probably a bit risky, but I've tested a number of games and haven't run into any video problems. I still don't really know what "PMP videos" are and have not tested the specialized code dedicated to them.

Here are the major things this does:

  1. Prevents searching for a startcode beyond available data. This could've been causing corrupted audio frames in some cases.
  2. More strictly and more immediately demuxes MPEG frames. This may also help corrupted audio frames, but might also break somewhere it was accidentally working.
  3. Exposes the MPEG write position to the game in an actual ring. This is how it's really supposed to work anyway, and might make it easier to decode more directly from PSP RAM some day.
  4. Reduces the required mpeg memory for older libs, per tests. Might matter for some memory allocation.
  5. Advances packetsAvail incrementally (required for Stuck issue with Mega Man Maverick Hunter X  #3318), rather than resetting it after decode. I played this one safe, and only did it for the necessary lib versions.

However, one note: I have found two 1.03 mpeg.prx's that have different behavior. Technically, some of this behavior is correct for 1.03a but not for 1.03b. Hoping it doesn't matter, I don't want to go down the route of crcing them for version numbers...

-[Unknown]

It seems it started requiring 64k at 1.05.
This mimics behavior with clamped sizes and wrap around, and also makes it
easier to implement the garbage data handling the PSP has.
This also parses a bit earlier, not requiring a full 2048 bytes ahead at
all times.
Older libraries only, but this will cause it to reject packets that don't
make sense.  So far, this seems to mirror the behavior of various garbage
packets sent to the real firmware.
When we've got garbage data, this has to stay incorrect.  Without this,
Megaman X gets confused when playing its video (because it enqueues
garbage packets.)

See hrydgard#3318.
@zminhquanz
Copy link
Contributor

Are you plan to write UMD video in the future ?

@unknownbrackets
Copy link
Collaborator Author

Well, it'd probably be more worthwhile to ask FFmpeg to detect and support atrac3plus (which it can already decode) in standard video files. If they did that, most likely UMD videos would be playable in just any video player you wanted, as long as it used ffmpeg.

That sounds like a better outcome than PPSSPP having support for it, especially because PPSSPP is a pretty terrible video player. It can't handle subtitles or fonts in the stream, doesn't have fancy scrubbing controls, doesn't support embedded album art, doesn't add latency to video when audio is lagging (which would be a terrible thing for an emulator but is a great thing for a video player), etc.

Basically, there are a bunch of really popular video players out there that people have devoted hundreds or even thousands of hours to making great. And they are great. I feel no motivation to throw hundreds of my own hours into making a not-as-great video player.

Isn't it better for a great video player to support another video format, rather than a great emulator to try to "me too" being a video player?

-[Unknown]

@daniel229
Copy link
Collaborator

Also fixes #6657

@unknownbrackets
Copy link
Collaborator Author

Oh, cool. I guess that explains what was happening there. Does it help #8526?

-[Unknown]

@daniel229
Copy link
Collaborator

No.

@unknownbrackets
Copy link
Collaborator Author

@zminhquanz btw, here's the ticket for that: https://trac.ffmpeg.org/ticket/3233

-[Unknown]

@zminhquanz
Copy link
Contributor

oh , i'm very pleasure , good job

@zminhquanz
Copy link
Contributor

Does it add from #8702

@hrydgard
Copy link
Owner

hrydgard commented Jun 5, 2016

No, that's not related.

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.

Go! Sudoku - random lag Stuck issue with Mega Man Maverick Hunter X
4 participants