Implement --auto-lower-loudness-target
#273
Conversation
ffmpeg_normalize/__main__.py
Outdated
| @@ -249,6 +249,22 @@ def create_parser() -> argparse.ArgumentParser: | |||
| ), | |||
| ) | |||
|
|
|||
| group_ebu.add_argument( | |||
| "--safe-ebu", | |||
There was a problem hiding this comment.
I'm not sure if this is the most easily understandable option name. It implies that it is always the safest option … is that the case?
Maybe --auto-lower-loudness-target — and then we should just clarify in the README how to use these various options.
There was a problem hiding this comment.
Making overly verbose options does not make reading the manual unnecessary. It only makes writing and memorizing them harder. Especially when there is no bash completion for it.
There was a problem hiding this comment.
I understand that reasoning, however I'd like to strike a balance between usability and understandability here.
I personally very much prefer speaking arguments, even if they are longer, and maybe there will be a "one size fits all" option in the future that could carry this name? Or let's call it -sm / --safe-ebu-margin?
If completions are an issue, let's create bash/zsh completions and ship them with this repo. I think this should be easy to do …
There was a problem hiding this comment.
Added some basic completions here: d0d2741
| type=float, | ||
| help=textwrap.dedent( | ||
| """\ | ||
| Automatically lower EBU Integrated Loudness Target to prevent falling |
There was a problem hiding this comment.
And the option value is a relative one, right?
So it should say "automatically lower EBU Integrated Loudness Target by this value" or something.
Still thinking about the use case of explicitly providing a value here? Shouldn't this rather just be a boolean option and, if enabled, lower the loudness target automatically to the right value?
There was a problem hiding this comment.
Well, the second paragraph mentions "by the given amount" and the --help text shows that it expects a value --safe-ebu MARGIN so it seems clear enough?
I thought about the boolean option, but I couldn't decide what the right value would be. Sometimes a margin value of 0 works just fine, at others it falls back to dynamic again. Should we just set it to some arbitrary small value like 0.005 or something?
There was a problem hiding this comment.
I think having a default value that's small but prevents most cases of dynamic fallback is the better option!
In your tests, did 0.005 suffice? I think even 0.1 or something would not make a difference in terms of actual perceived loudness.
There was a problem hiding this comment.
A value of 0.005 seemed to work the few times I've used it but as you've said even a 0.1 is unnoticeable so better to go with that.
ahmetsait
force-pushed
the
safe-ebu
branch
2 times, most recently
from
168301c to
e785dea
Compare
ahmetsait
changed the title
--safe-ebu--auto-lower-loudness-target
|
Merged with added completions, thanks! |
No description provided.