search

slhck / ffmpeg-normalize

Audio Normalization for Python/ffmpeg
MIT License
1.28k stars 118 forks source link

Implement `--lower-only` #271

Closed ahmetsait closed 5 days ago

slhck commented 1 week ago

Thanks for the PR. I'll review the functionality later, but can you please undo the version change? This is done separately. Thanks.

ahmetsait commented 1 week ago

Looks like naively removing the filter causes a bunch of issues for parser code so it's gonna take a bit more time to flesh this PR out. In the meantime please mention if you see better ways to handle things in the code-base.

slhck commented 1 week ago

I think you will have to return the original input labels in the _get_audio_filter_cmd() method as output_labels, because the output mapping expects the -filter_complex to output the audio streams and the particular output labels used for the -map option.

Because later in the second pass we do:

        for ol in output_labels:
            cmd.extend(["-map", ol])

and if no normalization filter was added, these labels do not exist.

ahmetsait commented 1 week ago

@slhck Should be ready for review.

I opted for replacing loudnorm with a no-op acopy filter if input loudness is already lower than target.

slhck commented 1 week ago

Thanks! I see you changed a few other things related to parsing. On first sight it looks reasonable but I'll do a thorough test in the next few days. (The test suite is not good, coverage should be higher.)

slhck commented 5 days ago

In principle everything looks good and works as intended with respect to the --lower-only option, but there is one failing test when parsing the loudnorm output. Since you changed this part quite significantly, could you please check?

slhck commented 5 days ago

You used some typing syntax that does not work in Python 3.8. If you rebase onto master, it should work — I just removed 3.8 support because it's EOL anyway. Merged manually now since all tests pass, but I'll do some manual further tests before finally making a release later. Thanks for the contribution!

slhck commented 5 days ago

See commit 7730205a0c20a17508de9f1d3c2d54a1e1827453

slhck commented 5 days ago

Released as v1.30.0

ahmetsait commented 4 days ago

Awesome thanks! Any documentation on your release steps? Things like gitchangelog > CHANGELOG.md and commands to run for generating docs etc.

slhck commented 4 days ago

I used to have a script per repository and then eventually settled on a bash script that does this: https://github.com/slhck/dotfiles/blob/master/scripts%2Frelease-python.sh

It's not perfect, and I would like to migrate all of my projects away from setup.py eventually, using virtual environments, pyproject.toml with development dependencies, etc., but this gets the job done for now.