Maybe this should be considered more than a minor bug. This prevents
anyone from using normalize-mp3 without changing the code or using
undocumented tokens in the value of the --mp3encode command-line
argument.
Simply using the --mp3encode argument doesn't work because it replaces
the entire command line for the MP3 encoder, therefore removing the
tokens used for the wav and mp3 file (%w & %m).
For example, if I use
normalize-mp3 --mp3encode=lame *.mp3
I get the usage statement from lame because it was called without any
command line arguments. It only works if I use
normalize-mp3 --mp3encode="lame -quiet %w %m" *.mp3
Here are the important details of the problem with line numbers:
30: $MP3ENCODE = " -quiet %w %m";
...
342: find_mp3encode unless ($MP3ENCODE);
Line 342 doesn't do anything because, at this point, the $MP3ENCODE
variable has a value even if there is no encoder specified.
430: } elsif ($_ eq "--mp3encode") {
431: if ($#ARGV < 1) { print "$progname: option $_ requires an
argument\n"; exit 1; }
432: $MP3ENCODE = $ARGV[1];
433: shift; shift; next ARG_LOOP;
434: } elsif (/^--mp3encode=/) {
435: ($MP3ENCODE = $ARGV[0]) =~ s/^.*?=//;
436: shift; next ARG_LOOP;
Lines 430 - 436 show the problem. If we use --mp3encode, this
replaces the entire value of the $MP3ENCODE variable, including the
arguments.
606: $encoder = $MP3ENCODE;
...
622: @encode_args = split(/\s+/, $encoder);
623: for (@encode_args) {
624: s/^\%w$/$tmp_file/;
625: s/^\%m$/$output_file/;
626: s/^\%b$/$bitrate/;
627: }
In line 606 we set the value of $encoder to the value of the
$MP3ENCODE variable and in 622 - 627 we replace those tokens and
assign them to the @encode_args array.
In spite of the name of the variable, it looks like @encode_args is
intended to include the command as well as the arguments.
797: $ret = system(@encode_args);
798: if ($verbose < 2) {
799: close(STDOUT);
800: open(STDOUT, ">&OLDOUT");
801: }
802: $ret == 0 || die "Error encoding, stopped";
In lines 797 - 802 we see the failure. system tries to execute the
list contained in @encode_args.
If we don't specify the --mp3encode argument, it contains only the
arguments: "-quiet", the name of the wav file and the name of the mp3
file.
If we do specify the --mp3encode argument, such as "--mp3encode=lame",
it contains only that value: "lame".
If we specify a fully-tokenized argument, such as "--mp3encode='lame
-quiet %w %m'", it'll work.
I'd recommend putting lame into the script and listing it as a
requirement along with mpg123, oggdec/oggenc, and flac, which are
already hard-coded. Then remove the --mp3encode, --mp3decode,
--oggencode, --oggdecode command-line arguments. They can't be used
without quoting the argument and including the undocumented tokens
anyway.
If the ability to choose the encoders/decoders from the command-line
is considered an important feature, it could be done by separating the
encoder and the arguments into two different variables. Then the
checks called from lines 341 - 348 would have some significance. But
then the tokens should really be documented.
Thank you.
--
-- Vince
--
To UNSUBSCRIBE, email to [email protected]
with a subject of "unsubscribe". Trouble? Contact [email protected]
