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

long loading of the player if there are several thousand videos in the folder #7

Closed
AdventurerRussia opened this issue Mar 8, 2023 · 54 comments

Comments

@AdventurerRussia
Copy link

AdventurerRussia commented Mar 8, 2023

I noticed that when using the fuzzydir.lua script

the video runs for about 30 seconds if there are a lot of videos in the folder, is it possible to solve it somehow?
setting max_search_depth to 1 does not help
if you remove these lines from the config file, the player starts instantly

--audio-file-paths=**
--sub-file-paths=**

when you start the player, it starts like this for 30 seconds
image

@AdventurerRussia
Copy link
Author

@sibwaf

@AdventurerRussia
Copy link
Author

I tried the --audio-file-paths parameters=**
--sub-file-paths=** output a separate profile so that the script does not work when running, then I applied the profile and reloaded with a reboot script, but for some reason it does not work.

@sibwaf
Copy link
Owner

sibwaf commented Mar 8, 2023

Let me guess - you're opening something from a NAS share? Those tend to have really bad directory listing times, and in your case even with depth=1 a lot of paths will be added to sub-file-paths, so mpv will struggle to scan all of those.

It's finally time to implement #4, eh? I'll take a swing at it.

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 8, 2023

Let me guess - you're opening something from a NAS share? Those tend to have really bad directory listing times, and in your case even with depth=1 a lot of paths will be added to sub-file-paths, so mpv will struggle to scan all of those.

It's finally time to implement #4, eh? I'll take a swing at it.
Yes from the shared drive on the other PC
I also wanted to ask why the external tracks are not loaded when I apply the profile with the ** keys and reload the player with the reload script?

@sibwaf
Copy link
Owner

sibwaf commented Mar 8, 2023

Sorry, can you explain the profile stuff a bit more? A config or something would be really helpful.

Also - which reboot/reload script, the one I made?

@smad2005
Copy link

smad2005 commented Mar 8, 2023

@sibwaf Is it possible to implement it in an async way? (in a separate thread like "run and forget"). In such way, it won't block the main thread and prevent long init of player. I don't know lua, maybe it is called coroutine.

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 8, 2023

Sorry, can you explain the profile stuff a bit more? A config or something would be really helpful.

Also - which reboot/reload script, the one I made?

yes a simple profile

**[ext]
--audio-file-paths= **
--sub-file-paths= **

f1 show-text "ext" ; apply-profile "ext"**

and I reboot the player with your reload script

@sibwaf
Copy link
Owner

sibwaf commented Mar 8, 2023

@sibwaf Is it possible to implement it in an async way? (in a separate thread like "run and forget"). In such way, it won't block the main thread and prevent long init of player. I don't know lua, maybe it is called coroutine.

Yeah, it's possible, but not that good from a usability standpoint IMO. Sure, you got your video playing immediately, but subtitles will be missing for like a minute or more in a bad case. Or even worse - you opened a video with embedded subtitles you wanted to use, then after a minute they suddenly get switched with the other ones.

Sorry, can you explain the profile stuff a bit more? A config or something would be really helpful.
Also - which reboot/reload script, the one I made?

yes a simple profile

**[ext] --audio-file-paths= ** --sub-file-paths= **

f1 show-text "ext" ; apply-profile "ext"**

and I reboot the player with your reload script

Alright, I'll take a look at this after the slow startup stuff.

@smad2005
Copy link

smad2005 commented Mar 8, 2023

https://github.com/jjensen/lua-filefind (just example) maybe need to use more effective lookup method. It use FindNextFileA OS native function that has a buildin filter with cache.

@sibwaf
Copy link
Owner

sibwaf commented Mar 8, 2023

https://github.com/jjensen/lua-filefind (just example) maybe need to use more effective lookup method. It use FindNextFileA OS native function that has a buildin filter with cache.

Wouldn't help, sadly. Half of the time is spent by the script trying to discover all the directories, and the other half is spent by mpv scanning the same directories for subtitle files.

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 8, 2023

Alright, I'll take a look at this after the slow startup stuff.

@sibwaf Is it possible to implement it in an async way? (in a separate thread like "run and forget"). In such way, it won't block the main thread and prevent long init of player. I don't know lua, maybe it is called coroutine.

