Delete "watch later" config when ending playback of a file #8141
Conversation
CyberShadow
commented
Oct 3, 2020
CyberShadow
commented
CyberShadow
force-pushed
the
unlink-watch-later
branch
2 times, most recently
from
e86be2a
to
dea7902
Compare
| mpctx->stop_play == PT_NEXT_ENTRY || | ||
| mpctx->stop_play == PT_CURRENT_ENTRY) | ||
| mp_clear_playback_resume(mpctx, mpctx->filename); | ||
|
|
There was a problem hiding this comment.
Instead of doing this, why not call the function in the switch/case statement farther down? Also, do you really have to care about anything other than AT_END_OF_FILE for this?
There was a problem hiding this comment.
The goal of that switch seems to be rather different, with the relevant stop_play values overlapping in a disjoint way. I don't see a reasonable way to combine them. Though, I will say that I did not see an "obviously correct" place for where to insert this check within the function, so its position is arbitrary to some extent.
There was a problem hiding this comment.
Do you need PT_NEXT_ENTRY or PT_CURRENT_ENTRY though? I thought the goal was to delete it at the end of playback of a file.
There was a problem hiding this comment.
I believe so. Otherwise, if you navigate to the next file, then re-open mpv with the same playlist, it will play the file that was navigated away from.
There was a problem hiding this comment.
That behavior is intentional although it can certainly be argued that said behavior could be changed/made optional. Deleting any watch_later data when you get AT_END_OF_FILE makes sense to me though (not sure how other people feel).
There was a problem hiding this comment.
I don't understand which goal would be furthered by not calling mp_check_playlist_resume.
If you don't want a playlist to automatically start from the first file that has watch later data, but just to resume from the saved position when you actually navigate/reach that file in the playlist. Seems like a valid use case and you mentioned that earlier in this comment thread (guess that doesn't address everything though). As for what you listed in #8138, the watch_later stuff all works on a per-file basis not on a playlist basis hence why you see that behavior.
There was a problem hiding this comment.
If you don't want a playlist to automatically start from the first file that has watch later data, but just to resume from the saved position when you actually navigate/reach that file in the playlist. Seems like a valid use case and you mentioned that earlier in this comment thread (guess that doesn't address everything though).
I don't think I did; if something I said implied that, then I misspoke, sorry.
If we want to consider users wanting to detach playlist position and individual file position, then that's a whole other use case. Here and in #8138 I only consider the perspective where there exists one single playback position per playlist, which consists of the file within the playlist, and then the position within the file.
As for what you listed in #8138, the watch_later stuff all works on a per-file basis not on a playlist basis hence why you see that behavior.
Yes. I linked to the relevant code there as well (mp_check_playlist_resume again), so I think we've already established that we are on the same page here.
It seems to me like we're beginning to talk past each other, so perhaps it would be better if you could list any pending questions you have that I can address, without referencing prior discussion.
There was a problem hiding this comment.
Right it seems we're talking a bit in circles here. My apologizes.
Anyways back to the PR itself, the reason for my objection to PT_NEXT_ENTRY is that it would delete watch_later data by simply advancing to the next item in the playlist. That seems undesirable to me. Someone could want to save data with the write-watch-later-config, skip forward somewhere else in the playlist, and reasonably expect that their saved data is well, still saved. I confess that I'm actually not totally sure what triggers PT_CURRENT_ENTRY though.
There was a problem hiding this comment.
I see what you mean. I admit I haven't really considered this case, as it seems to rely on an artifact of mpv's current playlist playback position model, which I understand is what we have out of simplicity rather than intentional design; but even so, I agree that it may be unfair to dismiss it outright based on that argument alone.
I can't judge if continuing to support this hypothetical use case warrants a more complicated patch, but I'd be willing to have a go at whatever solution we decide is best (though something as comprehensive as fixing #8138 would probably be better tackled by a veteran).
One more possible approach is to add an optional flag for write-watch-later-config which tells mpv whether it should delete the file when it's done playing. Though the patch would be small, this seems unattractive in that it solidifies a quirk that's an outcome of the current playlist logic as part of the API.
I think I'll revisit the delete-watch-later-config command idea, it's starting to look like the best solution after all.
There was a problem hiding this comment.
Yeah, a delete-watch-later-config command sounds like a good idea to me.
|
The idea seems fine. I think it's more intuitive for watch later data to delete itself if you actually play all the way through a file. The commit message would need to be fixed as per the contribution guidelines as well. |
This is the most minimal change in mpv which allows having "persistent" playback resuming. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. However, it is possible to make the above scripts "just work" with one simple change - delete the state file not just on startup (when it is read), but also when ending the playback of a file. Doing so deletes any saved positions written by write-watch-later-config; the script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A small modification of the one written by @Hakkin, in that this one also saves the state immediately when a new file is loaded.)
CyberShadow
force-pushed
the
unlink-watch-later
branch
from
dea7902
to
d46498c
Compare
Fixed, I think. Thanks for pointing that out. |
|
Like Dudemanguy said I do occasionally execute Edit: if doing that it would also be necessary to check if the resume files haven't been updated since mpv started, or |
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
Good to know, thanks for confirming.
Though not an unreasonable idea, I feel like this would only bury us deeper in quirks-sand. Things behaving one way while mpv is running and suddenly a different way after it's restarted will add more complexity to the model and opportunity for confusion or frustration.
OK, here it is: #8199 |
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to #8141. The general problem that this change is attempting to help solve has been described in #336, #3169 and #6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in #8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
|
I think with #8199, it's okay to close this now. |
This introduces the delete-watch-later-config command, to complement write-watch-later-config. This is an alternative to mpv-player#8141. The general problem that this change is attempting to help solve has been described in mpv-player#336, mpv-player#3169 and mpv-player#6574. Though persistent playback position of a single file is generally a solved problem, this is not the case for playlists, as described in mpv-player#8138. The motivation is facilitating intermittent playback of very large playlists, consisting of hundreds of entries each many hours long. Though the current "watch later" mechanism works well - provided that the files each occur only once in that playlist, and are played only via that playlist - the biggest issue is that the position is lost completely should mpv exit uncleanly (e.g. due to a power failure). Existing workarounds (in the form of Lua scripts which call write-watch-later-config periodically) fail in the playlist case, due to the mechanism used by mpv to determine where within a playlist to resume playback from. The missing puzzle piece needed to allow scripts to implement a complete solution to this problem is simply a way to clean up the watch-later configuration that the script asked mpv to write using write-watch-later-config. With that in place, scripts can then register an end-file event listener, check the stop playback reason, and in the "eof" and "stop" case, invoke delete-watch-later-config to delete any saved positions written by write-watch-later-config. The script can then proceed to immediately write a new one when the next file is loaded, which altogether allows mpv to resume from the correct playlist and file position upon next startup. Because events are delivered and executed asynchronously, delete-watch-later-config takes an optional filename argument, to allow scripts to clear watch-later configuration for files after mpv had already moved on from playing them and proceeded to another file. A Lua script which makes use of this change can be found here: https://gist.github.com/CyberShadow/2f71a97fb85ed42146f6d9f522bc34ef (A modification of the one written by @Hakkin, in that this one takes advantage of the new command, and also saves the state immediately when a new file is loaded.)
