slhck / ffmpeg-normalize

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

Allow multiple `-e` flags #188

Closed sdfg2 closed 2 years ago

sdfg2 commented 2 years ago

:warning: Please read this carefully and edit the example responses! If you do not fill out this information, your feature request may be closed without comment.

Is your feature request related to a problem? Please describe. I would like to build the command programmatically in a bash script. Part of that is constructing the extra options to pass directly to ffmpeg, the -e options. Currently if there is more than one -e, only the last one is used.

Describe the solution you'd like Allow for multiple -e options. I don't think ffmpeg needs any particular order.

Describe alternatives you've considered

Additional context

slhck commented 2 years ago

This is already possible. You can pass as many parameters as you want to the -e or -ei option.

For instance -e="-foo bar -baz bar".

slhck commented 2 years ago

I updated the README to make this aspect more clear.

sdfg2 commented 2 years ago

Yes, I'm asking for -e="-foo bar" and then later -e="-baz bar"

slhck commented 2 years ago

This sounds like an XY problem. Why can't you construct one -e "-foo bar -baz bar" option? You could collect the extra options in a Bash array first and later create one string to pass to -e.

sdfg2 commented 2 years ago

My bad! I forgot to write up the alternatives section. I obviously hadn't had my cup of tea when I wrote it.

It's not an XY problem, I have a work around, it was a QOL feature request.

I am writing a wrapper script to deal with album-level normalization to contribute to the wiki here. As such, I am taking all options to be passed to ffmpeg-normalize as an option in my script (in exactly the way you provide -e for ffmpeg itself). However, some of my script's options require adding extra parameters to ffmpeg-normalize -e, specifically if I want to retain the ORIGINALDATE of a release instead of the DATE (this works around one of Lidarr's issues).

So, the alternative I'm using is with sed.

SETTINGS="-vn -c:a libopus -b:a 75k -ar 48000 -ext opus -e=\"-vbr on -compression_level 10\""
date="1979" # as an example, I actually extract this from the metadata
finalsettings=$(echo "$SETTINGS" | sed -E -n 's|(^.*)-e=\"(.*)\"|\1-e=\"\2 -metadata:s:a:0 DATE=\"'$date'\"\"|p')

Of course the next issue is ffmpeg-normalize taking this string of settings as an explicit string argument, that's what I'm working on today!

slhck commented 2 years ago

I still believe you're going about this in a rather convoluted way. Have a look at this: https://unix.stackexchange.com/questions/473241/build-a-command-dynamically — you could collect your various extra options in an array and add them dynamically if needed. At the end, you render a final option.

#!/usr/bin/env bash

extraOpts=(
  -vbr on
  -compression_level 10 
)

# dynamically add another option
extraOpts+=(-metadata:s:a:0 DATE=10)

echo ffmpeg-normalize -e=\""${extraOpts[@]}"\"

This will print:

ffmpeg-normalize input.mp4 -e="-vbr on -compression_level 10 -metadata:s:a:0 DATE=10"
sdfg2 commented 2 years ago

Yeah, it's convoluted at the moment, but it's both a learning experience, and iterative. I doubt ffmpeg-normalize was as elegant as it is with the first commit!

Yes, passing it as an array would be good, but it is not just the -e options that get passed to ffmpeg-normalize.

usage: normmusic [-hvpay] [-t TARGET_LOUDNESS] [ -e 'OPTIONS' ]
                    -i INPUT_DIRECTORY -o OUTPUT_DIRECTORY

options:
  -h, --help            show this help message and exit
  -v                    verbose output
  -p                    show what would be carried out
  -a                    normalize to average album level. Forces -nt rms for
                        ffmpeg-normalize
  -y                    use original release year for albums
  -i PATH               base path for input files (REQUIRED)
  -o PATH               base path for output files (REQUIRED)
  -t NUMBER             target dB level (default -23)
  -e 'OPTIONS'          a single-quoted list of options to pass through to
                        ffmpeg-normalize

example:

  normmusic -vp -t 23 -i /music/reference -o /music/normalized \
  -e '-f -vn -c:a libopus -b:a 75k -ar 48000 \
  -ext opus -e="-vbr on -compression_level 10"'

The script additionally respects environment variables:

  - Lidarr_AddedTrackPaths
        When called by Lidarr as a custom script (On Download/Upgrade) the
        script gets called in an automated way.  LIDARR_OUTPUTDIR,
        LIDARR_SETTINGS, and LIDARR_TARGET are used.

        Edit this script to change these options.

Author: Steve Gillespie
License: MIT

I initially thought I could output the string of ffmpeg-normalize options to ffmpeg-normalize, but I keep getting the error ffmpeg-normalize: error: argument -f/--force: ignored explicit argument ' -vn -c:a libopus -b:a 75k -ar 48000 -ext ogg -e="-vbr on -compression_level 10 -metadata:s:a:0 DATE="2001"" -nt rms -pr' no matter how I escape or quote things.

From that, I have researched and yes, putting all the options into an array would be the best way of doing it. I'm currently trying to figure out how to extract that from the arguments passed by the user. Once that is done then I can use those arrays to construct the call to ffmpeg-normalize.

EDIT: If I was just thinking about myself, I had this script working days ago, and I've already used an earlier iteration to normalize over 2000 albums. But I want to contribute to the project because it has proven so useful to me, and everyone has different options they want to use, which is why I'm trying to make it a proper wrapper script! :-)

slhck commented 2 years ago

As for dynamically creating ffmpeg-normalize options, you would have to follow the same approach and expand them as shown in the Stack Overflow example I've linked to.

That said, if I were at this point, I would probably write the wrapper in Python because even though I know Bash quite well, at some point every Bash script reaches a level of complexity that makes it a nighmare to maintain (which is the whole reason for this project being written in Python … it started out as a Bash script).

I made a mistake in the original expansion of the extra options array. Here is a complete example:

#!/usr/bin/env bash

ffOptions=(
  "$1"
  -f -v -n -d
)

extraOpts=(
  -vbr on
  -compression_level 10 
)

# dynamically add another option
extraOpts+=(-metadata:s:a:0 DATE=10)

ffmpeg-normalize "${ffOptions[@]}" "-e=${extraOpts[*]}"

Call it with ./script /path/to/file.wav.

sdfg2 commented 2 years ago
#!/bin/bash

SETTINGS="-vn -c:a libopus -ar 48000 -ext opus -e=\"-vbr on -compression_level 10\" -b:a 75k"
ALBUM=true
VERBOSE=true

date="1979"

oFIS="$IFS"; IFS="-" read -ra arr <<< "$SETTINGS"; IFS="$oIFS"
ffnopts=()
ffopts=()
ffstarted=
for i in "${arr[@]}"; do
    if [ -n "$i" ]; then
        if [ -n "$ffstarted" ]; then
            if [ "${i: -1}" = "\"" ] || [ "${i: -2}" = "\" " ]; then
                firstpass=("-${i//\"}")
                if [ "${firstpass: -1}" = " " ]; then
                    ffopts+=("${firstpass% *}")
                else
                    ffopts+=("$firstpass")
                fi
                ffstarted=""
            else
                if [ "${i: -1}" = " " ]; then
                    ffopts+=("-${i% *}")
                else
                    ffopts+=("-$i")
                fi
            fi
        elif [ "$i" = "e=\"" ]; then
            ffstarted="true"
        else
            if [ "${i: -1}" = " " ]; then
                ffnopts+=("-${i% *}")
            else
                ffnopts+=("-$i")
            fi
        fi
    fi
done

if [ "$ALBUM" = "true" ]; then ffnopts+=("-nt rms"); fi
if [ "$VERBOSE" = "true" ]; then ffnopts+=("-pr"); fi
ffopts+=("-metadata:s:a:0 DATE=\"$date\"")

echo "ffopts"
for i in "${ffopts[@]}"; do
    echo "|$i|"
done

echo ""
echo "ffnopts"

for i in "${ffnopts[@]}"; do
    echo "|$i|"
done

echo ffmpeg-normalize "${ffnopts[@]}" "-e=\"${ffopts[@]}\""
ffmpeg-normalize "${ffnopts[@]}" "-e=\"${ffopts[@]}\""

Results in:

sdfg@heracles /store $ ./settings_array_test 
ffopts
|-vbr on|
|-compression_level 10|
|-metadata:s:a:0 DATE="1979"|

ffnopts
|-vn|
|-c:a libopus|
|-ar 48000|
|-ext opus|
|-b:a 75k|
|-nt rms|
|-pr|
ffmpeg-normalize -vn -c:a libopus -ar 48000 -ext opus -b:a 75k -nt rms -pr -e="-vbr on -compression_level 10 -metadata:s:a:0 DATE="1979""
usage: ffmpeg-normalize [-h] [-o OUTPUT [OUTPUT ...]] [-of OUTPUT_FOLDER] [-f] [-d] [-v] [-q] [-n] [-pr] [--version] [-nt {ebu,rms,peak}] [-t TARGET_LEVEL] [-p] [-lrt LOUDNESS_RANGE_TARGET] [--keep-loudness-range-target] [-tp TRUE_PEAK] [--offset OFFSET] [--dual-mono] [--dynamic] [-c:a AUDIO_CODEC] [-b:a AUDIO_BITRATE] [-ar SAMPLE_RATE] [-koa] [-prf PRE_FILTER] [-pof POST_FILTER] [-vn] [-c:v VIDEO_CODEC] [-sn] [-mn] [-cn]
                        [-ei EXTRA_INPUT_OPTIONS] [-e EXTRA_OUTPUT_OPTIONS] [-ofmt OUTPUT_FORMAT] [-ext EXTENSION]
                        input [input ...]
