mpv-player / mpv

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

playlist: fix some playlist-prev/playlist-next edge cases #14387

Closed Dudemanguy closed 3 months ago

Dudemanguy commented 3 months ago

Previously, playlist-prev didn't work if you played a playlist to completion using --idle and tried to go back. Naturally, one would expect this to bring up the last item in the playlist, but nothing happens. This is just because playlist_get_next is stupid and doesn't take this into account since pl->current is NULL and thus returns NULL. Fix this by also considering the direction and grabbing the last playlist entry in the index.

github-actions[bot] commented 3 months ago

Download the artifacts for this pull request:

Windows * [mpv-i686-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1615092595.zip) * [mpv-x86_64-w64-mingw32](https://nightly.link/mpv-player/mpv/actions/artifacts/1615094379.zip) * [mpv-x86_64-windows-msvc](https://nightly.link/mpv-player/mpv/actions/artifacts/1615103574.zip)
macOS * [mpv-macos-12-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1615095652.zip) * [mpv-macos-13-intel](https://nightly.link/mpv-player/mpv/actions/artifacts/1615091865.zip) * [mpv-macos-14-arm](https://nightly.link/mpv-player/mpv/actions/artifacts/1615097703.zip)
Dudemanguy commented 3 months ago

I realized that playlist-next has exactly the same problem but the other way around. e.g., start mpv with --idle, loadfile or loadlist with append, and then try to go to the next file. So I added a commit to deal with that too.

verygoodlee commented 3 months ago

playlist-next works as expected, but playlist-prev doesn't work on idle. this case is append files to playlist after mpv start with --idle, not play a playlist to completion.

mpv --force-window --idle
# run command in console
loadfile 1.mp4 append
loadfile 2.mp4 append
playlist-prev

edit: In play a playlist to completion case, playlist-prev works but playlist-next doesn't work.

In my opinion, there is no difference between these two cases (both them are playlist-pos equals -1 and playlist-count not 0), they should have same behavior.

Dudemanguy commented 3 months ago

It is completely intended for playlist-prev to not work in that case. To me, your proposed behavior makes zero intuitive sense but I guess if enough people complain we can allow it.