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

Add option to include language only when matching audio #2861

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

bjornorri
Copy link

Addresses feature request 541865.

When configuring a language profile, the user can now choose to include a language:

  • Always
  • When matching audio
  • When not matching audio

The change includes the following UI changes:

Before:
before

After:
after

@bjornorri
Copy link
Author

I was a bit unsure about the cutoff_met logic in movies.py and series.py.

It seemed to me that maybe there was a pre-existing bug there, but I just tried to match the existing logic. It might be worth double-checking that part 🙂

@morpheus65535
Copy link
Owner

I don't understand why you say there's a bug in cutoff logic. Can you explain a little more here or on Discord?

@bjornorri
Copy link
Author

I don't understand why you say there's a bug in cutoff logic. Can you explain a little more here or on Discord?

I might not be understanding the context correctly, but my assumption is we should only be setting cutoff_met = true if the language in the current iteration is desired. Looking at the following code (before my changes):

if cutoff_temp['audio_exclude'] == 'True' and \
                            any(x['code2'] == cutoff_temp['language'] for x in
                                get_audio_profile_languages(movie_subtitles.audio_language)):
                        cutoff_met = True

To me it reads as:
"If we want to exclude this language when it matches the audio and it does match the audio, then the cutoff is met."

Or in simpler terms:
"If this language is not desired, then the cutoff is met."

My hunch is that we'd like to flip the logic to be:
"If we want to exclude this language when it matches the audio and it does not match the audio, then the cutoff is met."

@morpheus65535
Copy link
Owner

I might not be understanding the context correctly, but my assumption is we should only be setting cutoff_met = true if the language in the current iteration is desired. Looking at the following code (before my changes):

if cutoff_temp['audio_exclude'] == 'True' and \
                            any(x['code2'] == cutoff_temp['language'] for x in
                                get_audio_profile_languages(movie_subtitles.audio_language)):
                        cutoff_met = True

This is how it works:

If we want to exclude the requested language in profile from search and any audio languages from video file match the specified language in profile, cutoff is met.

Does it make sense? I've wrote this a long time ago so it's not fresh in my memory!

@bjornorri
Copy link
Author

If we want to exclude the requested language in profile from search and any audio languages from video file match the specified language in profile, cutoff is met.

Ok, I think I understand. I previously assumed the cutoff only applied to subs, but in this case it is achieved via one of the audio tracks. Let me know if I'm still off here 😅

I changed my addition such that we don't meet the cutoff when an audio_only_include language doesn't match the audio.

I also added comments to clarify these cases.

@morpheus65535
Copy link
Owner

I may be dumb but I'm still not sure I understand your PR... You moved from and "exclude" logic to an "include" and it may be what confuse me. I'll try to take time to pull your PR later during the week to better grasp how it impact the logic.

@bjornorri
Copy link
Author

I may be dumb but I'm still not sure I understand your PR... You moved from and "exclude" logic to an "include" and it may be what confuse me. I'll try to take time to pull your PR later during the week to better grasp how it impact the logic.

No worries. I did flip the "Exclude" to "Include" in the UI because it felt more intuitive to me, but I'm open to flipping it back if you prefer. I just thought "Include when matching audio" was better than "Exclude when not matching audio", which felt like a double negative.

I'll describe my use case here in case it clarifies things:

  1. I always want subs. If the movie is in a language I understand, I still want subs in that language.
  2. I live in a German speaking country and I'm ok at it. If the movie is in German, I want the subs to train my German.
  3. If the movie is in Icelandic (my native language), I want the Icelandic subs. However, Icelandic subs are usually not available non-Icelandic movies. If I just add it to my profile, my wanted queue will fill up with Icelandic subs that will never be found.

So, I always want English subs, but I also want German subs if the movie is in German and Icelandic ones if its in Icelandic.

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

Successfully merging this pull request may close these issues.

2 participants