ffmpeg-normalize: error: argument -t/--target-level: invalid float value: ' rms'

And I have absolutely no idea how this is happening. I was getting this error when I was just using a string instead of an array, but thought it was something to do with the fact that I was using a string, and not an array. Now, I don't know. It's like it's taking -nt as the flag -n and the flag -t together, and -t is obviously expecting a value. Any insight?

slhck commented 2 years ago

Not on a computer now, but instead of

ffnopts+=("-nt rms");

You must do:

ffnopts+=(-nt rms);

Don't use quotes around pairs of arguments/values. Each array entry must be separate. If you put them in quotes, they will later be expanded to one single argument, which likely makes the parsing fail.

Your echo should output "-nt" and "rms" on separate lines.

The same goes for the other arguments.

sdfg2 commented 2 years ago

Ahh, so it is to do with strings. That worked for that bit. Now I guess I have to de-string all the other things.

sdfg2 commented 2 years ago

Thanks for your help!

I see why I was always only ever half way there, before. For those who might find this in the future:

The ffmpeg-normalize parameters need to be not strings, as @slhck described above. The -e options that ffmpeg-normalize sends to ffmpeg, however, needs to be a string.

#!/bin/bash

SETTINGS="-vn -c:a libopus -ar 48000 -ext opus -e=\"-vbr on -compression_level 10\" -b:a 75k"
ALBUM=true
VERBOSE=true

date="1979"

# Split by quotes, which will separate the ffmpeg options from the ffmpeg-normalize options
oFIS="$IFS"; IFS="\"" read -ra preffopts <<< "$SETTINGS"; IFS="$oIFS"

# Split the first part (ffmpeg-normalize options) by whitespace
oFIS="$IFS"; IFS=" " read -ra ffnopts <<< "${preffopts[0]}"; IFS="$oIFS"
# Remove the last element, which will be -e=
unset 'ffnopts[-1]'
# Save the ffmpeg options to a string
ffopts="${preffopts[1]}"
# Split the third part (ffmpeg-normalize options) by whitespace
oFIS="$IFS"; IFS=" " read -ra ffnopts2 <<< "${preffopts[2]}"; IFS="$oIFS"

# Add the elements of the third part to the first part
for i in "${ffnopts2[@]}"; do
    ffnopts+=($i)
done

# ffmpeg-normalize options are added to the array of parameters
if [ "$ALBUM" = "true" ]; then ffnopts+=(-nt rms); fi
if [ "$VERBOSE" = "true" ]; then ffnopts+=(-pr); fi
# ffmpeg options are concatenated
final_ffopts="$ffopts -metadata:s:a:0 DATE=\"$date\""

echo "ffopts"
for i in "${ffopts[@]}"; do
    echo "$i"
done

echo ""
echo "ffnopts"

for i in "${ffnopts[@]}"; do
    echo "$i"
done

echo ffmpeg-normalize ${ffnopts[@]} -e=\"$final_ffopts\"
ffmpeg-normalize ${ffnopts[@]} -e=\"$final_ffopts\"

The way @slhck suggested was to create an array for both sets of options, and using them in different ways at the end

"${ffnopts[@]}" for the ffmpeg-normalize parameters spits out each non-string. "-e=${extraOpts[*]}" for the ffmpeg parameters concatenates all elements of the array together. I didn't bother with an intermediary array in the end, I just used string concatenation.

slhck commented 2 years ago

Actually, everything you're using here is "strings" — it's just a matter of word splitting, which is very important in Bash.

By quoting something you are basically gluing two words together and prevent the shell from splitting it based on spaces.

Normally you want each word separated by a space … the reason it gets complicated is that the -e option requires all its arguments to be just one “string”, i.e., everything double-quoted, so that it can be passed as one string to Python's argument parser. I later split everything again from within Python, to construct the ffmpeg command.

sdfg2 commented 2 years ago

Well, spoke too soon. I must still be doing something wrong in the production code.

SETTINGS="-vn -c:a libopus -b:a 75k -ar 48000 -ext opus -e=\"-vbr on -compression_level 10\""

### Settings extraction
# Split by quotes, which will separate the ffmpeg options from the ffmpeg-normalize options
oFIS="$IFS"; IFS="\"" read -ra preffopts <<< "$SETTINGS"; IFS="$oIFS"