Yeah, it's possible, but not that good from a usability standpoint IMO. Sure, you got your video playing immediately, but subtitles will be missing for like a minute or more in a bad case. Or even worse - you opened a video with embedded subtitles you wanted to use, then after a minute they suddenly get switched with the other ones.

Sorry, can you explain the profile stuff a bit more? A config or something would be really helpful.
Also - which reboot/reload script, the one I made?

yes a simple profile
[ext] --audio-file-paths= ** --sub-file-paths= **
f1 show-text "ext" ; apply-profile "ext"

and I reboot the player with your reload script

Alright, I'll take a look at this after the slow startup stuff.

I tried to load --sub-auto=fuzzy through the profile and it works as it should, loading tracks on command and after restarting the player, though it doesn't matter even if --sub-auto=fuzzy is not present when the player starts, the player starts for a long time because of the script and **
so this crutch did not solve the problem, everything rests on a long launch of the player.

@sibwaf
Copy link
Owner

sibwaf commented Mar 8, 2023

@AdventurerRussia

Pushed a couple of changes for startup speedup, should be a lot better now. The lag still exists in some cases, but it's not nearly as bad as it was before. I'm not really sure if it's possible to improve it further.

As for the profiles - yup, can confirm, doesn't work. The reason for this is that fuzzydir reads config values at the mpv start and then replaces them. Thus, we can't really handle config changes as we wouldn't know if the value we see is user-provided or is already post-processed. Maybe could be solved somehow with property observers, but no guarantees. How much do you need this? I can try playing around, but I'd rather not to be honest.

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 9, 2023

@sibwaf sadly, it didn't help, the player also takes a long time to start.
as for profiles, it's not much and it's not necessary, It would be better to find a solution to the problem with a long launch of the player.
and so thank you for answering and trying to help.

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Like, no change in loading time at all?

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 9, 2023

Like, no change in loading time at all?

Right now I've got a stopwatch
new 28 sec
old 49

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 9, 2023

I was wrong here, it just felt like it was running for almost as long and I decided that nothing had changed,

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Right, no worries. Could you please run dir /ad with your terminal in that directory and check how long that takes?

@AdventurerRussia
Copy link
Author

Right, no worries. Could you please run dir /ad with your terminal in that directory and check how long that takes?

I'm sorry, how do I do this? is it to type cmd in the folder and enter dir/ad? the window opens instantly, or do I have to do something else?

@AdventurerRussia
Copy link
Author

or you need to enter mpv dir /ad
?

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Open the directory in Explorer -> Shift + Right click -> there should be an option to open a terminal here. And then just run the command.

Though Windows can also miserably fail to do this as far I can remember and set the working directory to drive C or something. If it does fail you can try following instructions from here (scroll until "How to access network path using command line in Windows 10")

@AdventurerRussia
Copy link
Author

image
I was also advised tree / f and it displays all files in 1 second
image

@AdventurerRussia
Copy link
Author

instantly displays a list of files

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Oh, so you don't have any subfolders. No wonder the fix didn't really help.

Was the dir /ad fast?

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 9, 2023

Oh, so you don't have any subfolders. No wonder the fix didn't really help.

Was the dir /ad fast?

instantly output the information to the command line

@AdventurerRussia
Copy link
Author

image
image

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Alright, I'll try incorporating this instead of the mpv's one I'm currently using.

@sibwaf
Copy link
Owner

sibwaf commented Mar 9, 2023

Ugh, Windows is certainly going to make me lose my mind...

Can you please:

  1. Create a new empty directory, something like "coub/testdir"
  2. Open PowerShell and run Get-ChildItem -Path "FileSystem::\\full\samba\path\to\coub" -Directory | foreach { $_.Name }
  3. Check that this script works AND has acceptable performance
  4. Provide me with whatever it managed to output?

Don't have a Windows 10 installation handy, sorry.

@AdventurerRussia
Copy link
Author

Get-ChildItem : The path "\full\samba\path\to\coub" cannot be found because it does not exist.

--
did I run PowerShell from under the folder, or do I need to change the paths to my own?

