Allow passing different arguments to different postprocessors

[pukkandan]

Please follow the guide below

• You will be asked some questions, please read them carefully and answer honestly
• Put an x into all the boxes [ ] relevant to your pull request (like that [x])
• Use Preview tab to see how your pull request will actually look like

---

Before submitting a pull request make sure you have:

• Searched the bugtracker for similar pull requests
• Read adding new extractor tutorial
• Read youtube-dl coding conventions and adjusted the code to meet them
• Covered the code with tests (note that PRs without tests will be REJECTED)
• Checked the code with flake8

In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:

• I am the original author of this code and I am willing to release it under Unlicense
• I am not the original author of this code but it is in public domain or released under Unlicense (provide reliable evidence)

What is the purpose of your pull request?

• Bug fix
• Improvement
• New extractor
• New feature

---

Description of your pull request and other information

Allow passing different arguments to different postprocessors like this:

--postprocessor-args "VideoConvertor:-c:v h264_nvenc -preset slow" --postprocessor-args "Metadata:-dn"

For backward compatibility, --postprocessor-args <args> is equivalent to:

--post-processor-args "default:<args>"

Closes #27593 #26863

[dstftw]

It's also could be beneficial to have common args for each post-processor entry in postprocessors list for providing arguments programmatically for post-processors known beforehand right away without bothering with postprocessor_args.

[pukkandan]

It's also could be beneficial to have common args for each post-processor entry in postprocessors list for providing arguments programmatically for post-processors known beforehand right away without bothering with postprocessor_args.

@dstftw I assume you are talking about API access. In that case, it would be beyond the scope of this PR

[remitamine]

i think that passing arguments to all post-processors should no longer be encouraged(with the default prefix), instead there should be a prefix for every supported program(ffmpeg, AtomicParsley, etc...) that will pass arguments to all post-processors using the same program.

[dstftw]

One may need to pass different args to different post-processors based on the same program.

[remitamine]

just to clarify my requested change, instead of introducing and encouraging users to use default prefix, i think it would be better to have ffmpeg, AtomicParsley, etc... prefixes.
it's only about the default prefix, it's not related to the invidual post-processor prefixes FFmpegExtractAudio, EmbedThumbnail, etc...

[dstftw]

Suggesting using ffmpeg prefix will have the same effect of argument clashes as we have now with applying args to every post-processor so I don't think it's a good idea. As for default, I agree that it should not be mentioned.

[remitamine]

Suggesting using ffmpeg prefix will have the same effect of argument clashes as we have now with applying args to every post-processor so I don't think it's a good idea.

this happened when there wasn't another way to specify arguments for a specific post-processor, so when both posibilities are available the user should:

• use general program prefix(ex: ffmpeg) when he intends to pass a specific parameter to all program invocation(ex: global options such as --log-level, -y, -n, ...) or when the post-processor is using multiple programs.
• use specifc post-processor when he want to pass the arguments only to a specific post-processor.

while the default prefix is more prone to causing problems(sending arguments to different programs will cause some of them to fail).

[pukkandan]

@remitamine I get your point and I have an idea how to implement this. But first we need to decide what should happen if the user passes both a ffmpeg and an FFmpegExtractAudio, for example. Should both set of options be used? or only ine should be prefered.

while the default prefix is more prone to causing problems(sending arguments to different programs will cause some of them to fail).

True, but sending the args to all PP is the current behaviour. So we must maintain an option to still do it. We could maybe discourage the user from using default by either:

1. Providing a warning when default is used
2. Leaving the default option undocumented
3. Removing the default prefix. Note that the default args can still be set by using the older --postprocessor-args ARGS syntax

imo, a simple warning is sufficient. But I will go with you guys' decision

[remitamine]

@remitamine I get your point and I have an idea how to implement this. But first we need to decide what should happen if the user passes both a ffmpeg and an FFmpegExtractAudio, for example. Should both set of options be used? or only ine should be prefered.

if it's going to be added, i think both arguments should be used, if a user uses both the EmbedThumbnail and ffmpeg prefix then both arguments should be passed to the ffmpeg command.
anyway, adding the program suffixes is just a suggestion.

while the default prefix is more prone to causing problems(sending arguments to different programs will cause some of them to fail).

True, but sending the args to all PP is the current behaviour. So we must maintain an option to still do it. We could maybe discourage the user from using default by either:

1. Providing a warning when default is used

2. Leaving the default option undocumented

3. Removing the default prefix. Note that the default args can still be set by using the older `--postprocessor-args ARGS` syntax

imo, a simple warning is sufficient. But I will go with you guys' decision

passing arguments to all post-processor is still allowed by not using any prefix and it will be kept only for backward compatibility(ex: users already having the option in their config file).
so i think option 3 would be best way, for future use the user will be encouraged to only use the specific prefixes and there won't be a default prefix to be used only the old way without specifiying a prefix(a warning here that this usage is deprecated and might be removed in the future can be usefull).

[pukkandan]

@remitamine I will implement your suggestions tonight and let you know

[pukkandan]

@remitamine I added the ability to use general program prefix like ffmpeg.

Additionally, you can also give the arguments much more specifically by using <PP>+<exe>:<args>. For example, you can give FFmpegExtractAudio+FFmpeg: and FFmpegExtractAudio+FFprobe: seperately. Suggestions are welcome for a different syntax to do this.

I have not yet updated the documentation to reflect these changes. Honestly, I am not entirely sure how to explain the feature in a simple way. Please give me suggestions if you have any

Also, I noticed that EmbedThumbnailPP (when using AtomicParsley), MetadataFromTitlePP and XAttrMetadataPP currently ignore --postprocessor-args.

• I have added support for AtomicParsley
• MetadataFromTitle does not invoke any external application, so the option does not make sense for it
• I dont understand XAttrMetadata enough to add support.

[remitamine]

as it now allowed to use the --postprocessor-args option multiple times, it would be better to see how to handle:

• the same option(--postprocessor-args) repeated multiple times with
  • the same prefix but with diffirent arguments(ex: EmbedThumbnail used multiple times)
  • with mutiple prefix levels(ex: prefix-less arguments with specfic arguments in the same command)
• multiple options passed to the post-processor programs(same option passed multiple times to ffmpeg or AtomicParsly with different values).

[pukkandan]

the same prefix but with diffirent arguments(ex: EmbedThumbnail used multiple times)

The same way all switches are handled. The latter arguments override the former

multiple options passed to the post-processor programs(same option passed multiple times to ffmpeg or AtomicParsly with different values).

If you pass it with a single switch, it is passed as-is. If you pass it seperately, the latter takes priority. For example,
--post-processor-args "ffmpeg:-map 0 -map 1" = -map 0 -map 1
--post-processor-args "ffmpeg:-map 0" --post-processor-args "ffmpeg:-map 1" = -map 1
This is consistent with how other options work.

with mutiple prefix levels(ex: prefix-less arguments with specfic arguments in the same command)

I don't understand what you mean. Could you provide example?

[remitamine]

I don't understand what you mean. Could you provide example?

--post-processor-args "ffmpeg:-map 0" --post-processor-args "-map 1"

[pukkandan]

--post-processor-args "ffmpeg:-map 0" --post-processor-args "-map 1"

hm.. I looked through the code again and this case is not properly handled. I will make a commit to fix it.

Here's how I think this should work: (--ppa = --postprocessor-args - I dont wanna type it every time)
Eg:

--ppa "EmbedThumbnail: et_args" --ppa "ffmpeg: ff_args" --ppa "EmbedThumbnail+ffprobe: probe_args" --ppa "def_args"

should result in:

• ffprobe in EmbedThumbnail gets probe_args
• ffmpeg in EmbedThumbnail gets et_args + ff_args
• Atomicparsley in Embedthumbnail gets et_args
• All other ffmpeg gets ff_args
• Any other PP gets def_args

Let me know if this a reasonable approach or changes need to be made

[remitamine]

Any other PP gets def_args

i think to preserve the current behaviour, the arguments would be passed to all PPs including the ones that are using specific prefixes.

[pukkandan]

i think to preserve the current behaviour, the arguments would be passed to all PPs including the ones that are using specific prefixes.

I don't think it should work like that. It shouldn't be a issue that the new syntax, when used, is overriding the old one. Also, this allows passing args easily to all PP except a specific one. Eg: --ppa "def_args" --ppa "ffprobe:"

[remitamine]

i think to preserve the current behaviour, the arguments would be passed to all PPs including the ones that are using specific prefixes.

I don't think it should work like that. It shouldn't be a issue that the new syntax, when used, is overriding the old one. Also, this allows passing args easily to all PP except a specific one. Eg: --ppa "def_args" --ppa "ffprobe:"

this inconsitent with the behaviour that you've described in you're example:

ffmpeg in EmbedThumbnail gets et_args + ff_args

where the ffmpeg command will combine the program specific args and the PP specific args, while in the example i described the options are not combined.

[pukkandan]

while in the example i described the options are not combined.

I assume you mean this:

--post-processor-args "ffmpeg:-map 0" --post-processor-args "-map 1"

There is no PP specific args given in that

---

this inconsitent with the behaviour that you've described in you're example:

No, it is exactly how I specified it

--ppa "EmbedThumbnail: et_args" --ppa "ffmpeg: ff_args" --ppa "EmbedThumbnail+ffprobe: probe_args" --ppa "def_args"
ffprobe in EmbedThumbnail gets probe_args
ffmpeg in EmbedThumbnail gets et_args + ff_args
Atomicparsley in Embedthumbnail gets et_args
All other ffmpeg gets ff_args
Any other PP gets def_args

If PP+exe args is specifically given, only it used.
Otherwise, PP and exe args are combined.
If none of the PP, exe or PP+exe are given, the default is used

Please explain what you find inconsistent with it

[remitamine]

as i think of the prefixes, i think of them as classes from the most specfic to the more generic:

• PP+exe
• PP
• exe
• ``(prefixless)

but i think you're considiring PP and exe in the same level.
in the way you're classifying the behviour would be consitent(but not backward compatible), but not in the way that i'm classifying the prefixes.
so it would come to:

• what classification is more correct.
• what behaviour would be better(overiding the generic args with the more specific or combine them) or to allow both of them.

[pukkandan]

but i think you're considering PP and exe in the same level.

Yes, there is no reasonable way to say which of PP/exe is more specific. AtomicParsley is more specific than EmbedThumbnail which in turn (arguably) has more specificity than ffmpeg. So considering them to be at the same level and combining them makes sense imo.

First I also thought of simply combining all the args. This is also the easiest to code. But it would make it very difficult for the user to override previous args.

Also, this allows passing args easily to all PP except a specific one. Eg: --ppa "def_args" --ppa "ffprobe:"

If you still think that is the way to go, I can modify the code to be so.

[remitamine]

Yes, there is no reasonable way to say which of PP/exe is more specific. AtomicParsley is more specific than EmbedThumbnail which in turn (arguably) has more specificity than ffmpeg. So considering them to be at the same level and combining them makes sense imo.

a program is not bound to only a specific PP, there nothing that prevents AtomicParsley from been used in another PP, so that's why i think the program prefix is more generic than PP prefix, while in the case of a PP using multiple programs, it is more specific then PP prefix and it's covered by PP+exe prefix.

[pukkandan]

a program is not bound to only a specific PP

I could argue the same in reverse. But that is beside the point. Honestly, what we are discussing is simply convention and I don't have any strong preference either way. As one of the core maintainers of this project, it is up to you to decide how it should behave. Any of the behaviors we discussed are easy to implement and not too different in usage. We simply need to decide on a convention.

Maybe discuss with @dstftw and finalize how the behaviour should be, and I will make it so.

If PP+exe args is specifically given, only it used.
Otherwise, PP and exe args are combined.
If none of the PP, exe or PP+exe are given, the default is used

For the time being, I am going to commit this behaviour

[remitamine]

I could argue the same in reverse. But that is beside the point. Honestly, what we are discussing is simply convention and I don't have any strong preference either way. As one of the core maintainers of this project, it is up to you to decide how it should behave. Any of the behaviors we discussed are easy to implement and not too different in usage. We simply need to decide on a convention.

as i'm not the one who started the review process, i'm not even trying to make a full review here(otherwise i would'nt use just comments and would've used the GitHub requested changes process), again, what i have provided untill now are just suggestions, there where some points that i wanted to be taken into considiration.
i think it's better to think about good design(by discussing) that wouldn't just work now, but try to cover future uses as well, generally when things are rushed anddo whatever that would work just now, it would bring more problems in the future.
anyway, i think the points that i wanted to raise can be found in my previous comments.