torque / mpv-progressbar

A simple progress bar for mpv.
ISC License
154 stars 8 forks source link

Significant performance impact in paused state since mpv 0.31 #56

Closed FichteFoll closed 4 years ago

FichteFoll commented 4 years ago

Through some trial and error session, I was able to determine this script as being the culprit for causing my GPU usage to jump to thrice its normal load when playback is paused. Yes, this occurs especially when playback is paused and causes the graphics pipeline I observe via radeontop to jump from a healthy 20% to 70%, resulting in my gpu fans spinning audibly.

To reproduce:

The extend to how much GPU usage increases depends on the constallation of settings, but I am able to pretty easily reproduce it with the following mpv config folder (which is just progressbar.lua with the patch from #52 applied and the config file below). Unextract and use with --config-dir.

progressbar.tar.gz

interpolation=yes
video-sync=display-resample
deband=yes
deband-iterations=4
deband-threshold=70
deband-range=20
deband-grain=10

Regardless of the exact settings used, just having the script loaded (and no other settings) prevents GPU usage to drop to 0% (or 1%) while in paused state, which was not a problem before.

FichteFoll commented 4 years ago

Forgot my system information:

OS: Arch Linux x86_64 (Kernel: 5.5.8) GPU: AMD Radeon HD 7950 Drivers: mesa 19.3.4-2, xf86-video-amdgpu 19.1.0-1 Window Server: xorg-server 1.20.7-1

FichteFoll commented 4 years ago

It appears that the script that syncplay uses also causes this behavior, so I still experience this issue when not using progressbar but watching via syncplay.

FichteFoll commented 4 years ago

Through bisecting, I have determined that https://github.com/mpv-player/mpv/commit/07287262513c0d1ea46b7beaf100e73f2008295f is the first bad commit

Script below (I have radeontop in my sudoers config):

```sh #!/usr/bin/env zsh ./waf configure --prefix=/usr --confdir=/etc/mpv --enable-libmpv-shared || exit 125 ./waf build || exit 125 # start video ./build/mpv --config-dir=$HOME/tmp/mpv "some video file" & mpv_pid=$! echo "mpv_pid: $mpv_pid" # pause mpv sleep 1 xdotool key space # check gpu usage sleep 1 sudo radeontop -d - -l1 | rg 'gpu ([\d.]+)' -r '$1' -o | read -r usage # terminate mpv kill $mpv_pid printf "\nusage: %s\n" $usage if (( usage > 30 )); then exit 1; fi ```

Small excerpt from avih in #mpv@freenode:

[04-13][16:12:32] FichteFoll: then the script was probably using the undocumented OSD api, and now the old api is emulated for backward compatibility, but it may be slightly less efficient than before. the script author needs to use the new api or debug it to understand better [04-13][16:16:08] FichteFoll: if it turns out that the new cannot be used as efficiently as the old api, then it's worth reporting it at the issue tracker, at least to put it on record. so far, i don't think we've noticed such thing though [04-13][16:20:05] "osd-overlay" is the new command api, or mp.create_osd_overlay for scripts. the old api was scripting-only mp.set_osd_ass()

FichteFoll commented 4 years ago

I have determined this to be the result of the above commit removing mpv's internal update check on whether the ass text to render has been changed. I'll submit a fix to upstream tomorrow.

torque commented 4 years ago

Thank you for continuing to make high quality bug reports here, I really appreciate it. One day I will stop working 80 hour weeks and have some free time/brain energy to spend on personal projects once more.

I looked around at the other issues you opened, and I don't see much of an answer from mpv on what the correct thing to do here is. Is this fixable by adjusting some API usage, or does the new implementation require juggling my redraw loop around the pause state?

FichteFoll commented 4 years ago

It's fixable on the scripts side by only calling the osd ass API when the text to display has changed since the the previous call.

torque commented 4 years ago

Hmm, this is already supposed to happen, but my internal change detection is probably incorrect. I will investigate further.

torque commented 4 years ago

I haven't actually tried to measure the GPU usage impact on my local computer (there's no obvious external effect like fans spinning up for me), but I have improved/corrected the use of caching such that redraws should only be submitted to mpv when there is actually something that needs to be updated. At the very least, CPU usage appears to have decreased by a bit, possibly both during playback and when paused.

Edit: I double checked and the old version was not causing significantly increased GPU load when paused for me. Don't know if that's because I'm on macOS or if my mpv build is funky (it's something from git master around version 0.32).

The built version is here on the build branch. Please let me know if it did not properly address this issue so I can take another cut at it.

FichteFoll commented 4 years ago

Confirmed fixed. GPU usake still spikes when I continuously hover over the trigger area and move out again, like due to a high update frequency, but that's perfectly manageable.

I have switched over to uosc over the past weeks, as that didn't have the issue and provides some other cool and interesting features, however it's also missing some compared to progressbar. I'll see how I go about this in the future.

torque commented 4 years ago

uosc has the major advantage that it's under active development and appears to have done a better job of dealing with mpv API churn. It also generally looks pretty slick, but seems a bit heavy if all of the features are enabled.

I will keep mpv-progressbar alive, if for no other reason than because I still use it. That said, it's unlikely to see any interesting new features being added any time soon (by me, at least), so if that's a concern, I'm glad there are alternatives.

FichteFoll commented 4 years ago

I don't think progress bar needs any more features (aside from #52). It does its job pretty well imo, which is showing a progress bar and some other features related to starting or stopping playback (the icon animations). The only thing that uosc does over with regard to progressbar's features is highlighting chapter sections based on some regex rules, but I could easily do without that. It's mainly that you're unlikely to receive reports from me in the future when I don't use progressbar anymore, seeing that I do in fact like some of the features that uosc provides on top of that, along with the very fast responses and implementations on issues.

Unlike progressbar, uosc additionally has the goal to provide an actual hotkeyable interface for mpv with its menus, which I kinda dig and find pretty well implemented. One could argue that this would be better as a separate script following the UNIX philosophy, and I would, but the idea of having a full package replacement for the default osc is definitely a valid one. And I haven't even checked out its configuration file. I do miss the display of the title a bit, though, and having to press a key to have mpv display that will take some getting used to. And the stop-play icons to more easily keep track of the playback state, e.g. when a video is finished. I'll likely create issues for those in the near future.

Regardless, thanks for sticking around and for developing progressbar in the first place, as it has served me well over the past 3 or so years.