# Split the first part (ffmpeg-normalize options) by whitespace
oFIS="$IFS"; IFS=" " read -ra ffnopts <<< "${preffopts[0]}"; IFS="$oIFS"
# Remove the last element, which will be -e=
unset 'ffnopts[-1]'
ffopts="${preffopts[1]}"
# Split the third part (ffmpeg-normalize options) by whitespace
oFIS="$IFS"; IFS=" " read -ra ffnopts2 <<< "${preffopts[2]}"; IFS="$oIFS"
# Add the elements of the third part to the first part
for i in "${ffnopts2[@]}"; do
    ffnopts+=($i)
done

[...]

ffmpeg-normalize "${file}" -nt rms -t "${target}" -of "$outputpath" ${ffnopts[@]} -e=\"${ffopts}\"

Results:

ERROR: Error while running command /usr/bin/ffmpeg -y -nostdin -i './Incubus - If Not Now, When! - 11 - Tomorrow’s Food (Incubus).flac' -filter_complex '[0:0]volume=-12.423306dB[norm0]' -map_metadata 0 -map_metadata:s:a:0 0:s:a:0 -map_chapters 0 -map '[norm0]' -c:a libopus -b:a 75k -ar 48000 -c:s copy '-vbr on -compression_level 10' /tmp/76lylhhz.opus! Error: Error running command ['/usr/bin/ffmpeg', '-y', '-nostdin', '-i', './Incubus - If Not Now, When! - 11 - Tomorrow’s Food (Incubus).flac', '-filter_complex', '[0:0]volume=-12.423306dB[norm0]', '-map_metadata', '0', '-map_metadata:s:a:0', '0:s:a:0', '-map_chapters', '0', '-map', '[norm0]', '-c:a', 'libopus', '-b:a', '75k', '-ar', '48000', '-c:s', 'copy', '-vbr on -compression_level 10', '/tmp/76lylhhz.opus']: ffmpeg version n5.1.1 Copyright (c) 2000-2022 the FFmpeg developers                   
built with gcc 12.2.0 (GCC)
configuration: --prefix=/usr --disable-debug --disable-static --disable-stripping --enable-amf --enable-avisynth --enable-cuda-llvm --enable-lto --enable-fontconfig --enable-gmp --enable-gnutls --enable-gpl --enable-ladspa --enable-libaom --enable-libass --enable-libbluray --enable-libbs2b --enable-libdav1d --enable-libdrm --enable-libfreetype --enable-libfribidi --enable-libgsm --enable-libiec61883 --enable-libjack --enable-libmfx --enable-libmodplug --enable-libmp3lame --enable-libopencore_amrnb --enable-libopencore_amrwb --enable-libopenjpeg --enable-libopus --enable-libpulse --enable-librav1e --enable-librsvg --enable-libsoxr --enable-libspeex --enable-libsrt --enable-libssh --enable-libsvtav1 --enable-libtheora --enable-libv4l2 --enable-libvidstab --enable-libvmaf --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxcb --enable-libxml2 --enable-libxvid --enable-libzimg --enable-nvdec --enable-nvenc --enable-opencl --enable-opengl --enable-shared --enable-version3 --enable-vulkan
libavutil      57. 28.100 / 57. 28.100
libavcodec     59. 37.100 / 59. 37.100
libavformat    59. 27.100 / 59. 27.100
libavdevice    59.  7.100 / 59.  7.100
libavfilter     8. 44.100 /  8. 44.100
libswscale      6.  7.100 /  6.  7.100
libswresample   4.  7.100 /  4.  7.100
libpostproc    56.  6.100 / 56.  6.100
Unrecognized option 'vbr on -compression_level 10'.
Error splitting the argument list: Option not found

It seems -e=\"${ffopts}\" has ${ffopts} sent as a single quotes string?

slhck commented 2 years ago

Above I used:

e=\""${extraOpts[@]}"\"

But I'm not on a computer for another day or so. Maybe you can get help on superuser. com or unix.stackexchange.com?

sdfg2 commented 2 years ago

Yeah, I'm not using an array any more, just a string. There's no desperate time constraint to this, but once I get that done the script is good to go. I'm using it right now on my entire collection (I just built the command directly instead of using variables). I've even managed to connect it up to lidarr and beets so I've got a complete toolchain going.

slhck commented 2 years ago

If you could share your final script in https://github.com/slhck/ffmpeg-normalize/issues/145, that might help others!

sdfg2 commented 2 years ago

I will do, but it's not final. I still have this problem to overcome where ffmpeg-normalize is sending a string to ffmpeg instead of appending options.