dyphire / mpv-scripts

userscripts for mpv
MIT License
53 stars 5 forks source link

chapter-make-read.lua Opening video file from NAS in large flat directory via SMB is very slow #17

Closed porg closed 1 year ago

porg commented 1 year ago

Reproduction

  1. Move chapter-make-read.lua into ~/.config/mpv/scripts/
  2. Open a local video file → Opens local video file plus chapter sidecar file if existent → ✅ Works in an instant.
  3. Open video file on NAS from folder which has 500+ video files → ❌ Takes 2-3 minutes to open and start playback!
  4. Move chapter-make-read.lua from ~/.config/mpv/scripts/ to ~/.config/mpv/scripts-off/
  5. Open video file on NAS → ✅ Takes the usual ca. 3 secs again. No chapter functionality ofc.
  6. Move chapter-make-read.lua back to ~/.config/mpv/scripts/ and move the video into a folder with no or only few sister files.
  7. Open video file on NAS → ✅ Takes the usual ca. 3 secs again. The chapters are shown.

Suspicion

Possible improvements

  1. Make lua file access aware whether it is local or network. And if network, make a call which really only searches for the existence of a file and avoid a large flat directory refreshing. → I estimate this to be not possible at all, or too much effort to implement (especially multi platform!)
  2. Workaround: Exclusion-list or inclusion-list of filepaths for loading chapter files.
    • Possibility to list multiple folder names and/or file names and/or filepaths for which to (not) open chapters:
    • Ideally this supports wildcards or RegEx, and has an easy way to distinguish file vs. folder,
    • e.g. the rsync include/exclude syntax would be a good existing syntax to use.
dyphire commented 1 year ago

This is a very practical issue, and scripts should not significantly affect the normal use of network sharing.

Workaround: Exclusion-list or inclusion-list of filepaths for loading chapter files

This is actually not difficult to implement, but the problem with this solution is that users only realize the need to exclude paths in the script after experiencing a bad experience and conducting troubleshooting.

Make lua file access aware whether it is local or network.

Yes, we cannot achieve this, which is the real issue.

dyphire commented 1 year ago

I have added a commit https://github.com/dyphire/mpv-scripts/commit/9de27403b9303ee2eff319760168d47c77b32e7f to try to improve this aspect of usage. Can you test if it would be helpful?

porg commented 1 year ago

Before I test could you please tell me

One more idea

Looking up the chapter file for this video took too long!

dyphire commented 1 year ago

Looking up the chapter file for this video took too long!

The reason for the slow loading of videos is not due to searching for chapters, but rather to the script's previous attempt to obtain a directory list. This can lead to poor performance on network shared paths.

To avoid such slowdowns you can include / exclude certain filepath(s) for chapter file lookup.

If the existing solution is not effective, I will consider it.

What fix strategy your commit follows? (Detecting local vs. remote file access?) Whether this is supposed to work on macOS? (it's Unix, but still some tools / syscalls may be different)

There will be no issues with local files, with a focus on performance on network sharing. SMB performance on the macOS platform has been consistently poor, but this issue should not be related to it.

The script previously attempted to obtain a directory list using utils.readdir and io.open functions, which unexpectedly took a lot of time on network sharing. The commit https://github.com/dyphire/mpv-scripts/commit/9de27403b9303ee2eff319760168d47c77b32e7f using utils.file_info function to avoid obtaining directory lists, hoping to significantly improve the performance of network sharing.

Update: The previous commit had some minor issues and has been pushed again. Please test new commit https://github.com/dyphire/mpv-scripts/commit/467f51f31592bf22999bd216ab6f24b1e5090a1e.

porg commented 1 year ago

✅ Commit 9de2740 fixed it really well!

What I tested

porg commented 1 year ago

So from my side we can close this as fixed 🙂

porg commented 1 year ago
$ ssh nas
$ cd /path/to/video-dir/
$ nano video-file-autocompletion   then add .chp
create your chapters
ctrl-x → save buffer → yes
dyphire commented 1 year ago

As expected, the problem lies in obtaining the directory list. fixed

porg commented 1 year ago

Thanks a lot!