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

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

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

Dudemanguy
Copy link
Member

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.

Copy link

github-actions bot commented Jun 18, 2024

Download the artifacts for this pull request:

Windows
macOS

@Dudemanguy Dudemanguy force-pushed the playlist-prev branch 2 times, most recently from 56fa020 to aa0a98e Compare June 18, 2024 23:47
@Dudemanguy Dudemanguy changed the title playlist: let playlist-prev go to last item in playlist playlist: fix some playlist-prev/playlist-next edge cases Jun 18, 2024
@Dudemanguy
Copy link
Member Author

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.

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 considering the direction, checking if the playlist was
played to completion and grabbing the last entry in the index.
Similar to the previous commit but the other way around. If you start
mpv as idle, load up a playlist without immediately hitting play and
then try to go to the next item, nothing happens. Naturally, you would
expect this to go to the first item. Fix this detecting if the playlist
has not started yet, the direction, and going to the first item in the
list.
Copy link
Contributor

@kasper93 kasper93 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense.

@Dudemanguy Dudemanguy merged commit 0db6aba into mpv-player:master Jun 25, 2024
20 checks passed
@Dudemanguy Dudemanguy deleted the playlist-prev branch June 25, 2024 02:19
@verygoodlee
Copy link

verygoodlee commented Jun 25, 2024

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
Copy link
Member Author

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.

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

Successfully merging this pull request may close these issues.

None yet

3 participants