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

bugfix for ffmpegCommandRemoveStreamByProperty removes all streams #688

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

rgreen83
Copy link

@rgreen83 rgreen83 commented Jul 30, 2024

Used UnknownWitcher's code from #576

I'm not much of a coder so i used a web ai to convert the code to typescript and then followed the standard plugin compiling rules. passed linting and tests. Have tested multi value includes and not includes myself and found it does fix the issue and no longer removes streams it shouldn't.

Should resolve #576 and resolve #562

Also added some tooltip info to instruct on proper handling of mp4 subs to close #682 and related. I think this is a more precise solution than running Tdarr_Plugin_MC93_Migz1Remux with force_conform option as the job log tooltip suggests.

example of log output from my testing on the same file with includes and not includes
image
image

@HaveAGitGat
Copy link
Owner

Thanks, haven't merged this as changing the variable to valuesToCompare will break/give different behaviour for people already using this plugin. Likewise removing the for loop logic will give different behaviour as prop.includes(val) is very different from valuesToCompare.includes(prop). With former you can have e.g. 'mov,mp4,m4a,3gp,3g2,mj2'.includes('mov') (true) and it will work as planned but ['mov','xxx'].includes('mov,mp4,m4a,3gp,3g2,mj2') (false) will work very differently.

revert back to original naming convention per HGG
fresh start on comparison logic, tested for corner cases with invalid inputs per HGG
exclude stream 0 since this is the video stream, otherwise not_includes will remove it unless video codec specified
change match to be exact to avoid issues such as eac3 getting removed for ac3 match
change not_includes to compare against full array instead of individually to fix bug that caused everything to get removed if using multiple values
@rgreen83
Copy link
Author

rgreen83 commented Nov 7, 2024

Updated to address issues, request review @HaveAGitGat
New version # to avoid conflict
revert back to original naming convention per HGG
fresh start on comparison logic, tested for corner cases with invalid inputs per HGG
exclude stream 0 since this is the video stream, otherwise not_includes will remove it unless video codec specified
change match to be exact to avoid issues such as eac3 getting removed for ac3 match
change not_includes to compare against full array instead of individually to fix bug that caused everything to get removed if using multiple values
also sorry im terrible at git
image
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants