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

--untimed performance regression since commit e3af545 #13622

Closed
owl0w1 opened this issue Mar 3, 2024 · 2 comments
Closed

--untimed performance regression since commit e3af545 #13622

owl0w1 opened this issue Mar 3, 2024 · 2 comments
Labels

Comments

@owl0w1
Copy link
Contributor

owl0w1 commented Mar 3, 2024

Important information

Version information:

  • mpv version: current master
  • Linux Distribution and Version: Ubuntu 24.04
  • Source of the mpv binary: compiled from source
  • Version of mpv that introduced the problem: commit e3af545

Reproduction steps

While setting up fuzzing of mpv, I noticed that execution with benchmark options (--untimed and friends) is significantly slower in current master than release 0.37.0. git bisect located the problem at commit e3af545.

First bad commit:

  1. Check out commit e3af545
  2. Build from source
  3. Benchmark performance of --untimed option
user@laptop:~/mpv-e3af545$ hyperfine './build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv'
Benchmark 1: ./build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv
  Time (mean ± σ):     515.6 ms ±   5.0 ms    [User: 216.0 ms, System: 51.4 ms]
  Range (min … max):   509.9 ms … 527.5 ms    10 runs
 
user@laptop:~/mpv-e3af545$ hyperfine './build/mpv --untimed ~/9bitwhite.mkv'
Benchmark 1: ./build/mpv --untimed ~/9bitwhite.mkv
  Time (mean ± σ):      1.951 s ±  0.018 s    [User: 0.342 s, System: 0.107 s]
  Range (min … max):    1.932 s …  1.982 s    10 runs

A 9 second video file took roughly 0.5s to play with vo=null and --untimed options. With normal video output and --untimed option only, it took roughly 1.9s to play.

Last good commit:

  1. Check out commit 7051e94, parent of commit e3af545
  2. Build from source
  3. Benchmark performance of --untimed option
user@laptop:~/mpv-7051e94$ hyperfine './build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv'
Benchmark 1: ./build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv
  Time (mean ± σ):     101.7 ms ±   3.1 ms    [User: 206.8 ms, System: 53.0 ms]
  Range (min … max):    95.7 ms … 108.8 ms    27 runs
 
user@laptop:~/mpv-7051e94$ hyperfine './build/mpv --untimed ~/9bitwhite.mkv'
Benchmark 1: ./build/mpv --untimed ~/9bitwhite.mkv
  Time (mean ± σ):      1.618 s ±  0.019 s    [User: 0.375 s, System: 0.093 s]
  Range (min … max):    1.593 s …  1.650 s    10 runs

The same 9 second video file took roughly 0.1s to play with vo=null and --untimed options. With normal video output and --untimed option only, it took roughly 1.6s to play.

There is a 5x performance regression in --untimed between these two commits.

Expected behavior

Do not sleep when outputting video frames with --untimed enabled.

Actual behavior

Additional delay other than decoding cost introduced between frames even when --untimed is enabled.

Sample files

The sample file used in reproduction steps: https://github.com/strongcourage/fuzzing-corpus/blob/master/mkv/9bitwhite.mkv

The issue seems to be independent of the input file.

@owl0w1 owl0w1 added the os:linux label Mar 3, 2024
@llyyr
Copy link
Contributor

llyyr commented Mar 3, 2024

can you try #13614 ?

@owl0w1
Copy link
Contributor Author

owl0w1 commented Mar 3, 2024

#13614 fixes the issue.

Benchmark:

user@laptop:~/mpv-410a9cd$ hyperfine './build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv'
Benchmark 1: ./build/mpv --vo=null --untimed --ao=null --ao-null-untimed ~/9bitwhite.mkv
  Time (mean ± σ):     120.7 ms ±   6.6 ms    [User: 267.8 ms, System: 56.3 ms]
  Range (min … max):   113.1 ms … 138.4 ms    26 runs

xrun1 pushed a commit to xrun1/mpv-svp-fix that referenced this issue Mar 12, 2024
Ended up being too flawed and caused trouble in other areas. There's
other approaches to trying to solve the issue this meant to address in
the works that should be better, so let's wait on that. Fixes mpv-player#13613 and
fixes mpv-player#13622.

This reverts commit e3af545.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants