search

davidde / mpv-autosub

Fully automatic subtitle downloading for the MPV media player
https://mpv.io/
MIT License
295 stars 42 forks source link

subliminal is not using the -d parameter anymore, needs the full path #10

Closed bxabi closed 4 years ago

bxabi commented 4 years ago

Sublimal fails to find the movie to calculate the hashes when the -d parameter is given.

davidde commented 4 years ago

I'm not sure why you think subliminal is not using the -d parameter anymore, but it hasn't changed; check the subliminal docs to verify.

This seems like something wrong on your side. My best guesses would be either a messed up control_downloads function (which initializes the directory variable), or a bad file naming scheme (e.g. series_name/season_1/episode_1 would just send episode_1 to subliminal download).

bxabi commented 4 years ago

Hi, I only saw the docs on the github page of sublimal, where this was not mentioned. As I see it is for where to save the subtitle. So I suppose it should be kept. But it is not used for finding the movie, the movie needs the full path instead of just the file name, otherwise only the string search is used, the hashes not. I can submit a different fix then, or you can also just make the change fitting the existing code best and I remove my fork. I needed this fix because the download only worked if the script is run from the folder of the movie, but not from outside (and SMPlayer calls it with the full path).

davidde commented 4 years ago

But it is not used for finding the movie, the movie needs the full path instead of just the file name, otherwise only the string search is used, the hashes not.

I don't know what you mean here, but subliminal needs only the movie name, and not the full path. If the movie name is in the parent folder instead of the filename, then I guess it would need the full path, but that is generally considered a bad practice.

I needed this fix because the download only worked if the script is run from the folder of the movie, but not from outside (and SMPlayer calls it with the full path).

I'm sorry, but I cannot reproduce this. Downloading works fine for me from wherever I run my mpv command.

davidde commented 4 years ago

It might be a platform difference though. Are you on Windows, Mac or Linux?

bxabi commented 4 years ago

You can't reproduce it probably because you are testing a movie where the downloading by the filename works. I have a series where it doesn't, the download succeeds by the hash. I can paste the log. If the command is run from outside the movie folder, the hash is skipped.

[autosub] DEBUG:subliminal.cli:Collecting path The Office [2.14] The Carpet.avi [autosub] INFO:subliminal.core:Scanning video 'The Office [2.14] The Carpet.avi' in '' [autosub] DEBUG:subliminal.core:Size is 183564288 [autosub] DEBUG:subliminal.core:Computed hashes {'opensubtitles': 'f0a105839b81d5a3', 'shooter': '66b242134125964a423109b27718c110;b76ac049dcc57e7bd3eb59a8b504ab16;059ec92f1fddbbf5a740d1cceac7990c;0c8cc8ab61852610d663b43fe9d560dc', 'thesubdb': '74a0f67b03aa51e1d508b841e06ea974', 'napiprojekt': '8de7b51e9b1ced4e65e3afb6f5b06f4c'} [autosub] INFO:subliminal.core:Refining video with metadata

The line "[autosub] DEBUG:subliminal.core:Computed hashes" is missing when the movie is given without the path, therefore searches using that are skipped, and less subtitles are found. Sending the movie with full path solves the issue.

davidde commented 4 years ago

This does mean that the downloading works in the majority of cases though.

It looks like this is an internal inconsistency in subliminal; the first PR claimed the exact opposite of this one.

Since it is unclear to what extent the success rate is influenced by the omission of the path, I feel like we need more info on this, or at least more example cases before reverting back to the full path. It's a difficult trade-off, but I hope you can understand why I won't merge this now.

bxabi commented 4 years ago

I made a second commit to keep the -d parameter.

There is no trade-off, as giving the full path has no drawback, just the advantage that it works for all cases, not only for a part of them.

davidde commented 4 years ago

There is definitely a trade-off; read the linked PR.

bxabi commented 4 years ago

Interesting. I wonder if maybe subliminal solved this in the meanwhile. Anyhow, I will keep using the full path as for my few tests that was better (I'm a new user), and see if any example comes up where the subtitle is not found, then I can check with just the filename.

davidde commented 4 years ago

Great idea, keep me updated! If after a few months you still feel the full path is better, maybe we can revisit this.