Fix conversion conflicts when both input and output filenames are same (#459)

* Workaround conversion conflicts by appending '.temp' to input filename

* Fix tests

* Add a test and some minor changes

* Update CHANGES.md
This commit is contained in:
Ritiek Malhotra
2019-01-02 18:54:27 +00:00
committed by GitHub
parent 1cf421960c
commit 53dd292b55
6 changed files with 78 additions and 28 deletions

4
.gitignore vendored
View File

@@ -1,8 +1,10 @@
# Spotdl generated files
*.m4a
*.mp3
config.yml
Music/
*.txt
*.m3u
upload.sh
.pytest_cache/

View File

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Refactored core downloading module ([@ritiek](https://github.com/ritiek)) (#410)
### Fixed
- Workaround conversion conflicts when input and output filename are same ([@ritiek](https://github.com/ritiek)) (#459)
- Applied a check on result in case of search using Spotify-URL `--no-metadata` option ([@Amit-L](https://github.com/Amit-L)) (#452)
- Included a missing `import spotipy` in downloader.py ([@ritiek](https://github.com/ritiek)) (#440)

View File

@@ -18,25 +18,46 @@ https://trac.ffmpeg.org/wiki/Encode/AAC
def song(input_song, output_song, folder, avconv=False, trim_silence=False):
""" Do the audio format conversion. """
convert = Converter(input_song, output_song, folder, trim_silence)
if avconv and trim_silence:
raise ValueError("avconv does not support trim_silence")
if not input_song == output_song:
log.info("Converting {0} to {1}".format(input_song, output_song.split(".")[-1]))
elif input_song.endswith(".m4a"):
log.info('Correcting container in "{}"'.format(input_song))
else:
return 0
convert = Converter(input_song, output_song, folder, delete_original=True)
if avconv:
exit_code, command = convert.with_avconv()
else:
exit_code, command = convert.with_ffmpeg()
exit_code, command = convert.with_ffmpeg(trim_silence=trim_silence)
return exit_code, command
class Converter:
def __init__(self, input_song, output_song, folder, trim_silence=False):
self.input_file = os.path.join(folder, input_song)
def __init__(self, input_song, output_song, folder, delete_original):
_, self.input_ext = os.path.splitext(input_song)
_, self.output_ext = os.path.splitext(output_song)
self.output_file = os.path.join(folder, output_song)
self.trim_silence = trim_silence
rename_to_temp = False
same_file = os.path.abspath(input_song) == os.path.abspath(output_song)
if same_file:
# FFmpeg/avconv cannot have the same file for both input and output
# This would happen when the extensions are same, so rename
# the input track to append ".temp"
log.debug('Input file and output file are going will be same during encoding, will append ".temp" to input file just before starting encoding to avoid conflict')
input_song = output_song + ".temp"
rename_to_temp = True
delete_original = True
self.input_file = os.path.join(folder, input_song)
self.rename_to_temp = rename_to_temp
self.delete_original = delete_original
def with_avconv(self):
if log.level == 10:
@@ -56,45 +77,48 @@ class Converter:
"-y",
]
if self.trim_silence:
log.warning("--trim-silence not supported with avconv")
if self.rename_to_temp:
os.rename(self.output_file, self.input_file)
log.debug(command)
return subprocess.call(command), command
code = subprocess.call(command)
def with_ffmpeg(self):
if self.delete_original:
log.debug('Removing original file: "{}"'.format(self.input_file))
os.remove(self.input_file)
return code, command
def with_ffmpeg(self, trim_silence=False):
ffmpeg_pre = "ffmpeg -y "
if not log.level == 10:
ffmpeg_pre += "-hide_banner -nostats -v panic "
_, input_ext = os.path.splitext(self.input_file)
_, output_ext = os.path.splitext(self.output_file)
ffmpeg_params = ""
if input_ext == ".m4a":
if output_ext == ".mp3":
if self.input_ext == ".m4a":
if self.output_ext == ".mp3":
ffmpeg_params = "-codec:v copy -codec:a libmp3lame -ar 44100 "
elif output_ext == ".webm":
elif self.output_ext == ".webm":
ffmpeg_params = "-codec:a libopus -vbr on "
elif output_ext == ".m4a":
ffmpeg_params = "-vn -acodec copy "
elif self.output_ext == ".m4a":
ffmpeg_params = "-acodec copy "
elif input_ext == ".webm":
if output_ext == ".mp3":
elif self.input_ext == ".webm":
if self.output_ext == ".mp3":
ffmpeg_params = "-codec:a libmp3lame -ar 44100 "
elif output_ext == ".m4a":
elif self.output_ext == ".m4a":
ffmpeg_params = "-cutoff 20000 -codec:a aac -ar 44100 "
if output_ext == ".flac":
if self.output_ext == ".flac":
ffmpeg_params = "-codec:a flac -ar 44100 "
# add common params for any of the above combination
ffmpeg_params += "-b:a 192k -vn "
ffmpeg_pre += " -i"
ffmpeg_pre += "-i "
if self.trim_silence:
if trim_silence:
ffmpeg_params += "-af silenceremove=start_periods=1 "
command = (
@@ -104,5 +128,14 @@ class Converter:
+ [self.output_file]
)
if self.rename_to_temp:
os.rename(self.output_file, self.input_file)
log.debug(command)
return subprocess.call(command), command
code = subprocess.call(command)
if self.delete_original:
log.debug('Removing original file: "{}"'.format(self.input_file))
os.remove(self.input_file)
return code, command

View File

@@ -136,8 +136,6 @@ class Downloader:
except FileNotFoundError:
output_song = self.unconverted_filename(songname)
if not const.args.input_ext == const.args.output_ext:
os.remove(os.path.join(const.args.folder, input_song))
if not const.args.no_metadata and self.meta_tags is not None:
metadata.embed(
os.path.join(const.args.folder, output_song), self.meta_tags

View File

@@ -273,6 +273,9 @@ def get_arguments(raw_args=None, to_group=True, to_merge=True):
if parsed.write_m3u and not parsed.list:
parser.error('--write-m3u can only be used with --list')
if parsed.avconv and parsed.trim_silence:
parser.error("--trim-silence can only be used with FFmpeg")
parsed.log_level = log_leveller(parsed.log_level)
return parsed

View File

@@ -112,6 +112,7 @@ class TestDownload:
class TestFFmpeg:
def test_convert_from_webm_to_mp3(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "ffmpeg -y -i {0}.webm -codec:a libmp3lame -ar 44100 -b:a 192k -vn {0}.mp3".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".webm", filename_fixture + ".mp3", const.args.folder
@@ -119,6 +120,7 @@ class TestFFmpeg:
assert ' '.join(command) == expect_command
def test_convert_from_webm_to_m4a(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "ffmpeg -y -i {0}.webm -cutoff 20000 -codec:a aac -ar 44100 -b:a 192k -vn {0}.m4a".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".webm", filename_fixture + ".m4a", const.args.folder
@@ -126,6 +128,7 @@ class TestFFmpeg:
assert ' '.join(command) == expect_command
def test_convert_from_m4a_to_mp3(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "ffmpeg -y -i {0}.m4a -codec:v copy -codec:a libmp3lame -ar 44100 -b:a 192k -vn {0}.mp3".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".m4a", filename_fixture + ".mp3", const.args.folder
@@ -133,6 +136,7 @@ class TestFFmpeg:
assert ' '.join(command) == expect_command
def test_convert_from_m4a_to_webm(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "ffmpeg -y -i {0}.m4a -codec:a libopus -vbr on -b:a 192k -vn {0}.webm".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".m4a", filename_fixture + ".webm", const.args.folder
@@ -140,15 +144,24 @@ class TestFFmpeg:
assert ' '.join(command) == expect_command
def test_convert_from_m4a_to_flac(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "ffmpeg -y -i {0}.m4a -codec:a flac -ar 44100 -b:a 192k -vn {0}.flac".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".m4a", filename_fixture + ".flac", const.args.folder
)
assert ' '.join(command) == expect_command
def test_correct_container_for_m4a(self, filename_fixture, monkeypatch):
expect_command = "ffmpeg -y -i {0}.m4a.temp -acodec copy -b:a 192k -vn {0}.m4a".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".m4a", filename_fixture + ".m4a", const.args.folder
)
assert ' '.join(command) == expect_command
class TestAvconv:
def test_convert_from_m4a_to_mp3(self, filename_fixture):
def test_convert_from_m4a_to_mp3(self, filename_fixture, monkeypatch):
monkeypatch.setattr("os.remove", lambda x: None)
expect_command = "avconv -loglevel debug -i {0}.m4a -ab 192k {0}.mp3 -y".format(os.path.join(const.args.folder, filename_fixture))
_, command = convert.song(
filename_fixture + ".m4a", filename_fixture + ".mp3", const.args.folder, avconv=True