diff --git a/flac2mp3.pl b/flac2mp3.pl index 0d944b8..94ff8bd 100755 --- a/flac2mp3.pl +++ b/flac2mp3.pl @@ -33,6 +33,7 @@ use Scalar::Util qw/ looks_like_number /; use FreezeThaw qw/ cmpStr /; use Digest::MD5; +use Time::HiRes qw/ time /; # ------- User-config options start here -------- # Assume flac and lame programs are in the path. @@ -47,13 +48,22 @@ # c:/windows/system32/flac.exe # or # c:\\windows\\system32\\flac.exe -my $flaccmd = 'flac'; -my $lamecmd = 'lame'; + +my ($flaccmd, $lamecmd); +if ($^O eq 'MSWin32' or $^O eq 'MSWin64') { + # This is an example of typical Windows filepaths. + # Change them if necessary: + $flaccmd = q|C:\Program Files (x86)\FLAC\flac.exe|; + $lamecmd = q|C:\Program Files\LAME\lame.exe|; +} +else { + $flaccmd = 'flac'; + $lamecmd = 'lame'; +} # Modify lame options if required my @lameargs = qw ( --noreplaygain - --vbr-new -V 2 -h --nohist @@ -153,7 +163,7 @@ GetOptions( \%Options, "quiet!", "tagdiff", "debug!", "tagsonly!", "force!", "usage", "help", "version", "pretend", "skipfile!", "skipfilename=s", - "processes=i", "tagseparator=s", "lameargs=s", "copyfiles" + "processes=i", "tagseparator=s", "lameargs=s", "copyfiles", "delete" ); # info flag is the inverse of --quiet @@ -191,9 +201,13 @@ $cmdpath = which($cmd); } croak "$cmd not found" unless $cmdpath; - $Options{info} && msg("Using $cmd from: $cmdpath"); + msg_info("Using $cmd from: $cmdpath"); } +# Turn off unsyncing, due to broken implementation in +# some software (such as Logitech Media Server) +MP3::Tag->config(id3v23_unsync => 0); + # Convert directories to absolute paths $source_root = File::Spec->rel2abs($source_root); $target_root = File::Spec->rel2abs($target_root); @@ -201,50 +215,273 @@ die "Source directory not found: $source_root\n" unless -d $source_root; -# count all flac files in source_dir -# Display a progress report after each file, e.g. Processed 367/4394 files -# Possibly do some timing and add a Estimated Time Remaining -# Will need to only count files that are going to be processed. -# Hmmm could get complicated. - -$Options{info} - && msg( $pretendString . "Processing directory: $source_root" ); +msg_info( $pretendString . "Processing directory: $source_root" ); # Now look for files in the source dir # (following symlinks) +my %flac_mp3_files = get_all_paths('name', '.flac', $source_root, $target_root, '.mp3'); +my @flac_files = sort keys { %flac_mp3_files }; + +my $file_count = scalar @flac_files; +msg_info( "Found $file_count flac file" . ( $file_count > 1 ? 's' : '' . "\n" ) ); + +# If allowed, delete surplus files and folders from target directory, keeping +# it in perfect sync with (i.e. a mirror of) the source directory. +delete_excess_files_from_dest($source_root, $target_root) if ( $Options{delete} ) ; + +# Traverse all flac-files, but process only those that don't need transcoding. +# Save for later the name and size of each of the flac files that DO need transcoding. +my %flac_file_size = (); +my $total_file_size_to_transcode = 0; +my $cntr_processed = 0; +my $cntr_all; +my $t0 = 0; +foreach my $src_file (@flac_files) { + if ( $Options{force} || ( (my $result = path_and_conversion($src_file,0)) == 1 ) ) { + my $filesize = -s $src_file; # this file needs transcoding, get the size + # save size data in a hash + $flac_file_size{$src_file} = $filesize; + $total_file_size_to_transcode += $filesize; + } + else { + $cntr_processed++; + if ( scalar @$result) { + msg_info(""); + foreach my $string ( @$result ) { + msg_info($string); + } + } + }; + $cntr_all ++; + + # Show the progress every second + if ( $Options{info} && + ( ($Options{force}) || ((time - $t0) >= 1) || ($cntr_all==$file_count) ) ) { + $t0 = time; + print("\r" . $cntr_processed . " of " . $cntr_all . " flac files were processed without transcoding."); + }; + +} +msg_info(""); + + +my $files_to_trancode = scalar (keys %flac_file_size); +if ($files_to_trancode) { + my $t0 = time; # starting time for the transcoding part + my $files_transcoded_cntr = 0; + my $size_transcoded_so_far = 0; + + msg_info(""); + msg_info("The remaining $files_to_trancode files will be transcoded."); + msg_info("Total size of files to convert: " . + sprintf("%.1f", $total_file_size_to_transcode/(1024**2)) . " MB"); + # use parallel processing to launch multiple transcoding processes + msg_info("Using $Options{processes} transcoding processes.\n"); + my $pm = new Parallel::ForkManager($Options{processes}); + + $pm->run_on_finish( sub { + # This callback code is run after each transcode and outputs messages generated + # by the children, as well as overall progress info + # + # According to the Parallel::Forkmanager documentation, the structure of the + # input data @_ to this callback function is as follows: + # ($pid, $exit_code, $ident, $exit_signal, $core_dump, $data_structure_reference) + my $src = $_[2]; # (this is "$ident") + my $messages = $_[5]; # (this is "$data_structure_reference") + + # Update counter of transcoded files + $files_transcoded_cntr++; + + # display information about the transcoded/tagged file + foreach my $string ( @$messages ) { + msg_info($string); + } + # display updated progress info + msg_info("Processed " . + ($cntr_processed + $files_transcoded_cntr) . "/$file_count files."); + my $elapsedtime = time - $t0; + $size_transcoded_so_far += $flac_file_size{$src}; + + # After 20 seconds, we have enough data to provide a qualified guess + # about the estimated finish time. + if ($elapsedtime > 20) { + my $remains_to_be_transcoded + = $total_file_size_to_transcode - $size_transcoded_so_far; + my $transcoding_rate = $size_transcoded_so_far / $elapsedtime; + my $estim_finish_time + = localtime(time + $remains_to_be_transcoded / $transcoding_rate); + msg_info("Estimated finish time is $estim_finish_time."); + }; -my @flac_files = @{ find_files( $source_root, qr/\.flac$/i ) }; + msg_info(""); + }); -# Get directories from target_dir and put in an array -my ( $target_root_volume, $target_root_path, $target_root_file ) = File::Spec->splitpath( $target_root, 1 ); -my @target_root_elements = File::Spec->splitdir($target_root_path); + # Transcoding loop starts here + foreach my $src_file (sort keys %flac_file_size) { + $pm->start($src_file) and next; # forks here -# use parallel processing to launch multiple transcoding processes -msg("Using $Options{processes} transcoding processes.\n"); -my $pm = new Parallel::ForkManager($Options{processes}); -foreach my $src_file (@flac_files) { - $pm->start and next; # Forks and returns the pid for the child - path_and_conversion($src_file); - $pm->finish; # Terminates the child process + # transcode and generate messages with file info + my $messageref = path_and_conversion($src_file,1); + + # terminate child process, send messages to callback sub + $pm->finish(0, $messageref ); + } + $pm->wait_all_children; + +}; + +# If allowed, copy non-flac files to destination dirs +copy_non_flacs($source_root, $target_root) if ( $Options{copyfiles} ); + +1; +# ------------ Main program ends here -------------------------------------- + + +# ------------ Subroutines start here -------------------------------------- + +sub delete_excess_files_from_dest { + my ($source_root, $target_root) = @_; + + # Generate (source => target) hashes for the files found using + # each of the following combinations of root dirs and file suffixes + my %existing_target_mp3_files = get_all_paths('name', '.mp3', $target_root, $target_root, '.mp3'); + my %existing_source_mp3_files = get_all_paths('name', '.mp3', $source_root, $target_root, '.mp3'); + my %non_flac_files = get_all_paths('not_name', '.flac', $source_root, $target_root, ''); + my %existing_target_non_mp3_files = get_all_paths('not_name', '.mp3', $target_root, $target_root, ''); + + # 1. calculate what files to expect in directory after finished transcoding and copying + my @expected_transcoded_mp3s = keys { reverse %flac_mp3_files }; # expected mp3 files in target from transcoded flac files + my @expected_copied_files = keys { reverse %non_flac_files }; # expected files in target copied from non-flac files in source + my @expected_files = uniq(@expected_transcoded_mp3s, @expected_copied_files); # Join the arrays and remove any duplicates + + # 2. check what files are actually present + my @actual_mp3s = keys { reverse %existing_target_mp3_files }; # actual existing mp3 files in target + my @actual_non_mp3s = keys { reverse %existing_target_non_mp3_files }; # existing non-mp3 files in target + my @actual_files = (@actual_mp3s, @actual_non_mp3s); # Join the arrays (being mutually exclusive, there is no overlap) + + # 3. determine which files to remove from target directory tree + my @files_to_remove = single_difference(\@expected_files, \@actual_files); + + # 4. determine which subdirectories to remove from target directory tree + my @expected_subdirs_in_target = get_all_dirs($source_root,$target_root); + my @actual_subdirs_in_target = get_all_dirs($target_root,$target_root); + my @dirs_to_remove = single_difference(\@expected_subdirs_in_target, \@actual_subdirs_in_target); + + # 5. carry out the deletions + foreach my $file (@files_to_remove) { + $Options{pretend} || unlink $file or die "Unable to delete $file: $!"; + msg_info($pretendString . "Deleted \"$file\""); + } + foreach my $dir (reverse sort @dirs_to_remove) { + $Options{pretend} || File::Path->remove_tree($dir) or die "Unable to delete directory $dir: $!"; + msg_info($pretendString . "Deleted directory \"$dir\""); + } +} + +# Return all unique elements of input array @_ +sub uniq { + return sort keys %{{ map { $_ => 1 } @_ }} +}; + +# Acccept two arrays @A and @B as argument, return elements in @B that aren't in @A. +sub single_difference { + my ($A, $B) = @_; + + # build lookup table + my %seen = (); + my @bonly = (); + @seen{@$A} = (1) x @$A; + foreach my $item (@$B) { + push(@bonly, $item) unless $seen{$item}; + } + return sort @bonly; } -$pm->wait_all_children; +sub get_all_dirs { + my ($root, $new_root) = @_; + # we supply no suffix, so we search for directories (not files): + my @orig_dirs = @{ find_files_or_dirs($root) }; + + my @dirs = (); + foreach my $dir (@orig_dirs) { + # strip source root dir from path... + my $rel_path = File::Spec->abs2rel( $dir, $root ); + # then replace it with target root dir + push @dirs, File::Spec->rel2abs( $rel_path, $new_root ); + } + return sort @dirs; +} -if ( $Options{copyfiles} ) { - my @non_flac_files - = sort File::Find::Rule->file()->extras( { follow => 1 } )->not_name(qr/\.flac$/i) - ->in($source_root); +sub get_all_paths { + my ($rule, $suffix, $root, $new_root, $new_suffix) = @_; + my @orig_files = @{ find_files_or_dirs($root, $rule, $suffix) }; + + # Even if $root = $new_root, we need to do the following operations + # to get a consistent path format (otherwise problematic in e.g. MS Win) + # that is suitable for later string comparison: + my %paths = (); + foreach my $src (@orig_files) { + # Strip source root dir from file path + my $rel_path = File::Spec->abs2rel( $src, $root ); + # ... then replace it with target root dir and change file suffix. + ($paths{$src} = File::Spec->rel2abs( $rel_path, $new_root ) ) =~ s{$suffix$}{$new_suffix}xmsi; + } + return %paths +} + +sub find_files_or_dirs { + my $path = shift; + my $rule = shift; + my $suffix = shift; + + # If a matching rule and file suffix is defined we are looking for files, + # otherwise we are looking for directories. + my $found_list; + if (defined $rule && defined $suffix) { + $found_list = File::Find::Rule->file()->extras( { follow => 1 } )->$rule(qr{$suffix$}xmsi) + } + else { + $found_list = File::Find::Rule->directory->extras( { follow => 1 } ) + }; + + # skip any directories where a "skipfile" is found + my @found; + if ( $Options{skipfile} && ($path eq $source_root) ) { + my $skip_list = File::Find::Rule->directory->exec( + sub { + my ( $fname, $fpath, $frpath ) = @_; + if ( -f File::Spec->catdir( $frpath, $Options{skipfilename} ) ) { + return 1; + } + else { + return 0; + } + } + )->prune->discard; + @found = sort File::Find::Rule->or( $skip_list, $found_list )->in($path); + } + else { + @found = sort $found_list ->in($path); + } +return \@found; +} + +sub copy_non_flacs { + my ($source_root, $target_root) = @_; + + my %non_flac_files = get_all_paths('not_name', '.flac', $source_root, $target_root, ''); + my @non_flac_files = keys %non_flac_files; my $non_flac_file_count = scalar @non_flac_files; - $Options{info} && - msg( "Found $non_flac_file_count non-flac file" .( $non_flac_file_count != 1 ? 's' : '' . "\n" ) ); + msg_info( "Found $non_flac_file_count non-flac file" . + ( $non_flac_file_count != 1 ? 's' : '' . "\n" ) ); # Copy non-flac files from source to dest directories my $t0 = time; my $cntr_all = 0; my $cntr_copied = 0; foreach my $src_file (@non_flac_files) { - my ($dst_dir, $dst_file) = get_dest_file_path_non_flac($src_file); - # Flag which determines if file should be copied: + my $dst_file = $non_flac_files{$src_file}; + # Flag which determines if file should be copied: my $do_copy = 1; # Don't copy file if it already exists in dest directory and # has identical md5 to the source file @@ -256,11 +493,13 @@ }; } else { - # Create the destination directory if it - # doesn't already exist - mkpath($dst_dir) - or die "Can't create directory $dst_dir\n" - unless -d $dst_dir; + # Create the destination directory if it + # doesn't already exist + (undef, my $dst_dir) = + File::Basename::fileparse($dst_file); # retrieve directory name + unless ( $Options{pretend} || -d $dst_dir ) { + mkpath($dst_dir) or die "Can't create directory $dst_dir\n"; + } }; if ( $do_copy ) { unless ( $Options{pretend} ) { @@ -270,35 +509,14 @@ }; $cntr_all ++; # Show the progress every second - if ( ((time - $t0) >= 1) || ($cntr_all==$non_flac_file_count) ) { + if ( $Options{info} && + ( ((time - $t0) >= 1) || ($cntr_all==$non_flac_file_count) ) ) { $t0 = time; - print("\r" . $pretendString . $cntr_copied . " non-flac files of " . $cntr_all ." were copied to dest directories."); + print("\r" . $pretendString . $cntr_copied . + " non-flac files of " . $cntr_all ." were copied to dest directories."); }; }; - msg("\n"); # double line feed -}; - - -sub get_dest_file_path_non_flac { - my $source = shift; - - # remove $source_dir from front of $src_file - my $target = $source; - $target =~ s{\Q$source_root/\E}{}xms; - - # Get directories in target and put in an array - # Note: the filename is the source file name - my ( $target_volume, $target_path, $source_file ) = File::Spec->splitpath($target); - my @target_path_elements = File::Spec->splitdir($target_path); - - # Add the dst_dirs to the dst root and join back together - $target_path = File::Spec->catdir( @target_root_elements, @target_path_elements ); - - # Now join it all together to get the complete path of the dest_file - $target = File::Spec->catpath( $target_root_volume, $target_path, $source_file ); - my $target_dir = File::Spec->catpath( $target_root_volume, $target_path, '' ); - - return $target_dir,$target; + msg_info("\n"); # double line feed }; sub get_md5_of_non_flac_file { @@ -310,81 +528,69 @@ sub get_md5_of_non_flac_file { return $md5_code; }; -# use parallel processing to launch multiple transcoding processes sub path_and_conversion{ - my $source = shift; + # '$transcode_enabled = 0' means that we are only + # checking whether the file should be transcoded. If so, we + # return, and no further processing is taking place during this call. + # If the file is not to be transcoded, we stay and update tags if + # necessary. + + my $source = shift; + my $target = $flac_mp3_files{$source}; + my $transcode_enabled = shift; + my @messages = (); + my $t0; + my $elapsed_time = undef; - # remove $source_dir from front of $src_file - my $target = $source; - $target =~ s{\Q$source_root/\E}{}xms; - - # Get directories in target and put in an array - # Note: the filename is the source file name - my ( $target_volume, $target_path, $source_file ) = File::Spec->splitpath($target); - my @target_path_elements = File::Spec->splitdir($target_path); - - # Add the dst_dirs to the dst root and join back together - $target_path = File::Spec->catdir( @target_root_elements, @target_path_elements ); - # Add volume for OSes that require it (MSWin etc.) - $target_path = File::Spec->catpath( $target_root_volume, $target_path, '' ); - - # Get the basename of the dst file - my ( $target_base, $target_dir, $source_ext ) = fileparse( $source_file, qr{\Q.flac\E$}xmsi ); + $Options{debug} && msg("source: '$source'"); + $Options{debug} && msg("target: '$target'"); - # Now join it all together to get the complete path of the dest_file - $target = File::Spec->catpath( $target_volume, $target_path, $target_base . '.mp3' ); + # Step 1: get tags from flac file + my $source_tags = read_flac_tags($source); - convert_file( $source, $target ); -}; + # Step 2: hash to hold tags that will be updated + my $tags_to_update = preprocess_flac_tags( $source_tags ); -1; + # Step 3: Initialise file processing flags + my ($pflags, $mess) = examine_destfile_tags( $target, $tags_to_update ); + push @messages, @$mess; -sub find_files { - my $path = shift; - my $regex = shift; + if ( ( !$$pflags{exists} || $$pflags{md5} || $Options{force} ) + && !$Options{tagsonly} ) { - my @found_files; + # Return if this file would be transcoded + return 1 unless ($transcode_enabled); - my $found_list = File::Find::Rule->extras( { follow => 1 } )->name($regex); - if ( $Options{skipfile} ) { - my $skip_list = File::Find::Rule->directory->exec( - sub { - my ( $fname, $fpath, $frpath ) = @_; - if ( -f File::Spec->catdir( $frpath, $Options{skipfilename} ) ) { - return 1; - } - else { - return 0; - } - } - )->prune->discard; - @found_files = sort File::Find::Rule->or( $skip_list, $found_list )->in($path); - } - else { - - @found_files = sort $found_list ->in($path); - } + $t0 = time; + # Step 4: Transcode the file based on the processing flags + $mess = transcode_file( $source, $target, $pflags ); + push @messages, @$mess; + $elapsed_time = time - $t0; + }; - $Options{debug} && msg( Dumper(@found_files) ); + # Step 5: Write the tags based on the processing flags + $mess = write_tags( $target, $tags_to_update, $pflags ); + push @messages, @$mess; - if ( $Options{info} ) { - my $file_count = scalar @found_files; - msg( "Found $file_count flac file" . ( $file_count > 1 ? 's' : '' . "\n" ) ); - } + if (defined $elapsed_time) { + push @messages, sprintf("Conversion took %5.1f seconds.", $elapsed_time); + }; - return \@found_files; -} + return \@messages; +}; sub showusage { print <<"EOT"; -Usage: $0 [--pretend] [--quiet] [--debug] [--tagsonly] [--force] [--tagdiff] [--noskipfile] [--skipfilename=] [--lameargs='parameter-list'] --pretend Don't actually do anything +Usage: $0 [--pretend] [--quiet] [--debug] [--tagsonly] [--force] [--tagdiff] [--noskipfile] [--skipfilename=] [--lameargs='parameter-list'] + + --pretend Don't actually do anything --quiet Disable informational output to stdout --debug Enable debugging output. For developers only! --tagsonly Don't do any transcoding - just update tags --force Force transcoding and tag update even if not required --tagdiff Print source/dest tag values if different --lameargs='s' specify parameter(string) to be passed to the LAME Encoder - Default: "--noreplaygain --vbr-new -V 2 -h --nohist --quiet" + Default: "--noreplaygain -V 2 -h --nohist --quiet" --noskipfile Ignore any skip files --skipfilename Specify the name of the skip file. Default: flac2mp3.ignore @@ -395,35 +601,18 @@ sub showusage { same tag. Default: "/" --copyfiles Copy non-flac files to dest directories + --delete Delete surplus files and directories in destination, keeping in sync with source dir EOT exit 0; } -sub msg { - my $msg = shift; - print "$msg\n"; +sub msg { + print "@_\n" } -sub convert_file { - my ( $source, $target ) = @_; - - $Options{debug} && msg("source: '$source'"); - $Options{debug} && msg("target: '$target'"); - - # get tags from flac file - my $source_tags = read_flac_tags($source); - - # hash to hold tags that will be updated - my $tags_to_update = preprocess_flac_tags( $source_tags ); - - # Initialise file processing flags - my $pflags = examine_destfile_tags( $target, $tags_to_update ); - - # Transcode the file based on the processing flags - transcode_file( $source, $target, $pflags ); - - # Write the tags based on the processing flags - write_tags( $target, $tags_to_update, $pflags ); +sub msg_info { + # display only if "--quiet" option is not in use + $Options{info} && msg(@_) } sub read_flac_tags { @@ -512,6 +701,7 @@ sub examine_destfile_tags { my $destfilename = shift; my $frames_ref = shift; my %frames_to_update = %$frames_ref; # this is only to minimize changes + my @return_messages = (); # Initialise file processing flags my %pflags = ( @@ -621,11 +811,12 @@ sub examine_destfile_tags { if ( $dest_text ne $srcframe ) { $pflags{tags} = 1; if ( $Options{tagdiff} ) { - msg("frame: '$frame'"); - msg("srcframe value: '$srcframe'"); - msg("destframe value: '$dest_text'"); + push @return_messages, ( + "frame: '$frame'", + "srcframe value: '$srcframe'", + "destframe value: '$dest_text'" + ); } - } } } @@ -647,7 +838,7 @@ sub examine_destfile_tags { msg( Dumper \%frames_to_update ); } - return \%pflags; + return \%pflags, \@return_messages; } sub transcode_file { @@ -655,93 +846,91 @@ sub transcode_file { my $target = shift; my $pflags_ref = shift; my %pflags = %$pflags_ref; # this is only to minimize changes + my @return_messages = (); + + # Transcode to a temp file in the destdir. + # Rename the file if the conversion completes sucessfully + # This avoids leaving incomplete files in the destdir + # If we're "pretending", don't create a File::Temp object + my $tmpfilename; + my $tmpfh; + if ( $Options{pretend} ) { + $tmpfilename = $target; + } + else { + # retrieve destination directory name + (undef, my $dst_dir) = File::Basename::fileparse($target); - my ( $target_volume, $target_dir, $target_filename ) = File::Spec->splitpath($target); - my $dst_dir = File::Spec->catpath( $target_volume, $target_dir, '' ); - - if ( ( !$pflags{exists} || $pflags{md5} || $Options{force} ) - && !$Options{tagsonly} ) - { - - # Transcode to a temp file in the destdir. - # Rename the file if the conversion completes sucessfully - # This avoids leaving incomplete files in the destdir - # If we're "pretending", don't create a File::Temp object - my $tmpfilename; - my $tmpfh; - if ( $Options{pretend} ) { - $tmpfilename = $target; - } - else { - - # Create the destination directory if it - # doesn't already exist - unless (-d $dst_dir) { - # If necessary, allow a second check. Don't die just because the - # dir was created by another child (race condition): - mkpath($dst_dir) or (-d $dst_dir) - or die "Can't create directory $dst_dir\n"; - }; - $tmpfh = new File::Temp( - UNLINK => 1, - DIR => $dst_dir, - SUFFIX => '.tmp' - ); - $tmpfilename = $tmpfh->filename; - } - $Options{info} - && msg( $pretendString . "Transcoding \"$source\"" ); + # Create the destination directory if it + # doesn't already exist + unless (-d $dst_dir) { + # If necessary, allow a second check. Don't die just because the + # dir was created by another child (race condition): + mkpath($dst_dir) or (-d $dst_dir) + or die "Can't create directory $dst_dir\n"; + }; + $tmpfh = new File::Temp( + UNLINK => 1, + DIR => $dst_dir, + SUFFIX => '.tmp' + ); + $tmpfilename = $tmpfh->filename; + } + # Save message to be displayed on screen to the buffer + push @return_messages, $pretendString . "Transcoding \"$source\"" ; - my $convert_command = "\"$flaccmd\" @flacargs \"$source\"" . "| \"$lamecmd\" @lameargs - \"$tmpfilename\""; + my $convert_command = + "\"$flaccmd\" @flacargs \"$source\"" . "| \"$lamecmd\" @lameargs - \"$tmpfilename\""; - $Options{debug} && msg("transcode: $convert_command"); + $Options{debug} && msg("transcode: $convert_command"); - # Convert the file (unless we're pretending} - my $exit_value; - if ( !$Options{pretend} ) { - $exit_value = system($convert_command); - } - else { - $exit_value = 0; - } + # Convert the file (unless we're pretending} + my $exit_value; + if ( !$Options{pretend} ) { + $exit_value = system($convert_command); + } + else { + $exit_value = 0; + } - $Options{debug} - && msg("Exit value from convert command: $exit_value"); + $Options{debug} + && msg("Exit value from convert command: $exit_value"); - if ($exit_value) { - msg("$convert_command failed with exit code $exit_value"); + if ($exit_value) { + msg("$convert_command failed with exit code $exit_value"); - # delete the destfile if it exists - unlink $tmpfilename; + # delete the destfile if it exists + unlink $tmpfilename; - # should check exit status of this command + # should check exit status of this command + exit($exit_value); + } - exit($exit_value); - } + if ( !$Options{pretend} ) { - if ( !$Options{pretend} ) { + # If we get here, assume the conversion has succeeded + $tmpfh->unlink_on_destroy(0); + $tmpfh->close; + croak "Failed to rename '$tmpfilename' to '$target' $!" + unless rename( $tmpfilename, $target ); - # If we get here, assume the conversion has succeeded - $tmpfh->unlink_on_destroy(0); - $tmpfh->close; - croak "Failed to rename '$tmpfilename' to '$target' $!" - unless rename( $tmpfilename, $target ); + # the destfile now exists! + $pflags{exists} = 1; - # the destfile now exists! - $pflags{exists} = 1; + # and the tags need writing + $pflags{tags} = 1; + } - # and the tags need writing - $pflags{tags} = 1; - } - } if ( $Options{debug} ) { msg("pf_exists: $pflags{exists}"); msg("pf_tags: $pflags{tags}"); msg( "\$Options{pretend}: " . ( $Options{pretend} ? 'set' : 'not set' ) ); - } + } %$pflags_ref = %pflags; # this is only to minimize changes + + return \@return_messages; } sub write_tags { @@ -750,16 +939,13 @@ sub write_tags { my $pflags_ref = shift; my %frames_to_update = %$frames_ref; # this is only to minimize changes my %pflags = %$pflags_ref; # this is only to minimize changes + my @return_messages = (); # Write the tags - if ($pflags{exists} - && ( $pflags{tags} - || $Options{force} ) - ) - { + if ( $pflags{exists} && ( $pflags{tags} || $Options{force} ) ) { - $Options{info} - && msg( $pretendString . "Writing tags to \"$destfilename\"" ); + # save message to be displayed on screen + push @return_messages, $pretendString . "Writing tags to \"$destfilename\""; if ( !$Options{pretend} ) { my $mp3 = MP3::Tag->new($destfilename); @@ -822,6 +1008,7 @@ sub write_tags { # utime $srcstat->mtime, $srcstat->mtime, $destfilename; } } + return \@return_messages; } sub INT_Handler { @@ -847,8 +1034,7 @@ sub fixUpTrackNumber { $trackNum = sprintf( "%02u", $trackNum ); } else { - $Options{info} - && msg('TRACKNUMBER not numeric'); + msg_info('TRACKNUMBER not numeric'); } } return $trackNum; @@ -898,4 +1084,4 @@ sub picsToAPICframes { # vim:set softtabstop=4: # vim:set shiftwidth=4: -__END__ +__END__ \ No newline at end of file