mpv-player / mpv

🎥 Command line video player
https://mpv.io
Other
26.65k stars 2.83k forks source link

demux: allow initial refresh seek for for non-video streams #14399

Closed na-na-hi closed 6 days ago

na-na-hi commented 1 week ago

Since d41f0a54b07d855227b63a6b2c4e92794ff7b86f, enabling a stream will no longer perform a refresh seek, to fix issues with switching video tracks. Additionally, 62e9a0c5f6be63c4cbe6387cbd3419fe19e98b74 also prevents refresh seek on track switching when it happens right after a seek.

Unfortunately, when external audio files are loaded, preventing refresh seeks can cause A-V sync issues. Since external tracks have separate demuxer instances from the internal tracks, the demuxer instances from both internal and external tracks are explicitly sought to the same pts when seeking is performed. When enabling an external audio track for the first time, the existing logic prevents refresh seek because no packets have ever been read, so the track ends up not being sought. This means that switching the track in the middle of playback results in a huge A-V desync.

To make it worse, unlike one demuxer instance with multiple tracks, tracks from multiple demuxer instances are not synced in any meaningful way beyond the separate explicit seeking. The only thing which keeps these tracks synced is the generic A-V sync logic, which is unfit for such large desync above. This means the audio is at the start position after the track is switched, behind the video, so the audio is not considered ready, and audio is continuously filtered sequentially until it is synced to the video. While this is happening, the audio filtering exhausts all CPU resources.

Fix this by allowing an initial refresh seek for audio streams, even when no packets have been read yet. initial_state needs to be handled separately from after_seek since refresh seek on track switching right after a seek is not needed.

Fixes: #13915

github-actions[bot] commented 1 week ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1627706530.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1627708046.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1627713573.zip)
macOS * [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1627706202.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1627706571.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1627705592.zip)
kasper93 commented 6 days ago

I know nothing about this code, @Dudemanguy probably knows about A-V sync code.

dyphire commented 6 days ago

I can confirm that it did fix #13915

na-na-hi commented 6 days ago

Decided to always allow refresh seek for non-video streams, which also fixes the problem with cache range joining caused by a seek.

kasper93 commented 6 days ago

Have you maybe tried similar case, when ytdl is used with thumbnails and if you change from thumbnail to video? For some time there is terrible stuttering until it catches up with sync.

I think this is something similar to

While this is happening, the audio filtering exhausts all CPU resources.

na-na-hi commented 6 days ago

Have you maybe tried similar case, when ytdl is used with thumbnails and if you change from thumbnail to video? For some time there is terrible stuttering until it catches up with sync.

Can you provide replication steps? Does the CPU usage go up significantly in this case? If not, then it sounds like standard A-V sync correction to me when audio is behind, so video is slowed down until synced.

kasper93 commented 6 days ago

Can you provide replication steps? Does the CPU usage go up significantly in this case? If not, then it sounds like standard A-V sync correction to me when audio is behind, so video is slowed down until synced.

Probably you are right. I would have to find a way to reproduce. I tried now and it works pretty good. Don't worry about it.

na-na-hi commented 5 days ago

I just noticed that after this commit, the sample file and command from #13641 no longer causes the player to hang. It still hangs if --video=no is added though.

It seems the added refresh seek causes an EOF seek for the video which terminates playback.