-
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
Video scenes are choppy in newer FFmpeg versions #11490
Comments
You'll need to test more of the commits between among those 8000 to narrow it down - it won't really be possible to figure out what happened. That said, a few things:
-[Unknown] |
Good feedback! Thanks! Some answers:
Obviously testing 8k commits is unrealistic lol but I realize it's a large gap for sure. I'll try and find the time to test each release and see where it first started. Curious if anyone else sees this same issue?
I can confirm that gameplay reaches 100 percent and sometimes higher (as long as frameskip is 2). It averages at 99.5%. I can confirm the issue exists at both 720p and 1080p resolutions.
I'm testing it on Linux with rockchip kernel 4.4 with multiple boards.
Auto frame skip is off and I've tested with a frame skip of 10 without resolve. Audio is perfect.
It's PPSSPPSDL and it's using system ffmpeg. Same exact settings as I'm using on the libretro version compiled.
|
It's not The Warriors is it? I think that game specifically is not currently working right: #8991 I know that videos in general work great on Android and Windows in most games. A good game to validate against is Crisis Core, since it's popular. Have not tested Mac or Linux as recently, but I know they were working since v1.6.3 (maybe around 300 commits after) at least for sure. No recent changes to video handling, really. Even trying a build in between would cut that down to 4000. I think if we were looking at a range between stable releases, we could make some vague guesses. That said, of course 8000 commits can be bisected in probably 13 tries or so for someone experiencing the issue. It might also be worth trying FFmpeg 3.0.2 (this is the version we test against, and link on Android/Windows/etc. official binaries - no patches, but significantly reduced configure options to make it lighter.) Of course, FFmpeg is not careful about backwards compatible API breaks, but an issue was recentlyish (iirc after v1.5.4?) fixed that was causing horrible corruption during video playback in new system FFmpeg versions. If such an old PPSSPP is working fine on a newish system FFmpeg, it makes me suspicious that it might actually be using a static FFmpeg (build script error) or be using some unknown patch. -[Unknown] |
I'll test some of those games when I'm able and report back, thanks! I'll also test some different release versions and see if I can determine when it started. I know that video in 1.5.4 was definitely all garbled, so I'll start with v1,6. I had to patch sceMpeg.cpp in order to make the older ppsspp versions work with the newer ffmpeg versions. Can you advise what other source files in this project are involved in ffmpeg playback besides that one? I can test reverting some of them to see if I can identify the problematic file, which should narrow the commit scope down dramatically as well. |
Some things are interrelated, but Core/HW/MediaEngine.cpp is probably involved as well. Depending on what game is playing the video, it may also use Core/HLE/scePsmf.cpp not sceMpeg. -[Unknown] |
That was stellar info, thanks! For some reason I haven't been able to compile versions 1.6 or 1.6.3 without EGL, no matter what flags or cmakelists changes I tried. Oh well, I was able to fix it against the master commit from the other detail you provided. After testing, I can confirm that reverting the changes made to MediaEngine.cpp fixes the stuttering issue. It's not a complete revert, in order to successfully compile, but mostly. So the fix for this issue is somewhere in the attached patch. I haven't tested individual segments to narrow it down to root cause, but maybe you'll be able to have an idea when you look at it. If you want to send over a modified patch for me to try in my environment that doesn't revert everything included then feel free. |
Which game is this? -[Unknown] |
I have a hand full I'm testing. Not in front of the computer right now but I know both need for speed and tekken are in there. All the ones I've tested had playback issues and the patch has resolved. |
I know NFS is 30fps and tekken is 60fps, just FYI, that's why I know they are in my test group. But no settings were changed, just this patch applied. |
FYI - Crisis Core is in my test list, per your recommendation. Runs perfectly now. Stuttered before. |
You'll want to identify what change you're reverting. A few things to make sure you haven't lost track:
At this point, you seem to just want to "undo" all changes that have been made since v1.0.1 (4 years of changes) to anything related to videos, without regard to what it might break or why. I'm sure it's a worthy goal to make a newer version of FFmpeg work, but it's probably some API breakage (like us relying on a pts field that isn't being consistently filled anymore, or something.) For example, your proposed change breaks support for mono audio. It will also reintroduce #9262 and make video playback slower on mobile devices. I don't think this is pull request ready, no. -[Unknown] |
Agreed, definitely not ready for a PR on this, and you're correct that at this time my concentration is to resolve the issue for my specific platform without regard to how it affect others (such as mobiles). My "fix" was simply based off of knowing it used to work and using that working version to compare what had changed. Yes I am definitely using an FFMPEG version later than 3.0.2. As time permits I will continue testing and see if I'm able to narrow the patch down to the specific segment(s) necessary to resolve the issue so that it's not just a blanket revert, as it is in it's current state. Thanks for all your help on this, I'll let you know if I have time to work more on this. For now the issue can be closed :) |
Okay, well, it's still an issue so we're probably better off leaving it open. -[Unknown] |
Im my issue oppened mounths ago, i have slow performances on batocera Linux about ppsspp in the 1.6.3 version ...otherwise, if i use the old 1.5.4 version the games run well (if considerating i have an raspberry pi 3 with weak hardware and only 24 GFLOPS !! ) .... I will test today an new build with the 1.7 ppsspp version hoping this can be good .... but i dont know why the new 1.6.3 is bad performance only for RPi3 |
I've been happy with the 1.7 performance thus far. Seems on par with 1.5.4. Just had to fix those video scenes. |
I'm having the same issue on Crisis Core with:
The intro videos plays in a choppy manner, while audio is good. It's a "new" issue but previous versions (pre 1.7) used to crash due to #9026, which has been fixed (thanks!). So it's a net improvement for those of us shipping with system FFmpeg at current upstream versions, but there's still some room for improvement on compatibility with recent FFmpeg. I'm packaging PPSSPP for a Linux distro and as per policy, using ppsspp-ffmpeg or downgrading system FFmpeg is not an option. |
Why is using ppsspp-ffmpeg not an option? Just consider it part of PPSSPP... |
FFmpeg needs frequent updates to fix security vulnerabilities, and having duplicate instances of it at different versions means a lot of added work for our security team. See http://advisories.mageia.org/src_ffmpeg.html I fully understand why you're bundling ffmpeg with the wide array of platforms you need to support, but I'd advocate for trying to ensure that PPSSPP works with newer ffmpeg, if only to allow you to eventually update ppsspp-ffmpeg smoothly to a more recent and supported version. It's definitely some additional work but not unmanageable either, as #11176 shows that fixed a 2-year-old crasher with ffmpeg > 3.0. |
PSP games contain only a very restricted set of formats and security isn't much of an issue due to the nature of the thing (I guess possibly bad actors could try to distribute PSP ISOs with videos trying to exploit the parser in ffmpeg but it seems pretty far-fetched). Ideally FFMPEG would care more about keeping source compatibility between versions.... Us trying to follow it is not very workable given the historical precedent. Anyway, a one time update to 3.0.12 doesn't seem entirely unreasonable but it's still a big task to get it rebuilt and tested for all the platforms and I don't expect to find time for it in the near future. |
I fully agree, but from the perspective of a Linux distro we can't do a full audit of all packaged software to see if and how the security vulnerabilities of its dependencies could be exploited, so we patch everything in a preventive way. Since most applications are packaged to rely on system-wide shared libraries, we only need to rebuild those libraries (as long as the security fixes don't break ABI) and all its users are automatically (more) "secure". If some applications build those dependencies statically, we need to rebuild them too after patching their static bundled source (which might be hard if it's not the version for which upstream provides patches). Beyond the security arguments (which is the main reason), the fact that ffmpeg takes 3 times longer to build than ppsspp is also a strong incentive for being able to link against an already built system ffmpeg. |
Exactly the same spot that I was in. Did you try my patch I included? It only reverts a handful of commits for only one file. Definitely reverts some that are not necessary,, but resolves the issue for me. Just to confirm you have the exact same root cause.
|
I confirm that @mrfixit2001's patch fixes the choppiness for me with PPSSPPSDL 1.7.2 + system FFmpeg 4.0.2, playing the intro of Crisis Core. As mentioned it's not a proper fix as it reverts various features and bugfixes done in the meantime, but it shows that the choppy playback is actually caused by some of this code. |
@mrfixit2001 I won't have time to work on it in the near future, but in case you want to dig further and identify what commit caused the regression, you could try to attempt reverting those locally one by one:
7a7c655 2018-06-13 Jan Beich MediaEngine: adjust for AVStream.codec deprecation That's all changes to |
Actually I went ahead and tried reverting those commits myself. I found that 558b462 seems to be the one causing the regression. What I tested:
|
That was quick! :) great job in narrowing that down! Hope the ppsspp team can evaluate what needs to happen next in order to fix the master branch. Thanks for your efforts, I definitely would not have had time for awhile.
|
Here's a "clean" revert of 558b462 (leaving 7a7e4ed and 209500a alone): |
I'd suggest adjusting the patch so only mediaengine is being touched. I expect the other files should not be necessary to revert.
|
As an extra data point, I've now been compiling PPSSPP against system FFmpeg for several years (now against FFmpeg 4.1.5 and 4.2.2 on two distro releases), and while it used to be quite painful and bug-prone in the past, since a couple of years I haven't experienced any issue beyond the need to work around this issue (#11490). This simple So while I can't presume of how difficult it would be to make PPSSPP properly support recent FFmpeg, I think that it would be easier than it used to be, and upgrading |
FFMPEG has some define to check versions, right? Send the above patch surrounded with a proper #ifdef , checking for some appropriate intermediate version number, and a comment explaining what's going on as a pull request, and I'll accept it. It must still build with the PPSSPP ffmpeg version. |
Compiling with a recent ffmpeg version also reveals several deprecated warnings that would need to be addressed, but I personally would love to see this work done now before a compiler update renders the old ppsspp-ffmpeg too broken to build. |
@hrydgard Another concern is that ppsspp-ffmpeg is not currently compatible with anyone using an alternative libc implementation (musl, uclibc), for such environments a system version of ffmpeg would be much easier. |
That patch will cause bugs in some games. It doesn't cause bugs in all games, so you probably haven't noticed them. As discussed in the other issue, it's a hack, and disabling important code. -[Unknown] |
Right, I guess we could still accept it but display a warning, or something. Though I don't know what the text should say to make that a thing users would understand how to deal with. Though again, this doesn't affect the major platforms anyway. I don't know... |
I was not advocating for getting my hack merged upstream, just mentioning that it's everything needed to make things "work" with latest FFmpeg. As @unknownbrackets mentioned, it disabled important code and it likely introduces issues in some games, so it's not a good solution. |
Well, there's some deep investigation and debugging needed then, and I don't see this as a priority since by using our fork, it's guaranteed to work. And it's still building, isn't it? What's the worst deprecation warning that you get? |
I see a lot of warnings like this.
Full build log: ppsspp.log |
This is probably left by ffmpeg developers to hint deprecated parts of code/api (probably they modernizing some code paths and don't want to use those calls anymore). I think that C++17 standardized [[deprecated]] attribute that should works across compilers (but still this is attribute to tag methods/functions that should go away in future). |
It means that sometime in the future ffmpeg plans to remove those and any program which still has them at that point will not be able to build with the upstream ffmpeg. |
I misread your previous comment(s) (I was assuming those errors are generated by internal one, so I was thinking that deprecated warnings are about internal ffmpeg implementation). Sorry about that. Yeah this will end-up as you described. |
@hrydgard Please reopen this issue, as already explained its not fixed yet. |
Initially it seemed like this was just a codecpar issue - updating SetupStreams to use those fields fixed it for some videos. Tales of Eternia is an example not fixed. It seems like the frame rate is wrong, because it returns the same frame multiple times (repeated pts.) Setting the frame rate explicitly doesn't help. The problem is that FFmpeg fails to properly detect streams for two reasons:
Maybe we just cannot use FFmpeg for video handling at all. It seems like they are changing the API to be more incompatible with PSP games. -[Unknown] |
I suppose the way out is to do our own stream parser, trying to mimic the PSP, and just feed the extracted h.264 and at3+ data directly to the codec kernels manually, and not use the higher level APIs of ffmpeg at all... |
#14188 should have worked around this, please test your games. |
Only downside is it will only work for 29.97 FPS videos. I don't think I've ever seen a video at a different FPS in a PSP game, but that doesn't mean they don't exist. -[Unknown] |
Tales of Eternia seems fine now. However with Tales of Rebirth and the system ffmpeg it freezes at the end of the FMV and spams the console with this line requiring ppsspp to be manually killed.
It does not seem to happen unless the video plays out all the way to the end without being skipped by the player.
Would it be possible to create homebrew that intentionally breaks this assumption? If yes I suggest a more robust solution would be great. :) |
The games |
These FMV videos all seem to have micro stutters, but I'm not sure if its only with the newer system ffmpeg? |
PPSSPP is an open source project - please feel free to contribute. Writing sceMpeg tests is fairly time consuming, and generating valid sample data is annoying because ffmpeg doesn't actually output the files the same way (especially a concern for audio sync with video.) I wouldn't classify dealing with FFmpeg API changes made - for good reasons, to make common video playing easier - without consideration for the needs of emulators as "exciting", so I'll warn that my interest in making it so people stop shooting themselves in the foot as badly by using system FFmpeg is waning. I probably don't want to devote several days to writing tests - after all, I contribute to PPSSPP, essentially, for fun. Looking forward to someone else contributing a robust solution. More than happy to review. -[Unknown] |
I wasn't trying to imply that you do or do not make such tests. I was just wondering if its possible? Would a real psp potentially support such a demo? There is a lot of PSP homebrew already so maybe such a thing already exists? |
The video scenes play and are not garbled, but they are choppy/laggy. I've tried virtually all available video settings and none have any affect that I've found. But... when I run libretro-ppsspp from retropie the videos play perfectly. The libretro gameplay doesn't perform as well as this core emulator does, so I'd prefer to use this one.
The retropie version is based from an older commit of ppsspp and sourced in this repo: https://github.com/RetroPie/ppsspp/tree/libretro
I wrote a patch (attached) which specifically reverts /Core/HLE/sceMpeg.cpp back as close to the commit from the retropie repo as I can get it. It compiles but it doesn't resolve the problem. So some time between then and now something with the video scenes player broke, as it's clearly not the environment.
ffmpeg-revert.zip
The text was updated successfully, but these errors were encountered: