mpv-android / mpv-android

#mpv-android @ libera.chat
MIT License
1.84k stars 228 forks source link

Dropped frames with display-resample and hwdec #307

Closed brucethemoose closed 7 months ago

brucethemoose commented 3 years ago

Playing a 1080p video results in lots of dropped and mistimed frames when display-resample and hardware decoding is enabled. But playback is smooth when I switch to software decoding, or when I set video-sync to "audio".

Hardware: Razer Phone 2, SD845, display set to 120hz and 1080p, Android 9

Video: 1080p24 8 bit AVC, 6 channel AAC audio.

MPV settings: All default, except for AV sync and debug info.

Heres some logs with display-resample enabled. I start with hardware decoding, then switch to software decoding about half way through: 2020-09-08_16.44-hwdecbug.zip

This happens with the last couple of releases. However, going way back to the 2018.04.29 build results in flawless playback with hwdec and display-resample enabled, as mentioned here: https://github.com/mpv-android/mpv-android/issues/272

I created a new issue because this is specific to display-resample. And I know some hardware is too slow for display-resample, but I feel like an SD845 should be able to handle this, and there appears to be some kind of regression after the 2018 build.

Let me know if I should upload/test anything else.

sfan5 commented 3 years ago

On my device display-resample performs equally poorly whether I have hwdec enabled or not: yes-hwdec no-hwdec

Haven't tested the 2018.04.29 build yet or reverting to the relevant mpv version (which would be mpv-player/mpv@6bd2bdc). If the problem turns out to be reproducible that way there are 422 commits to be bisected between the 2019-04-18 and 2018-04-29 builds.

Edit: I forgot to say, mpv drops way fewer frames when not using screen recording. The amount of mistimes/delayed frames in the GIFs above is accurate however

brucethemoose commented 3 years ago

Ah so I was wrong about the build/timeframe!

1-30 performs perfectly with HW decoding and display-resample: HW_OLD

5-27 drops some frames and looks WAY more stuttery than those stats suggest: HW_NEW

But performs and (mostly) looks fine with SW decoding: SW_NEW

As it turns out, stats.lua was tripping me up in earlier testing. All builds seem to drop frames with it enabled, while the native stats overlay appears to be much less problematic. This is from the 1-30 build Old_Lua_HW

But even with stats.lua, playback looks smoother than 5-27.

brucethemoose commented 3 years ago

BTW I found 5-27 by testing the builds one at a time, starting with the 2018 one. So builds before 1-30 appear to perform fine as well.

The latest build still has the issue: newest_HW

And it still happens with the stats overlay disabled.

sfan5 commented 3 years ago

As it turns out, stats.lua was tripping me up in earlier testing. All builds seem to drop frames with it enabled, while the native stats overlay appears to be much less problematic.

Can reproduce that here.

5-27 drops some frames and looks WAY more stuttery than those stats suggest:

2020-05-27 has a known regression in hwdec performance, you should be testing 2020-06-02 instead.


Is this issue about dropped frames or mistimed/delayed frames (which are only shown in stats.lua)? And when the bug is present, the player constantly drops frames similar to the first GIFs I posted?

And these are your testing results, is that correct? Version Result
2020-09-04 not okay
2020-06-02 not okay
2020-01-30 okay
brucethemoose commented 3 years ago

OK so there was another small misunderstandings on my part. I mistakenly equated the mistimed frames in stats.lua with dropped frames in the native overlay.

As you can see above, 1-30 is not dropping frames on my device with stats.lua enabled.

Anyway:

And these are your testing results, is that correct? Version Result 2020-09-04 not okay 2020-06-02 not okay 2020-01-30 okay

Correct. Additionally, all builds between 2020-01-30 and 2018-04-29 are OK.

Is this issue about dropped frames or mistimed/delayed frames (which are only shown in stats.lua)?

Both? This is from the 2020-09-04 build: hw__lua_newest

It clearly has more dropped frames, more delayed/mistimed frames and more VSync jitter than the 1-30 build in stats.lua.

Additionally, I'm using a test scene where any kind of hiccup in frame delivery is very visible. Even with the stats overlay disabled entirely, I can see constant stutters in the 2020-09-04 build, and no visible stutters in the 1-30 build.

brucethemoose commented 3 years ago

2020-05-27 has a known regression in hwdec performance, you should be testing 2020-06-02 instead.

Good to know. The issue is still present in 2020-06-02: 06-hw-lua_1


I tested the same scene at 60hz instead of 120hz. Here is 09-04: 09-lua-hw-60

And here is 01-30: 01-lua-hw-60_1

Even at 60hz, there are fewer dropped frames, mistimed frames, and less VSync jitter in the 01-30 build.


I tried another clip with a different format: 1080p30 HEVC

At 60hz, performance in 1-30 01-hw-60

Is better than 09-04: 09-hw-60

At 120hz, the difference between 01-30: 01-hw-120_1

And 09-04: 09-hw-120

Is much more dramatic.

sfan5 commented 3 years ago

I can confirm that the amount of dropped mistimed/delayed frames is much higher on newer builds, but only when using hwdec.

0c9ac5835be70ae26e4aa875e833fe2c7b3b3bf3 is the first bad commit
commit 0c9ac5835be70ae26e4aa875e833fe2c7b3b3bf3
Author: wm4 
Date:   Fri Apr 10 01:33:38 2020 +0200

    video: remove another redundant wakeup

    The wakeup at the end of VO frame rendering seems redundant, because
    after rendering almost no state changes. The player core can queue a new
    frame once frame rendering begins, and there's a separate wakeup for
    this. The only thing that actually changes is in->rendering. The only
    thing that seems to depend on it and can trigger a wakeup is the
    vo_still_displaying() function. Change it so that it needs an explicit
    call to a new API function, so we can avoid wakeups in the common case.

    The vo_still_displaying() code is mostly just moved around due to
    locking and for avoiding forward declarations.

    Also a somewhat risky change (tasty new bugs).

 player/video.c |  4 +++-
 video/out/vo.c | 61 ++++++++++++++++++++++++++++++++++++++++------------------
 video/out/vo.h |  1 +
 3 files changed, 46 insertions(+), 20 deletions(-)
brucethemoose commented 3 years ago

Ahh, so this is a MPV issue, not necessarily an mpv-android specific one?

Should I post an issue there?

brucethemoose commented 3 years ago

As a workaround, adding this to mpv.conf fixes the stutter in the 24p video with display-resample:

vd-queue-enable=yes ad-queue-enable=yes vd-queue-max-bytes=2000MiB

It may not fix the performance regression, but it seems to give the decoder and vo more breathing room.

sfan5 commented 3 years ago

I talked to wm4 about it but it wasn't clear if there was an obvious way to fix it.

Reverting the affected commit (+2 more) produces this build which should have identical performance to before: https://kitsunemimi.pw/tmp/mpv-android-2020-09-25.apk

brucethemoose commented 3 years ago

That build performs well. No dropped frames:

Screenshot_20200926-175922small

Thanks!

brucethemoose commented 3 years ago

Is this fix going to be in builds going forward, or is it something that needs to be fixed in MPV?

Also, which 2 other commits did you omit? I think I'm going to just build MPV-Android from source.

sfan5 commented 3 years ago

The bug fix will need to be in mpv, no idea if/when/how that's gonna happen.

The reverted commits are (in order):

57462580180ecc9006b625b94e895a38e1118f69
6a13954d67143fb3c4ac8a4a7624c23e3ecb9a3c
0c9ac5835be70ae26e4aa875e833fe2c7b3b3bf3

Since I'm pretty sure there was a conflict here are the patch files: patch.zip

RAGEdemon commented 3 years ago

Can confirm - 4k videos with display-resample are a stuttery mess. Disabling HW acceleration as a workaround degrades performance even further because 4k videos need HW acceleration due to high processing demand.

sfan5 commented 7 months ago

Fixed upstream.