gaesa / mpv-autoload

A TypeScript implementation of mpv’s autoload.lua. It is optimized for modularity, readability and and simplicity.
GNU Lesser General Public License v2.1
9 stars 0 forks source link

Error due having mix files other than audio and video on the same directory #24

Closed Mav3r1ckHL closed 8 months ago

Mav3r1ckHL commented 8 months ago

Test scenario: Have a few video files and mix in a file that is not of the type audio of video such as a pdf file in the same folder.

Assumptions:

Steps to Reproduce:

  1. Select a video and launch mpv based on the expected test scenario
  2. Use the ` key to check the status to review error message

What happens: Autoload will fail to load due to having a file other than audio or video such as a pdf.

The following error occurs for autoload in the logs: [f][autoload] ProcessError: Command ["file","-b","--mime-type","--","D:/Temp/1pdftest.pdf"] returned non-zero exit status -3 [f][autoload] at Function.prototype.call (native) [f][autoload] at n (D:/MPV/mpv/portable_config/scripts/autoload.js:1) [f][autoload] at f (D:/MPV/mpv/portable_config/scripts/autoload.js:1) [f][autoload] at D:/MPV/mpv/portableconfig/scripts/autoload.js:1 [f][autoload] at (D:/MPV/mpv/portable_config/scripts/autoload.js:1) [f][autoload] at D:/MPV/mpv/portable_config/scripts/autoload.js:1 [f][autoload] at Array.prototype.filter (native) [f][autoload] at D:/MPV/mpv/portable_config/scripts/autoload.js:1 [f][autoload] at D:/MPV/mpv/portable_config/scripts/autoload.js:1 [f][autoload] at D:/MPV/mpv/portable_config/scripts/autoload.js:1 [f][autoload] at D:/MPV/mpv/portable_config/scripts/autoload.js:1 [f][autoload] at dispatch_event (@/defaults.js:52) [f][autoload] at mp_event_loop (@/defaults.js:791) [f][autoload] at run_script (native)

Expected behavior: Should ignore any other files other than the video and audio files within the same directory. This was handled correctly with the base autoload.lua file.

gaesa commented 8 months ago

Thank you for your report.

Before proceeding with troubleshooting, could you please confirm if you have the file command dependency installed on your system and make it available in PATH?

Additionally, I attempted to reproduce the issue on my end using the provided commands, but I encountered no errors. To further investigate, can you manually run file command yourself?

My test:

PS C:\Users\user> mpv --no-config --script=Z:\autoload.js '.\Downloads\example.flac'
PS C:\Users\user> file -b --mime-type -- .\Downloads\example.pdf
application/pdf
PS C:\Users\user> file -b --mime-type -- Downloads/example.pdf
application/pdf
PS C:\Users\user> file -b --mime-type -- Downloads\example.pdf
application/pdf
PS C:\Users\user> file -b --mime-type -- C:/Users/user/Downloads/example.pdf
application/pdf
Mav3r1ckHL commented 8 months ago

I am not too sure what the "file" command is. I don't recall Windows ever having a command type of that nature, but I did already put the mpv folder as an user based environmental PATH variable to execute mpv on any path. I did your test example just with the first command and I got the same error when mixing file types on the same directory.

PS C:> mpv --no-config --script="d:\autoload.js" D:\Temp\test.mkv (+) Video --vid=1 () (hevc 1920x1080 23.976fps) (+) Audio --aid=1 --alang=jpn () (aac 2ch 44100Hz) (+) Subs --sid=1 --slang=eng (*) 'English subs' (ass) AO: [wasapi] 48000Hz stereo 2ch float [autoload] ProcessError: Command ["file","-b","--mime-type","--","D:/Temp/1pdftest.pdf"] returned non-zero exit status -3 [autoload] at Function.prototype.call (native) [autoload] at n (d:\autoload.js:1) [autoload] at f (d:\autoload.js:1) [autoload] at d:\autoload.js:1 [autoload] at _ (d:\autoload.js:1) [autoload] at d:\autoload.js:1 [autoload] at Array.prototype.filter (native) [autoload] at d:\autoload.js:1 [autoload] at d:\autoload.js:1 [autoload] at d:\autoload.js:1 [autoload] at d:\autoload.js:1 [autoload] at dispatch_event (@/defaults.js:52) [autoload] at mp_event_loop (@/defaults.js:791) [autoload] at run_script (native) VO: [gpu] 1920x1080 yuv420p10 [autoload] Could not load js script d:\autoload.js AV: 00:00:02 / 00:23:40 (0%) A-V: 0.000 ct: -0.083 Dropped: 1 Exiting... (Quit)

Is "file" suppose to be a common command or is this a manual download I get to get somewhere and register its location under the windows PATH environment?

gaesa commented 8 months ago

The file command is a dependency of autoload.js. It's used to determine the MIME type of files when their extensions don't match any configured extensions.

On Windows, you usually need to manually download and install the file command. Please refer to the README.

Mav3r1ckHL commented 8 months ago

Yeah thanks that works now, had to review that portion again. So some personal feedback on this feature. From a "portability" perspective this will be difficult for the average user since they would need to setup the environmental variable along with either downloading a file or executing a Chocolatey from Powershell.

I will be personally using this as this is pretty good, but for full portable drag/drop of the portable_config for other users I will have them stick with the lua for now. If this can be done in a way without said dependencies in the future it would be great.

gaesa commented 8 months ago

Glad to hear that this works for you now.

Regarding portability, the lua version doesn't detect MIME types, which makes it more portable. However, users need to pay closer attention to file extensions. In my project, MIME types act as a fallback mechanism, with only common extensions checked initially. This approach enhances flexibility since users can overlook rare, new, or ambiguous extensions (like .ts files, which could be either text or video files).

Yet, for robust MIME type detection, my version rely on external dependencies like file. While this works seamlessly on Linux where file is typically available, it's not as portable for users on other operating systems. I could utilize npm packages for MIME type detection and bundle them for portability, but this would significantly increase the script's size without benefiting Linux users. Managing different versions for various platforms would complicate maintenance, so I prefer to stick with the current approach.

You may want to configure the script for heavier reliance on extensions by adding more extensions. See README.