@sibwaf
Copy link
Owner

sibwaf commented Mar 10, 2023

Yep, sorry if it wasn't clear. The working directory could be anywhere, just specify the full path (keep the FileSystem:: part)

@smad2005
Copy link

for windows

local utils = require "mp.utils"
local msg = require "mp.msg"

function traverse_directory(path, extension)
    local seen = {}
    local result = {}
    local file_handle = io.popen('for /r "' .. path .. '"  %i in (' .. extension .. ') do @echo %~dpi')
    for file in file_handle:lines() do
        if not seen[file] then
            seen[file] = true
            table.insert(result, file)
        end
    end
    file_handle:close()
    return result
end

function run()
    debug_print("------------SCRIPT START---------------")
    local video_path = mp.get_property("path")
    local search_path, _ = utils.split_path(video_path)

    local audio_paths = traverse_directory(search_path, "*.mka,*.ac3,*.aac")
    debug_print_array(audio_paths, "audio_paths list")
    mp.set_property_native("options/audio-file-paths", audio_paths)

    local sub_paths = traverse_directory(search_path, "*.ass,*.srt")
    debug_print_array(audio_paths, "sub_paths list")
    mp.set_property_native("options/sub-file-paths", sub_paths)
    debug_print("------------SCRIPT END---------------")
end

mp.add_hook("on_load", 50, run)

----
function debug_print(value)
    local time = os.date("%Y-%m-%d %H:%M:%S", current_time)
    msg.trace(time .. " : " .. value)
end

function debug_print_array(arr, headerText)
    debug_print(headerText)
    for _, value in ipairs(arr) do
        debug_print(value)
    end
    debug_print(headerText .. ' end')
end

image

@sibwaf
Copy link
Owner

sibwaf commented Mar 10, 2023

Oh, that's great, thanks a bunch!

The only problem is that this uses cmd (I guess?), which can't handle share paths directly. You're either using a local disk, or have a letter assigned to the share. The only workaround I managed to find is using PowerShell which can handle paths like \\server\share\somethingsomething - those appear when you open a file directly from the share.

Care to replace the command with powershell.exe -Command "$ { Get-ChildItem -Path "FileSystem::INSERT_PATH_HERE" -Directory | foreach { $_.Name } }" and check how it goes if it is indeed a share? Even better - if you have a bunch of files/directories over there.

@AdventurerRussia
Copy link
Author

it didn't output anything, or am I doing something wrong
image

@sibwaf
Copy link
Owner

sibwaf commented Mar 10, 2023

No-no, it certainly did output testdir, just what I wanted it to do. How's the performance?

@AdventurerRussia
Copy link
Author

No-no, it certainly did output testdir, just what I wanted it to do. How's the performance?

it worked instantly

@sibwaf
Copy link
Owner

sibwaf commented Mar 10, 2023

Great! I'll try making a quick patch if you're willing to test.

@AdventurerRussia
Copy link
Author

of course, it's in my best interest for it to work well.

@sibwaf
Copy link
Owner

sibwaf commented Mar 10, 2023

Here's the patch (hope I didn't mess up, can't really debug it myself without Windows):

  1. Add this function before traverse:
function fast_readdir(path)
    local process = mp.command_native({
        name = "subprocess",
        playback_only = false,
        capture_stdout = true,
        args = { "powershell.exe", "-Command", "& { Get-ChildItem -Path \"FileSystem::" .. path .. "\" -Directory | foreach { $_.Name } }" },
    })

    if process.status ~= 0 then
        return nil
    end

    local result = {}
    for line in string.gmatch(process.stdout, "([^\r\n]+)") do
        table.insert(result, line)
    end
    return result
end
  1. Replace utils.readdir(full_path, "dirs") with fast_readdir(full_path)

@sibwaf
Copy link
Owner

sibwaf commented Mar 19, 2023

Hey @AdventurerRussia , did you have a chance to check out the patch?

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 19, 2023

while my computer server broke down on which there was a hard disk with a video from where I tested the launch, I will buy a pocket for a hard disk the other day and check.
I have the main laptop.

@AdventurerRussia
Copy link
Author

