Merge pull request #1241 from spotDL/dev

* fixed ffmpeg installation for tests (#1235)
Authored by @xnetcat

* check errors from ffmpeg sub process (#1224)
Authored by @xnetcat 
Co-authored-by: Silverarmor <23619946+Silverarmor@users.noreply.github.com>
Co-authored-by: Andrzej Klajnert <github@aklajnert.pl>


* Update Spotify dev app Client ID & secret.

* Fix ffmpeg (#1245)
Authored by @Silverarmor 

* fixed regressions tests
Co-authored-by: Jakub <42355410+xnetcat@users.noreply.github.com>

* Remove outdated info re hotfix branch
* Bump version number to 3.5.1

Co-authored-by: Jakub <42355410+xnetcat@users.noreply.github.com>
Co-authored-by: Andrzej Klajnert <github@aklajnert.pl>

.github/workflows/spotify-downloader-ci.yml

@@ -73,9 +73,13 @@ jobs:
           python-version: 3.8
       - name: Install dependencies
         run: |
-          sudo add-apt-repository ppa:savoury1/ffmpeg4 -y
-          sudo apt-get update
-          sudo apt install ffmpeg -y
+          wget -nv https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz
+          wget -nv https://johnvansickle.com/ffmpeg/releases/ffmpeg-release-amd64-static.tar.xz.md5
+          tar xf ffmpeg-release-amd64-static.tar.xz
+          cd $(ls -d ffmpeg*/ | head -n 1)
+          sudo mv ffmpeg ffprobe /usr/local/bin/
+          cd ..
+          ffmpeg -version
           python -m pip install -e .[test]
       - name: Run tests
         run: |

INSTALLATION.md

@@ -34,12 +34,6 @@ You can install spotDL by opening a terminal and typing:
 pip install spotdl
 ```
 
-If you encounter errors, our `hotfix` branch may be able to help you:
-
-1. `pip install pip-autoremove`
-2. `pip-autoremove spotdl`
-3. `pip cache purge`
-4. `pip install https://codeload.github.com/spotDL/spotify-downloader/zip/hotfix`
 
 If you require further help, ask in our [Discord Server](https://discord.gg/xCa23pwJWY)
 

setup.cfg

@@ -1,5 +1,5 @@
 [metadata]
-version = 3.5.0
+version = 3.5.1
 
 name = spotdl
 url = https://github.com/spotDL/spotify-downloader
@@ -47,6 +47,7 @@ test =
     pytest-vcr >= 1.0.2
     pyfakefs >= 4.3.0
     pytest-cov >= 2.10.1
+    pytest-subprocess
 dev =
     tox
     mypy==0.790

spotdl/__main__.py

@@ -14,6 +14,7 @@
     get_artist_tracks,
     search_for_song,
 )
+from spotdl.download import ffmpeg
 
 # ! Usage is simple - call:
 #   'python __main__.py <links, search terms, tracking files separated by spaces>
@@ -90,9 +91,12 @@ def console_entry_point():
     '''
     arguments = parse_arguments()
 
+    if ffmpeg.has_correct_version(arguments.ignore_ffmpeg_version) is False:
+        sys.exit(1)
+
     SpotifyClient.init(
-        client_id='4fe3fecfe5334023a1472516cc99d805',
-        client_secret='0f02b7c483c04257984695007a4a8d5c'
+        client_id='5f573c9620494bae87890c0f08a60293',
+        client_secret='212476d9b0f3472eaa762d90b19b0ba8'
     )
 
     if arguments.path:
@@ -154,6 +158,8 @@ def parse_arguments():
     )
     parser.add_argument("url", type=str, nargs="+", help="URL to a song/album/playlist")
     parser.add_argument("-o", "--output", help="Output directory path", dest="path")
+    parser.add_argument("--ignore-ffmpeg-version",
+                        help="Ignore ffmpeg version", action="store_true")
 
     return parser.parse_args()
 

spotdl/download/downloader.py

@@ -17,6 +17,7 @@
 
 from spotdl.download.progressHandlers import DisplayManager, DownloadTracker
 from spotdl.search.songObj import SongObj
+from spotdl.download import ffmpeg
 
 
 # ==========================
@@ -43,6 +44,7 @@ def __init__(self):
             # ! it is default since Python 3.8 but has to be changed for previous versions
             loop = asyncio.ProactorEventLoop()
             asyncio.set_event_loop(loop)
+
         self.loop = asyncio.get_event_loop()
         # ! semaphore is required to limit concurrent asyncio executions
         self.semaphore = asyncio.Semaphore(self.poolSize)
@@ -206,65 +208,26 @@ async def download_song(self, songObj: SongObj) -> None:
             if downloadedFilePathString is None:
                 return None
 
-            downloadedFilePath = Path(downloadedFilePathString)
-
             if dispayProgressTracker:
                 dispayProgressTracker.notify_youtube_download_completion()
 
-            # convert downloaded file to MP3 with normalization
-
-            # ! -af loudnorm=I=-7:LRA applies EBR 128 loudness normalization algorithm with
-            # ! intergrated loudness target (I) set to -17, using values lower than -15
-            # ! causes 'pumping' i.e. rhythmic variation in loudness that should not
-            # ! exist -loud parts exaggerate, soft parts left alone.
-            # !
-            # ! dynaudnorm applies dynamic non-linear RMS based normalization, this is what
-            # ! actually normalized the audio. The loudnorm filter just makes the apparent
-            # ! loudness constant
-            # !
-            # ! apad=pad_dur=2 adds 2 seconds of silence toward the end of the track, this is
-            # ! done because the loudnorm filter clips/cuts/deletes the last 1-2 seconds on
-            # ! occasion especially if the song is EDM-like, so we add a few extra seconds to
-            # ! combat that.
-            # !
-            # ! -acodec libmp3lame sets the encoded to 'libmp3lame' which is far better
-            # ! than the default 'mp3_mf', '-abr true' automatically determines and passes the
-            # ! audio encoding bitrate to the filters and encoder. This ensures that the
-            # ! sampled length of songs matches the actual length (i.e. a 5 min song won't display
-            # ! as 47 seconds long in your music player, yeah that was an issue earlier.)
-
-            command = 'ffmpeg -v quiet -y -i "%s" -acodec libmp3lame -abr true ' \
-                f'-b:a {trackAudioStream.bitrate} ' \
-                '-af "apad=pad_dur=2, dynaudnorm, loudnorm=I=-17" "%s"'
-
-            # ! bash/ffmpeg on Unix systems need to have excape char (\) for special characters: \$
-            # ! alternatively the quotes could be reversed (single <-> double) in the command then
-            # ! the windows special characters needs escaping (^): ^\  ^&  ^|  ^>  ^<  ^^
-
-            if sys.platform == 'win32':
-                formattedCommand = command % (
-                    str(downloadedFilePath),
-                    str(convertedFilePath)
-                )
-            else:
-                formattedCommand = command % (
-                    str(downloadedFilePath).replace('$', '\$'),
-                    str(convertedFilePath).replace('$', '\$')
-                )
-
-            process = await asyncio.subprocess.create_subprocess_shell(formattedCommand)
-            _ = await process.communicate()
+            downloadedFilePath = Path(downloadedFilePathString)
 
-            # ! Wait till converted file is actually created
-            while True:
-                if convertedFilePath.is_file():
-                    break
+            ffmpeg_success = await ffmpeg.convert(
+                trackAudioStream=trackAudioStream,
+                downloadedFilePath=downloadedFilePath,
+                convertedFilePath=convertedFilePath
+            )
 
             if dispayProgressTracker:
                 dispayProgressTracker.notify_conversion_completion()
 
-            # embed song details
-            self.set_id3_data(convertedFilePath, songObj)
+            if ffmpeg_success is False:
+                # delete the file that wasn't successfully converted
+                convertedFilePath.unlink()
+            else:
+                # if a file was successfully downloaded, tag it
+                self.set_id3_data(convertedFilePath, songObj)
 
             # Do the necessary cleanup
             if dispayProgressTracker:

spotdl/download/ffmpeg.py

@@ -0,0 +1,100 @@
+import asyncio
+import subprocess
+import sys
+import re
+
+
+def has_correct_version(skip_version_check: bool = False) -> bool:
+    process = subprocess.Popen(
+        ['ffmpeg', '-version'],
+        shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE
+    )
+
+    proc_out, _ = process.communicate()
+
+    if process.returncode == 127:
+        print("FFmpeg was not found, spotDL cannot continue.", file=sys.stderr)
+        return False
+
+    if skip_version_check is False:
+        result = re.search(r"ffmpeg version \w?(\d+\.)?(\d+)", proc_out.decode("utf-8"))
+
+        if result is None:
+            print("Your FFmpeg version couldn't be detected", file=sys.stderr)
+            return False
+
+        version = result.group(0).replace("ffmpeg version ", "")
+        if float(version) < 4.3:
+            print(f"Your FFmpeg installation is too old ({version}), please update to 4.3+\n",
+                  file=sys.stderr)
+            return False
+
+    return True
+
+
+async def convert(trackAudioStream, downloadedFilePath, convertedFilePath) -> bool:
+    # convert downloaded file to MP3 with normalization
+
+    # ! -af loudnorm=I=-7:LRA applies EBR 128 loudness normalization algorithm with
+    # ! intergrated loudness target (I) set to -17, using values lower than -15
+    # ! causes 'pumping' i.e. rhythmic variation in loudness that should not
+    # ! exist -loud parts exaggerate, soft parts left alone.
+    # !
+    # ! dynaudnorm applies dynamic non-linear RMS based normalization, this is what
+    # ! actually normalized the audio. The loudnorm filter just makes the apparent
+    # ! loudness constant
+    # !
+    # ! apad=pad_dur=2 adds 2 seconds of silence toward the end of the track, this is
+    # ! done because the loudnorm filter clips/cuts/deletes the last 1-2 seconds on
+    # ! occasion especially if the song is EDM-like, so we add a few extra seconds to
+    # ! combat that.
+    # !
+    # ! -acodec libmp3lame sets the encoded to 'libmp3lame' which is far better
+    # ! than the default 'mp3_mf', '-abr true' automatically determines and passes the
+    # ! audio encoding bitrate to the filters and encoder. This ensures that the
+    # ! sampled length of songs matches the actual length (i.e. a 5 min song won't display
+    # ! as 47 seconds long in your music player, yeah that was an issue earlier.)
+
+    command = (
+        'ffmpeg -v quiet -y -i "%s" -acodec libmp3lame -abr true '
+        f"-b:a {trackAudioStream.bitrate} "
+        '-af "apad=pad_dur=2, dynaudnorm, loudnorm=I=-17" "%s"'
+    )
+
+    # ! bash/ffmpeg on Unix systems need to have excape char (\) for special characters: \$
+    # ! alternatively the quotes could be reversed (single <-> double) in the command then
+    # ! the windows special characters needs escaping (^): ^\  ^&  ^|  ^>  ^<  ^^
+
+    if sys.platform == "win32":
+        formattedCommand = command % (
+            str(downloadedFilePath),
+            str(convertedFilePath),
+        )
+    else:
+        formattedCommand = command % (
+            str(downloadedFilePath).replace("$", r"\$"),
+            str(convertedFilePath).replace("$", r"\$"),
+        )
+
+    process = await asyncio.subprocess.create_subprocess_shell(
+        formattedCommand,
+        stdout=asyncio.subprocess.PIPE,
+        stderr=asyncio.subprocess.PIPE,
+    )
+
+    _, proc_err = await process.communicate()
+
+    if process.returncode != 0:
+        message = (
+            f"ffmpeg returned an error ({process.returncode})"
+            f"\nthe ffmpeg command was \"{formattedCommand}\""
+            "\nffmpeg gave this output:"
+            "\n=====\n"
+            f"{proc_err.decode('utf-8')}"
+            "\n=====\n"
+        )
+
+        print(message, file=sys.stderr)
+        return False
+
+    return True

tests/regressions.py

@@ -1,6 +1,7 @@
 import sys
 
 from spotdl.__main__ import console_entry_point
+from spotdl.download import ffmpeg
 
 SONGS = {
     "https://open.spotify.com/track/6CN3e26iQSj1N5lomh0mfO": "Eminem - Like Toy Soldiers.mp3",
@@ -14,6 +15,7 @@ def test_regressions(monkeypatch, tmpdir):
     """
     monkeypatch.chdir(tmpdir)
     monkeypatch.setattr(sys, "argv", ["dummy", *SONGS.keys()])
+    monkeypatch.setattr(ffmpeg, "has_correct_version", lambda *_: True)
 
     console_entry_point()
 

tests/test_downloader.py

@@ -7,6 +7,7 @@
 
 from spotdl.download.downloader import DownloadManager
 from spotdl.search.songObj import SongObj
+from spotdl.download import ffmpeg
 
 
 def create_song_obj(name="test song", artist="test artist") -> SongObj:
@@ -53,14 +54,22 @@ async def communicate(self):
         self._output.open("w").close()
         return (None, None)
 
+    async def wait(self):
+        return None
 
-async def fake_create_subprocess_shell(command):
+    @property
+    def returncode(self):
+        return 0
+
+
+async def fake_create_subprocess_shell(command, stdout=None, stderr=None):
     return FakeProcess(command)
 
 
 @pytest.fixture()
 def setup(tmpdir, monkeypatch):
     monkeypatch.chdir(tmpdir)
+    monkeypatch.setattr(ffmpeg, "has_correct_version", lambda *_: True)
     monkeypatch.setattr(
         asyncio.subprocess, "create_subprocess_shell", fake_create_subprocess_shell
     )

tests/test_entry_point.py

@@ -6,6 +6,7 @@
 from spotdl.__main__ import console_entry_point, help_notice
 from spotdl.download.downloader import DownloadManager
 from spotdl.search.spotifyClient import SpotifyClient
+from spotdl.download import ffmpeg
 
 from tests.utils import tracking_files
 
@@ -25,6 +26,7 @@ def patch_dependencies(mocker, monkeypatch):
     """This is a helper fixture to patch out everything that shouldn't be called here"""
     monkeypatch.setattr(SpotifyClient, "init", new_initialize)
     monkeypatch.setattr(DownloadManager, "__init__", lambda _: None)
+    monkeypatch.setattr(ffmpeg, "has_correct_version", lambda *_: True)
     mocker.patch.object(DownloadManager, "download_single_song", autospec=True)
     mocker.patch.object(
         DownloadManager, "download_multiple_songs", autospec=True)