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

simplify subtitle/audio script-opts #58

Merged
merged 1 commit into from
May 21, 2022

Conversation

CogentRedTester
Copy link
Owner

Simplifies the subtitle/audio extension script-opts.

Pinging @dyphire for thoughts.

@dyphire
Copy link
Contributor

dyphire commented May 21, 2022

No longer use the extension blacklist?

Although I was also skeptical about the necessity of the blacklist of subtitle extensions at the beginning, what is the reason for the cancellation of the blacklist of audio extensions?

@CogentRedTester
Copy link
Owner Author

It hasn't been cancelled. Instead of using a whitelist/blacklist we just make the script-opt the actual list of all valid extensions. So you can add any extension, or remove any of the default extensions. This wasn't possible for the main extension list because it is so long, but for subtitles and audio tracks there are so few extensions it can be done without much hassle. This way it's really clear what the defaults are as well.

@dyphire
Copy link
Contributor

dyphire commented May 21, 2022

It hasn't been cancelled. Instead of using a whitelist/blacklist we just make the script-opt the actual list of all valid extensions. So you can add any extension, or remove any of the default extensions. This wasn't possible for the main extension list because it is so long, but for subtitles and audio tracks there are so few extensions it can be done without much hassle. This way it's really clear what the defaults are as well.

You're right. Users can choose to use the automatic configuration matching folder to modify the relevant extended list, which is the same as the previous blacklist behavior.

@CogentRedTester
Copy link
Owner Author

Do you have any objections to me changing it to this? I think it's simpler and neater.

@dyphire
Copy link
Contributor

dyphire commented May 21, 2022

Do you have any objections to me changing it to this? I think it's simpler and neater.

I also think this change looks good.

@CogentRedTester CogentRedTester merged commit 0f3472e into master May 21, 2022
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