Fails to copy non-flac files
Closed this issue · 8 comments
When pointing flac2all at a directory that contains a mix of flac and mp3 files, the flac files are converted but the existing mp3 files are not copied.
$ flac2all mp3 ~/tmp/mix -o ~/tmp/mp3 -c
It looks like flac2all will only attempt to copy files destination file exists and the -f flag isn't used. The second condition makes sense, but the first does not. If the destination file already existed, I wouldn't need flac2all to copy it over!
Well, if the target already has an mp3 in there, it wouldn't convert it unless you run it with "-f".
I am trying to replicate this bug, but so far am having trouble seeing the problem you are. I do see a race condition, where flac2all will convert a file before it realises that it already exists in the source dir and tries to copy it over (at which point it detects the file already existing, and doesn't copy).
Is that what you are referring to?
For my use case, the output directory ~/tmp/mp3 is empty and the input directory ~/tmp/mix holds a mix of flac and mp3. When running the above command, the flac files are converted to mp3 and output to ~/tmp/mp3, as expected. However, none of the mp3 files are copied over.
If I'm reading the code correctly, all the copying stuff is between line 643 and line 651, with the actual copying happening on line 648. To reach that copy command on line 648, it looks like 5 conditions have to be met:
- The file must not end in flac (line 643)
- The copy option must be true (line 644)
- The output file must exist (line 646)
- The overwrite option must be false (line 646)
- The difference between the mtime on the source and output files must be > 1 (line 647)
If any of those conditions are not met, nothing is going to be copied.
However, for my use case the following conditions exist:
- The file does not end in flac
- The copy option is true
- The output file does not exist
- The overwrite option is false (which is irreleveant, since the output file does not exist)
- There is no mtime difference (since the output file does not exist)
Yep, replicated. It happens when you have a mp3 in the input folder which is not got an equivalent flac file for converting. How odd that this was not spotted sooner. Will investigate the copy code, as it has been a while since I touched this part of the logic.
This is very odd, you are right, the logic would not result in files being copied at all. I can't imagine that it has been like this for years, so this must have been a regression, especially as the missing piece of code is self contained, looks like an else statement magically vanished from the logic at some point.
I have pushed changes into the repo. Could you pull down the latest revision and test that the fix works for you.
Yup, it works now. Thanks!
Awesome! :) Closing ticket....
Can you tag a new release for the AUR package?
I want to run some more tests before I tag a new release.