-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
select.lua: select files with watch later files with g-w #15441
base: master
Are you sure you want to change the base?
Conversation
player/lua/select.lua
Outdated
local watch_later_dir = mp.get_property("watch-later-dir") | ||
watch_later_dir = mp.command_native({"expand-path", | ||
watch_later_dir ~= "" and watch_later_dir or "~~state/watch_later"}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be exported as property that uses mp_get_playback_resume_dir
for real directory, instead reimplementing the logic here.
Download the artifacts for this pull request: |
4582d9b
to
7f2cb9b
Compare
Improved performance by not |
In general, played files should be showed in the history regardless of whether the playback has finished or not, watch later file is only a temporary solution for history, another disadvantage is that it support for the network file is not good, because it only save URL don't save the title, it’s impossible for user to understand |
|
Are you trying it without compiling the new |
I did checkout and build mpv, so it should be there, no? |
Well how to reproduce then? I only get that error if |
|
player/lua/select.lua
Outdated
show_error("No watch later files with media paths found. " .. | ||
"Enable --write-filename-in-watch-later-config to select recent files.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This OSD message is too long. It should be concise and easy to read as it disappears fast.
Yeah only |
player/lua/select.lua
Outdated
local watch_later_dir = mp.get_property("current-watch-later-dir") | ||
|
||
if not watch_later_dir then | ||
show_error("No watch later files found.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is warning, not an error. Nothing fails, just is not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Every other select binding does this when nothing available though? Do you mean to change to mp.msg.warning?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Select edition shows error only if current-edition
is nil, but shows empty list when there are no editions available.
I just feel like, lack of files in history is not an error, so mp.msg.warning
. Maybe other indeed could be changed like that. Generally OSD message is enough, but we show also terminal message, errors are scary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's not intended. I wrote it blindly and thought no current-edtion means there are no editions like no chapter means there are no chapters in the previous binding.
player/lua/select.lua
Outdated
end | ||
|
||
if #watch_later_files == 0 then | ||
show_error("No watch later files found.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is warning, not an error. Nothing fails, just is not available.
player/lua/select.lua
Outdated
end | ||
|
||
if #recent_files == 0 then | ||
show_error("Enable --write-filename-in-watch-later-config to select recent files.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is warning, not an error. Nothing fails, just is not available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will show, even if this option is enabled, but there is not recent files saved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It prints No watch later files found. and returns earlier if no recent files are saved. This is specifically if you have watch later files but did not enable --write-filename-in-watch-later-config
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, and it will show all the time until you watch at least two files with this option, enabled. I think it should detect if this option is enabled. Because it was confusing to me to see this error, after enabling this option.
player/lua/select.lua
Outdated
end | ||
|
||
input.select({ | ||
prompt = "Select a recent file:", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I open directory, this will show only one file played as recent from this directory. Also opening this file will open file directly instead of directory.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't --autocreate-playlist load the other files in the directory if that's what you want?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am using this binding, it works well if you don't want to enable --autocreate-playlist
globally to not apply it to the initially opened file:
g-w script-binding select/select-watch-later; no-osd set file-local-options/pause yes; no-osd set autocreate-playlist filter; write-watch-later-config
Dunno if it should just always play the parent directory if not a URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Playing the parent directory would break if multiples files in that directory have watch later files, if e.g. dir/1.mp4 and dir/2.mp4 have them, selecting 2.mp4 would start playing 1.mp4, while with --autocreate-playlist
it correctly plays 2.mp4.
ea7e22b
to
593e0cf
Compare
Regarding this I think both watch later files and complete history file are useful in different cases. I also have a minimal script to log watched files to tmpfs just so I can resume from where I was in large directories when I close mpv by accident, while parsing watch later files is useful to check what you need to continue watching. It makes sense to reuse internal watch later files for an internal script. We can bind to g-w for watch later if needed to clarify that it's not a real history, or g-r for resume, but that needs rebinding show-properties. Showing |
442fccb
to
911ebbf
Compare
Made it print the date of watch later files and further improved performance. Also I can easily add a real history selection on top of this if wanted. |
That would be amazing, and if it also has a clear history item it would be perfect. |
Just saw your screenshot on irc for clear history. It is perfect! Sorry for the delayed response, my online time has been hectic lately. Genuinely well done, yet another step into improving the overall experience on mpv. |
It will be used in the next commit.
player/lua/select.lua
Outdated
|
||
local items = {} | ||
for i, file in ipairs(files) do | ||
items[i] = os.date("%Y-%m-%d ", file[2]) .. file[1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think would be good to have some separator between filename and date. Files often contains a date in filename so it might be confusing if there are both.
Implement selection of files to resume playing from watch later config files. Requires --write-filename-in-watch-later-config.
Implement selection of files with watch later config files. Requires --write-filename-in-watch-later-config.
Suggested by llyyr. He also suggested showing this when mpv opens, but it completely covers the logo and the text, and stays open after dropping a file. Alternatively "Drop files or URLs to play here." could also suggest pressing g-h.