@sibwaf in short, we checked on the folders of common acquaintances, he says that when 2500 files were in the folder with the oldest script, the player was running for 10 seconds, and it began to run 3.

@AdventurerRussia
Copy link
Author

so I think you can make these edits to the main code that you gave above.

@AdventurerRussia
Copy link
Author

I think this is great news, thank you very much for paying attention and improving the script.

@dyphire
Copy link
Contributor

dyphire commented Mar 23, 2023

There are still issues with that patch, and many normal directories may fail to read.
The following is the error log information:

[  21.304][e][fuzzydir] Get-ChildItem : Cannot retrieve the dynamic parameters for the cmdlet. The specified wildcard character pattern is not 
[  21.304][e][fuzzydir] valid: [MSRSub&Todokoi] Seitokai Yakuindomo S2 OAD - 25 [DVDRip 720p HEVC-10bit AC3]
[  21.304][e][fuzzydir] At line:1 char:5
[  21.304][e][fuzzydir] + & { Get-ChildItem -Path "FileSystem::F:\Download\[MSRSub&Todokoi] Sei ...
[  21.304][e][fuzzydir] +     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[  21.304][e][fuzzydir]     + CategoryInfo          : InvalidArgument: (:) [Get-ChildItem], ParameterBindingException
[  21.304][e][fuzzydir]     + FullyQualifiedErrorId : GetDynamicParametersException,Microsoft.PowerShell.Commands.GetChildItemCommand
[  21.304][e][fuzzydir]  

@sibwaf
Copy link
Owner

sibwaf commented Mar 23, 2023

@AdventurerRussia Sure, no problem. Glad I managed to help.

@dyphire Oof. Yeah, seems that PowerShell is pretty funky with it's strings. Could you please try replacing:

-- this:
"& { Get-ChildItem -Path \"FileSystem::" .. path .. "\" -Directory | foreach { $_.Name } }"

-- with this:
"& { Get-ChildItem -LiteralPath FileSystem::\"" .. path .. "\" -Directory | foreach { $_.Name } }"

@dyphire
Copy link
Contributor

dyphire commented Mar 23, 2023

@AdventurerRussia Sure, no problem. Glad I managed to help.

@dyphire Oof. Yeah, seems that PowerShell is pretty funky with it's strings. Could you please try replacing:

-- this:
"& { Get-ChildItem -Path \"FileSystem::" .. path .. "\" -Directory | foreach { $_.Name } }"

-- with this:
"& { Get-ChildItem -LiteralPath FileSystem::\"" .. path .. "\" -Directory | foreach { $_.Name } }"

It works

@AdventurerRussia
Copy link
Author

@sibwaf
there is a mistake, if the name of the folder where the files are in Russian letters, the tracks are not connected, even if the name is only one Russian letter

@AdventurerRussia
Copy link
Author

image
image

@AdventurerRussia
Copy link
Author

and if the folders have Russian letters in the paths, then the following error appears. But files are connected if they are in folders with English letters

@AdventurerRussia
Copy link
Author

AdventurerRussia commented Mar 25, 2023

G:\Торенты[ReinForce] Aharen-san wa Hakarenai (BDRip 1920x1080 x264 FLAC)

@dyphire
Copy link
Contributor

dyphire commented Mar 25, 2023

@sibwaf there is a mistake, if the name of the folder where the files are in Russian letters, the tracks are not connected, even if the name is only one Russian letter

This is because the script cannot receive unicode from powershell in subprocess command.
The PR #8 should fixes it.

@AdventurerRussia
Copy link
Author

I didn't understand a little what --merge-files does, but with it, external tracks don't connect and such an error comes out, although they didn't connect with the old script either.
image
true, it is not so important, but I just noticed such a thing.

@AdventurerRussia
Copy link
Author

I came across this when I was looking for a way to speed up the player switching between videos

@sibwaf
Copy link
Owner

sibwaf commented Mar 26, 2023

That's a different problem. You can make a separate issue for this and I'll take a look.

@AdventurerRussia
Copy link
Author

It doesn't matter, I just noticed it and decided to report it.,

@sibwaf sibwaf closed this as completed Mar 26, 2023
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

No branches or pull requests

4 participants