mov-cli / mov-cli

Watch everything from your terminal.
https://mov-cli.github.io/
MIT License
411 stars 36 forks source link

[FEATURE] Partial subtitles support #249

Closed shinysocks closed 1 month ago

shinysocks commented 2 months ago

implements: #79

shinysocks commented 2 months ago

api.opensubtitles.com docs

shinysocks commented 2 months ago

check on discord, this is not integrated!!

THEGOLDENPRO commented 2 months ago

check on discord, this is not integrated!!

ah, my bad still a draft. I'm going to make some corrections however.

shinysocks commented 2 months ago

sounds good, otherwise lemme know if there's anything I can change on my end

shinysocks commented 2 months ago

thanks for your changes! any ideas on how to set it up with the rest of the codebase?

THEGOLDENPRO commented 2 months ago

thanks for your changes! any ideas on how to set it up with the rest of the codebase?

yeah, I'm going to look into it. :+1: First I need to get an API key and test this.

THEGOLDENPRO commented 2 months ago

how is the password not the same when I copy and pasted it from my password manager :sob: WTF

Screenshot_20240315_205822

THEGOLDENPRO commented 2 months ago

I made a little test suite for this. You can run it with this command from the root directory:

python scripts/test_subtitles.py
THEGOLDENPRO commented 2 months ago

I'll work more on this another day. Thanks for your contributions.

shinysocks commented 2 months ago

looks awesome, i'd love to contribute more

THEGOLDENPRO commented 2 months ago

@Poseidon444 @r3tr0ananas Also can we remove Codacy, it's bugging me lmao. image

I think if we want proper linting and type checking we should use something like ruff (which we already have). It's a hell of a lot faster and more accurate and if you want ruff to be more strict feel free to modify its toml file. Up to you guys are the end of the day though.

THEGOLDENPRO commented 2 months ago

looks awesome, i'd love to contribute more

feel free 🙏

r3tr0ananas commented 2 months ago

@Poseidon444 @r3tr0ananas Also can we remove Codacy, it's bugging me lmao.

Yeah, we can.

shinysocks commented 2 months ago

I still got time today to get this working if you wanna explain where I should call the subtitle methods to integrate it, I just don't really understand the way mov-cli works..

THEGOLDENPRO commented 2 months ago

mov-cli v4 is plugin based meaning most likely the subtitles class will be called from the scraper in the plugin itself rather than mov-cli although I am still uncertain.

I'm not too good at explaining things but I'll try.

Here's a brief path of how mov-cli operates with plugins:

mov-cli -s films {query}

Results in these operations:

cli module --> plugin ("films" in this case) --> plugin's scraper --> Scraper.search(query) --> prompt results to user --> Scraper.scrape_metadata_episdoes(result) --> prompt episodes to user --> Scraper.scrape(result, episode) --> Player.play(media)

I'm still trying to come up with a good way to implement subtitles, I think there will be a bunch of changes that will have to be done with the codebase before subtitles can be implemented.

shinysocks commented 2 months ago

okay thank you for the explaination, I'll try to use the template stuff that @EbanNox sent in discord and put the subtitle functionality into some sort of plugin in the mean time

THEGOLDENPRO commented 2 months ago

Just gonna dump my current thoughts here publically so I don't end up forgetting.

It should be the plugin's job to retrieve subtitles and pass it to the Media class, as if we do it natively (outside of the plugin's scraper) we will only have the user's query to work with in regards of determining which subtitle is actually the correct one for that media. The scraper can do a better job at this as it has the entire source to work with.

The subtitles url returned by the Subtitles class can be passed to the subtitles parameter in the Media class that the player will then handle. I say "handle" because with MPV you can pass the url as a subtitles file no problem and it just works but for VLC you will have to download the subs first then pass it; that can be done very easily from the VLC player class however.

Okay but how about if you don't want subtitles for the media you watching? This is where we must introduce a cli option like --subs or --subtitles. These will be boolean flags that you set whenever you want to pull subtitles. Although now we have another issue, remember that we gave the scraper control over subtitles. Now we'll need to tell the scraper when to pull subtitles and when to not.

To do this we can introduce a new parameter to Scraper.scrape() like scrape_subtitles:

def scrape(self, metadata: Metadata, episode: Optional[EpisodeSelector] = None, scrape_subtitles: bool = False) -> Series | Movie:
    ...

Togther with the command options --subs and --subtitles we must also introduce a toml key where if the default key is set to true subtitles will be pulled by default, something like so:

[mov-cli.subtitles]
default = false
open_subtitles_key = "{your-key}"
THEGOLDENPRO commented 1 month ago

Now we'll need to tell the scraper when to pull subtitles and when to not.

To do this we can introduce a new parameter to Scraper.scrape() like scrape_subtitles:

def scrape(self, metadata: Metadata, episode: Optional[EpisodeSelector] = None, scrape_subtitles: bool = False) -> Series | Movie:
    ...

🎉 This pull request is what is going to make this possible: #261

EbanNox commented 1 month ago

Just gonna dump my current thoughts here publically so I don't end up forgetting.

It should be the plugin's job to retrieve subtitles and pass it to the Media class, as if we do it natively (outside of the plugin's scraper) we will only have the user's query to work with in regards of determining which subtitle is actually the correct one for that media. The scraper can do a better job at this as it has the entire source to work with.

The subtitles url returned by the Subtitles class can be passed to the subtitles parameter in the Media class that the player will then handle. I say "handle" because with MPV you can pass the url as a subtitles file no problem and it just works but for VLC you will have to download the subs first then pass it; that can be done very easily from the VLC player class however.

Okay but how about if you don't want subtitles for the media you watching? This is where we must introduce a cli option like --subs or --subtitles. These will be boolean flags that you set whenever you want to pull subtitles. Although now we have another issue, remember that we gave the scraper control over subtitles. Now we'll need to tell the scraper when to pull subtitles and when to not.

To do this we can introduce a new parameter to Scraper.scrape() like scrape_subtitles:

def scrape(self, metadata: Metadata, episode: Optional[EpisodeSelector] = None, scrape_subtitles: bool = False) -> Series | Movie:
    ...

Togther with the command options --subs and --subtitles we must also introduce a toml key where if the default key is set to true subtitles will be pulled by default, something like so:

[mov-cli.subtitles]
default = false
open_subtitles_key = "{your-key}"

I my self think of it like this... have the sub plugin behave like a scraper that just passes on to another plugin scraper if called...

Like:

Mov-cli -s sub.films.vadapav "eng" "spider man" -p mpv etc.... [Mov-cli--> calls sub scraper(fetches urls and then calls vadapav scraper)--> vadapav(fetches media and passes on to player methos) --> play.media(having the sub and media mixed together and end up on the player)

But as this is having the -s option used in the example, I think the same method can be devised when iplmlementing plugin passing new arg options to mov-cli it self. I'm just gonna leave this here as a mind map for the end result .

THEGOLDENPRO commented 1 month ago

I don't think this subtitles pr will do. Sadly if you don't get subs directly from the providers you will either have no subtitles or incorrect and delayed subtitles. Open Subtitles also has heavy rate limits for non developers so effecting basically every user.

I don't think this is the implementation I want to encourage in mov-cli plugins as it will lead to the problems I listed above.

Subtitles should be grabbed directly from the media provider itself, just like how the vadapav scraper does it.

I'll merge a few things from this pr though like the player subtitle args. Thanks @shinysocks for your contribution.