From 29005f24ed84188774f251f3ca749ff742dc6f1d Mon Sep 17 00:00:00 2001 From: Ritiek Malhotra Date: Tue, 17 Mar 2020 03:09:56 +0530 Subject: [PATCH] Refactor exceptions * Suffix names for custom exceptions with "Error" * Introduce exceptions for when the coressponding encoder isn't found --- setup.py | 3 +- spotdl/encode/encode_base.py | 9 + spotdl/encode/encoders/avconv.py | 11 +- spotdl/encode/encoders/ffmpeg.py | 22 +- spotdl/encode/encoders/tests/__init__.py | 0 spotdl/encode/encoders/tests/test_ffmpeg.py | 188 ++++++++++++++++++ spotdl/encode/exceptions.py | 20 ++ spotdl/encode/tests/__init__.py | 0 spotdl/encode/tests/test_encode_base.py | 13 ++ spotdl/encode/tests/test_exceptions.py | 14 ++ spotdl/lyrics/exceptions.py | 4 +- spotdl/lyrics/providers/genius.py | 6 +- spotdl/lyrics/providers/lyricwikia_wrapper.py | 4 +- spotdl/lyrics/providers/tests/test_genius.py | 2 +- .../tests/test_lyricwikia_wrapper.py | 4 +- spotdl/spotify_tools.py | 4 +- 16 files changed, 276 insertions(+), 28 deletions(-) create mode 100644 spotdl/encode/encoders/tests/__init__.py create mode 100644 spotdl/encode/encoders/tests/test_ffmpeg.py create mode 100644 spotdl/encode/exceptions.py create mode 100644 spotdl/encode/tests/__init__.py create mode 100644 spotdl/encode/tests/test_encode_base.py create mode 100644 spotdl/encode/tests/test_exceptions.py diff --git a/setup.py b/setup.py index fc6d520..0f96c0c 100644 --- a/setup.py +++ b/setup.py @@ -14,7 +14,8 @@ setup( "spotdl", "spotdl.lyrics", "spotdl.lyrics.providers", - "spotdl.encoders", + "spotdl.encode", + "spotdl.encode.encoders", "spotdl.downloaders", "spotdl.patch", ], diff --git a/spotdl/encode/encode_base.py b/spotdl/encode/encode_base.py index 9c111dd..ec36b60 100644 --- a/spotdl/encode/encode_base.py +++ b/spotdl/encode/encode_base.py @@ -1,8 +1,11 @@ +import shutil import os from abc import ABC from abc import abstractmethod +from spotdl.encode.exceptions import EncoderNotFoundError + """ NOTE ON ENCODERS ================ @@ -25,6 +28,12 @@ from abc import abstractmethod class EncoderBase(ABC): @abstractmethod def __init__(self, encoder_path, loglevel, additional_arguments): + if shutil.which(encoder_path) is None: + raise EncoderNotFoundError( + "{} executable does not exist or was not found in PATH.".format( + encoder_path + ) + ) self.encoder_path = encoder_path self._loglevel = loglevel self._additional_arguments = additional_arguments diff --git a/spotdl/encode/encoders/avconv.py b/spotdl/encode/encoders/avconv.py index d94d84e..a3e9de3 100644 --- a/spotdl/encode/encoders/avconv.py +++ b/spotdl/encode/encoders/avconv.py @@ -2,16 +2,21 @@ import subprocess import os from logzero import logger as log from spotdl.encode import EncoderBase +from spotdl.encode.exceptions import EncoderNotFoundError +from spotdl.encode.exceptions import AvconvNotFoundError class EncoderAvconv(EncoderBase): def __init__(self, encoder_path="avconv"): - print("Using avconv is deprecated and this will be removed in", - "future versions. Use ffmpeg instead.") + print("Using EncoderAvconv is deprecated and will be removed" + "in future versions. Use EncoderFFmpeg instead.") encoder_path = encoder_path _loglevel = "-loglevel 0" _additional_arguments = ["-ab", "192k"] - super().__init__(encoder_path, _loglevel, _additional_arguments) + try: + super().__init__(encoder_path, _loglevel, _additional_arguments) + except EncoderNotFoundError as e: + raise AvconvNotFoundError(e.args[0]) def set_argument(self, argument): super().set_argument(argument) diff --git a/spotdl/encode/encoders/ffmpeg.py b/spotdl/encode/encoders/ffmpeg.py index 31dabc5..6818a7d 100644 --- a/spotdl/encode/encoders/ffmpeg.py +++ b/spotdl/encode/encoders/ffmpeg.py @@ -1,7 +1,10 @@ import subprocess import os from logzero import logger as log + from spotdl.encode import EncoderBase +from spotdl.encode.exceptions import EncoderNotFoundError +from spotdl.encode.exceptions import FFmpegNotFoundError RULES = { "m4a": { @@ -17,14 +20,15 @@ RULES = { }, } + class EncoderFFmpeg(EncoderBase): def __init__(self, encoder_path="ffmpeg"): - encoder_path = encoder_path _loglevel = "-hide_banner -nostats -v panic" _additional_arguments = ["-b:a", "192k", "-vn"] - - super().__init__(encoder_path, _loglevel, _additional_arguments) - + try: + super().__init__(encoder_path, _loglevel, _additional_arguments) + except EncoderNotFoundError as e: + raise FFmpegNotFoundError(e.args[0]) self._rules = RULES def set_argument(self, argument): @@ -41,18 +45,16 @@ class EncoderFFmpeg(EncoderBase): if initial_arguments is None: raise TypeError( 'The input format ("{}") is not supported.'.format( - input_extension, + input_encoding, ) ) - arguments = initial_arguments.get(output_encoding) if arguments is None: raise TypeError( 'The output format ("{}") is not supported.'.format( - output_extension, + output_encoding, ) ) - return arguments def set_debuglog(self): @@ -61,12 +63,10 @@ class EncoderFFmpeg(EncoderBase): def _generate_encode_command(self, input_file, output_file): input_encoding = self.get_encoding(input_file) output_encoding = self.get_encoding(output_file) - arguments = self._generate_encoding_arguments( input_encoding, output_encoding ) - command = [self.encoder_path] \ + ["-y", "-nostdin"] \ + self._loglevel.split() \ @@ -82,10 +82,8 @@ class EncoderFFmpeg(EncoderBase): input_file, output_file ) - returncode = subprocess.call(encode_command) encode_successful = returncode == 0 - if encode_successful and delete_original: os.remove(input_file) diff --git a/spotdl/encode/encoders/tests/__init__.py b/spotdl/encode/encoders/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/spotdl/encode/encoders/tests/test_ffmpeg.py b/spotdl/encode/encoders/tests/test_ffmpeg.py new file mode 100644 index 0000000..07b34f3 --- /dev/null +++ b/spotdl/encode/encoders/tests/test_ffmpeg.py @@ -0,0 +1,188 @@ +from spotdl.encode import EncoderBase +# from spotdl.encode import exceptions +from spotdl.encode.encoders import EncoderFFmpeg + +import pytest + + +class TestEncoderFFmpeg: + def test_subclass(self): + assert issubclass(EncoderFFmpeg, EncoderBase) + + +class TestEncodingDefaults: + def m4a_to_mp3_encoder(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:v', 'copy', + '-codec:a', 'libmp3lame', + '-ar', '48000', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_webm_encoder(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:a', 'libopus', + '-vbr', 'on', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_m4a_encoder(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-acodec', 'copy', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_flac_encoder(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:a', 'flac', + '-ar', '48000', + '-b:a', '192k', + '-vn', output_file + ] + return command + + @pytest.mark.parametrize("files, expected_command", [ + (("test.m4a", "test.mp3"), m4a_to_mp3_encoder("test.m4a", "test.mp3")), + (("abc.m4a", "cba.webm"), m4a_to_webm_encoder("abc.m4a", "cba.webm")), + (("bla bla.m4a", "ble ble.m4a"), m4a_to_m4a_encoder("bla bla.m4a", "ble ble.m4a")), + (("😛.m4a", "• tongue.flac"), m4a_to_flac_encoder("😛.m4a", "• tongue.flac")), + ]) + def test_generate_encode_command(self, files, expected_command): + encoder = EncoderFFmpeg() + assert encoder._generate_encode_command(*files) == expected_command + + +class TestEncodingInDebugMode: + def m4a_to_mp3_encoder_with_debug(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-loglevel', 'debug', + '-i', input_file, + '-codec:v', 'copy', + '-codec:a', 'libmp3lame', + '-ar', '48000', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_webm_encoder_with_debug(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-loglevel', 'debug', + '-i', input_file, + '-codec:a', 'libopus', + '-vbr', 'on', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_m4a_encoder_with_debug(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-loglevel', 'debug', + '-i', input_file, + '-acodec', 'copy', + '-b:a', '192k', + '-vn', output_file + ] + return command + + def m4a_to_flac_encoder_with_debug(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-loglevel', 'debug', + '-i', input_file, + '-codec:a', 'flac', + '-ar', '48000', + '-b:a', '192k', + '-vn', output_file + ] + return command + + @pytest.mark.parametrize("files, expected_command", [ + (("test.m4a", "test.mp3"), m4a_to_mp3_encoder_with_debug("test.m4a", "test.mp3")), + (("abc.m4a", "cba.webm"), m4a_to_webm_encoder_with_debug("abc.m4a", "cba.webm")), + (("bla bla.m4a", "ble ble.m4a"), m4a_to_m4a_encoder_with_debug("bla bla.m4a", "ble ble.m4a")), + (("😛.m4a", "• tongue.flac"), m4a_to_flac_encoder_with_debug("😛.m4a", "• tongue.flac")), + ]) + def test_generate_encode_command_with_debug(self, files, expected_command): + encoder = EncoderFFmpeg() + encoder.set_debuglog() + assert encoder._generate_encode_command(*files) == expected_command + + +class TestEncodingAndTrimSilence: + def m4a_to_mp3_encoder_and_trim_silence(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:v', 'copy', + '-codec:a', 'libmp3lame', + '-ar', '48000', + '-b:a', '192k', + '-vn', + '-af', 'silenceremove=start_periods=1', + output_file + ] + return command + + def m4a_to_webm_encoder_and_trim_silence(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:a', 'libopus', + '-vbr', 'on', + '-b:a', '192k', + '-vn', + '-af', 'silenceremove=start_periods=1', + output_file + ] + return command + + def m4a_to_m4a_encoder_and_trim_silence(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-acodec', 'copy', + '-b:a', '192k', + '-vn', + '-af', 'silenceremove=start_periods=1', + output_file + ] + return command + + def m4a_to_flac_encoder_and_trim_silence(input_file, output_file): + command = [ + 'ffmpeg', '-y', '-nostdin', '-hide_banner', '-nostats', '-v', 'panic', + '-i', input_file, + '-codec:a', 'flac', + '-ar', '48000', + '-b:a', '192k', + '-vn', + '-af', 'silenceremove=start_periods=1', + output_file + ] + return command + + @pytest.mark.parametrize("files, expected_command", [ + (("test.m4a", "test.mp3"), m4a_to_mp3_encoder_and_trim_silence("test.m4a", "test.mp3")), + (("abc.m4a", "cba.webm"), m4a_to_webm_encoder_and_trim_silence("abc.m4a", "cba.webm")), + (("bla bla.m4a", "ble ble.m4a"), m4a_to_m4a_encoder_and_trim_silence("bla bla.m4a", "ble ble.m4a")), + (("😛.m4a", "• tongue.flac"), m4a_to_flac_encoder_and_trim_silence("😛.m4a", "• tongue.flac")), + ]) + def test_generate_encode_command_and_trim_silence(self, files, expected_command): + encoder = EncoderFFmpeg() + encoder.set_trim_silence() + assert encoder._generate_encode_command(*files) == expected_command diff --git a/spotdl/encode/exceptions.py b/spotdl/encode/exceptions.py new file mode 100644 index 0000000..bffa07e --- /dev/null +++ b/spotdl/encode/exceptions.py @@ -0,0 +1,20 @@ +class EncoderNotFoundError(Exception): + __module__ = Exception.__module__ + + def __init__(self, message=None): + super(EncoderNotFoundError, self).__init__(message) + + +class FFmpegNotFoundError(EncoderNotFoundError): + __module__ = Exception.__module__ + + def __init__(self, message=None): + super(FFmpegNotFoundError, self).__init__(message) + + +class AvconvNotFoundError(EncoderNotFoundError): + __module__ = Exception.__module__ + + def __init__(self, message=None): + super(AvconvNotFoundError, self).__init__(message) + diff --git a/spotdl/encode/tests/__init__.py b/spotdl/encode/tests/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/spotdl/encode/tests/test_encode_base.py b/spotdl/encode/tests/test_encode_base.py new file mode 100644 index 0000000..7a171bf --- /dev/null +++ b/spotdl/encode/tests/test_encode_base.py @@ -0,0 +1,13 @@ +from spotdl.encode import EncoderBase + +import pytest + +def test_abstract_base_class_encoderbase(): + encoder_path = "ffmpeg" + _loglevel = "-hide_banner -nostats -v panic" + _additional_arguments = ["-b:a", "192k", "-vn"] + + with pytest.raises(TypeError): + # This abstract base class must be inherited from + # for instantiation + EncoderBase(encoder_path, _loglevel, _additional_arguments) diff --git a/spotdl/encode/tests/test_exceptions.py b/spotdl/encode/tests/test_exceptions.py new file mode 100644 index 0000000..56adbb5 --- /dev/null +++ b/spotdl/encode/tests/test_exceptions.py @@ -0,0 +1,14 @@ +from spotdl.encode.exceptions import EncoderNotFoundError +from spotdl.encode.exceptions import FFmpegNotFoundError +from spotdl.encode.exceptions import AvconvNotFoundError + +import pytest + + +class TestEncoderNotFoundSubclass: + def test_ffmpeg_not_found_subclass(self): + assert issubclass(FFmpegNotFoundError, EncoderNotFoundError) + + def test_avconv_not_found_subclass(self): + assert issubclass(AvconvNotFoundError, EncoderNotFoundError) + diff --git a/spotdl/lyrics/exceptions.py b/spotdl/lyrics/exceptions.py index ec0afe1..0143a80 100644 --- a/spotdl/lyrics/exceptions.py +++ b/spotdl/lyrics/exceptions.py @@ -1,5 +1,5 @@ -class LyricsNotFound(Exception): +class LyricsNotFoundError(Exception): __module__ = Exception.__module__ def __init__(self, message=None): - super(LyricsNotFound, self).__init__(message) + super(LyricsNotFoundError, self).__init__(message) diff --git a/spotdl/lyrics/providers/genius.py b/spotdl/lyrics/providers/genius.py index 399b690..e095387 100644 --- a/spotdl/lyrics/providers/genius.py +++ b/spotdl/lyrics/providers/genius.py @@ -2,7 +2,7 @@ from bs4 import BeautifulSoup import urllib.request from spotdl.lyrics.lyric_base import LyricBase -from spotdl.lyrics.exceptions import LyricsNotFound +from spotdl.lyrics.exceptions import LyricsNotFoundError BASE_URL = "https://genius.com" @@ -26,7 +26,7 @@ class Genius(LyricBase): try: response = urllib.request.urlopen(request, timeout=timeout) except urllib.request.HTTPError: - raise LyricsNotFound( + raise LyricsNotFoundError( "Could not find lyrics for {} - {} at URL: {}".format( self.artist, self.song, url ) @@ -40,7 +40,7 @@ class Genius(LyricBase): if lyrics_paragraph: return lyrics_paragraph.get_text() else: - raise LyricsNotFound("The lyrics for this track are yet to be released.") + raise LyricsNotFoundError("The lyrics for this track are yet to be released.") def get_lyrics(self, linesep="\n", timeout=None): url = self._guess_lyric_url() diff --git a/spotdl/lyrics/providers/lyricwikia_wrapper.py b/spotdl/lyrics/providers/lyricwikia_wrapper.py index 2ac3690..e7fb893 100644 --- a/spotdl/lyrics/providers/lyricwikia_wrapper.py +++ b/spotdl/lyrics/providers/lyricwikia_wrapper.py @@ -1,7 +1,7 @@ import lyricwikia from spotdl.lyrics.lyric_base import LyricBase -from spotdl.lyrics.exceptions import LyricsNotFound +from spotdl.lyrics.exceptions import LyricsNotFoundError class LyricWikia(LyricBase): @@ -13,6 +13,6 @@ class LyricWikia(LyricBase): try: lyrics = lyricwikia.get_lyrics(self.artist, self.song, linesep, timeout) except lyricwikia.LyricsNotFound as e: - raise LyricsNotFound(e.args[0]) + raise LyricsNotFoundError(e.args[0]) else: return lyrics diff --git a/spotdl/lyrics/providers/tests/test_genius.py b/spotdl/lyrics/providers/tests/test_genius.py index ae39d94..95f5c73 100644 --- a/spotdl/lyrics/providers/tests/test_genius.py +++ b/spotdl/lyrics/providers/tests/test_genius.py @@ -33,5 +33,5 @@ class TestGenius: raise urllib.request.HTTPError("", "", "", "", "") monkeypatch.setattr("urllib.request.urlopen", mocked_urlopen) - with pytest.raises(exceptions.LyricsNotFound): + with pytest.raises(exceptions.LyricsNotFoundError): track.get_lyrics() diff --git a/spotdl/lyrics/providers/tests/test_lyricwikia_wrapper.py b/spotdl/lyrics/providers/tests/test_lyricwikia_wrapper.py index 92142fa..7cab7d5 100644 --- a/spotdl/lyrics/providers/tests/test_lyricwikia_wrapper.py +++ b/spotdl/lyrics/providers/tests/test_lyricwikia_wrapper.py @@ -25,11 +25,11 @@ class TestLyricWikia: def lyricwikia_lyrics_not_found(msg): raise lyricwikia.LyricsNotFound(msg) - # Wrap `lyricwikia.LyricsNotFound` with `exceptions.LyricsNotFound` error. + # Wrap `lyricwikia.LyricsNotFoundError` with `exceptions.LyricsNotFoundError` error. monkeypatch.setattr( "lyricwikia.get_lyrics", lambda a, b, c, d: lyricwikia_lyrics_not_found("Nope, no lyrics."), ) track = LyricWikia("Lyricwikia", "Lyricwikia") - with pytest.raises(exceptions.LyricsNotFound): + with pytest.raises(exceptions.LyricsNotFoundError): track.get_lyrics() diff --git a/spotdl/spotify_tools.py b/spotdl/spotify_tools.py index 07d974e..cb92787 100644 --- a/spotdl/spotify_tools.py +++ b/spotdl/spotify_tools.py @@ -12,7 +12,7 @@ import functools from spotdl import const from spotdl import internals from spotdl.lyrics.providers import LyricClasses -from spotdl.lyrics.exceptions import LyricsNotFound +from spotdl.lyrics.exceptions import LyricsNotFoundError spotify = None @@ -82,7 +82,7 @@ def generate_metadata(raw_song): track = LyricClass(meta_tags["artists"][0]["name"], meta_tags["name"]) try: meta_tags["lyrics"] = track.get_lyrics() - except LyricsNotFound: + except LyricsNotFoundError: continue else: break