Tidal-Media-Downloader icon indicating copy to clipboard operation
Tidal-Media-Downloader copied to clipboard

Sleeping only for missing songs, edit error for Album not found

Open bloedboemmel opened this issue 2 years ago • 12 comments

Closes #1018

bloedboemmel avatar Nov 13 '22 16:11 bloedboemmel

I tested it out because I was going to suggest the same feature when I saw your PR! :)

It works really well, there is only one issue with it: When you are syncing a large playlist you are running into the Tidal 429 (too many connections) error and have to wait the 20 sec cooldown. I guess that comes from the much quicker execution of TIDAL_API.getStreamUrl for each track. Is there a way to even cut short execution before that call gets made?

strikeout avatar Nov 24 '22 12:11 strikeout

Is there a way to even cut short execution before that call gets made?

Hey @strikeout , thanks for your review. I think there should be a way, because you can get all playlist-tracks at once. I was a little lazy tbh, but I will have a look into it again.

bloedboemmel avatar Nov 24 '22 23:11 bloedboemmel

Now it avoids also the 429 Errors. @strikeout, please confirm :)

bloedboemmel avatar Nov 28 '22 09:11 bloedboemmel

Niiice! Confirmed working as intended.

strikeout avatar Nov 29 '22 22:11 strikeout

Ok I found a little bug. I'm not sure if it is directly related to your PR but here it goes: if a track is unavailable for streaming it's going to be downloaded as a video by default, and obviously fails. I traced this to the faulty logic in tidal.py, line 266+:

// tidal.py
for item in data:
    if item['type'] == 'track' and item['item']['streamReady']:
        tracks.append(aigpy.model.dictToModel(item['item'], Track()))
    else:
        videos.append(aigpy.model.dictToModel(item['item'], Video()))

should be along the lines of:

for item in data:
            if item['type'] == 'track' and item['item']['streamReady']:
                tracks.append(aigpy.model.dictToModel(item['item'], Track()))
            elif item['type'] == 'video' and item['item']['streamReady']:
                videos.append(aigpy.model.dictToModel(item['item'], Video()))
            else:
                <some cool error handling here>

strikeout avatar Nov 29 '22 23:11 strikeout

Niiice! Confirmed working as intended.

Thanks for checking :)

if a track is unavailable for streaming it's going to be downloaded as a video by default, and obviously fails

I don't think that has something to do with my pull request. You should open another issue for that.

bloedboemmel avatar Nov 30 '22 12:11 bloedboemmel

May I suggest moving the "skip if file exists" logic from the "downloadTrack" function to the "downloadTracks" function of download.py.

Problem: TIDAL_API.getAlbum() gets called for every track in playlist before the "skip if file exists"-logic of the current implementation resulting in "429 too many API calls"-Error for large playlists. It is also slow because of that.

Proposed solution: Implement "skip if file exists" -logic in the "downloadTracks"-function when looping through the tracklist.

Here is my working implementation of the suggested change:

download.py

def downloadTracks(...

   ...
   if not SETTINGS.multiThread:
        for index, item in enumerate(tracks):
           path = getTrackPath(item, album, playlist)
           if os.path.exists(path):
               Printf.success(aigpy.path.getFileName(path) + " (skip super early:already exists!)")
               continue

           itemAlbum = album
           if itemAlbum is None:
              itemAlbum = __getAlbum__(item)
              item.trackNumberOnPlaylist = index + 1

           downloadTrack(item, playlist=playlist, album=itemAlbum)
        ...

donbernhardo avatar Feb 24 '23 13:02 donbernhardo

May I suggest moving the "skip if file exists" logic from the "downloadTrack" function to the "downloadTracks" function of download.py.

Problem: TIDAL_API.getAlbum() gets called for every track in playlist before the "skip if file exists"-logic of the current implementation resulting in "429 too many API calls"-Error for large playlists. It is also slow because of that.

Proposed solution: Implement "skip if file exists" -logic in the "downloadTracks"-function when looping through the tracklist.

Here is my working implementation of the suggested change:

download.py

def downloadTracks(...

   ...
   if not SETTINGS.multiThread:
        for index, item in enumerate(tracks):
           path = getTrackPath(item, album, playlist)
           if os.path.exists(path):
               Printf.success(aigpy.path.getFileName(path) + " (skip super early:already exists!)")
               continue

           itemAlbum = album
           if itemAlbum is None:
              itemAlbum = __getAlbum__(item)
              item.trackNumberOnPlaylist = index + 1

           downloadTrack(item, playlist=playlist, album=itemAlbum)
        ...

For what is worth, I tried your fork and ran into the following error:

[ERR] 'Playlist' object has no attribute 'artists'

What are your specific settings to make this work?

j-alvarez-moreno avatar Mar 07 '23 08:03 j-alvarez-moreno

For what is worth, I tried your fork and ran into the following error:

[ERR] 'Playlist' object has no attribute 'artists'

What are your specific settings to make this work?

I checked out from here:

git fetch origin pull/1020/head:lobra
git checkout lobra

I attached the download.py with the changes that are working for me. The zip also includes all the other python-files changes by this pull request.

tidal-dl.zip

donbernhardo avatar Mar 07 '23 10:03 donbernhardo

Thank you @donbernhardo, that did the trick. To any person from the future who may be running into this thread: multithreaded downloads must be off for this fork.

If you want to make additional changes to the too many requests, waiting for x seconds backoff logic, take a look a tidal.py:44 and tweak the number of seconds to wait there.

In my case (multiple playlists with almost a thousand songs on each) 300 seconds is a safe value to avoid going over the threshold.

j-alvarez-moreno avatar Mar 09 '23 07:03 j-alvarez-moreno

hey, thanks for work on this, it will be a great speed up in my workflow when I manage to make it work :)

I didn't get to look into the root cause yet, just reporting for now it seems to me that the file format is hardcoded to .flac in this PR

+----------------+------------+
| TRACK-PROPERTY | VALUE      |
+----------------+------------+
| Title          | Some title |
| ID             | 12345678   |
| Album          | Some album |
| Version        | None       |
| Explicit       | False      |
| Max-Q          | LOSSLESS   |
| Get-Q          | HIGH       |
| Get-Codec      | mp4a.40.2  |
+----------------+------------+
100%|▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓| 11.57/11.57 MB
[ERR] DL Track[Some title] failed.'./download//Playlist/a playlist [playlist id]/01 - Artist - Some title.flac' is not a valid FLAC file

when using master branch the files are correctly saved as .m4a and there's no error message

any ideas?

thanks, wiktor

noname77 avatar Jun 17 '23 12:06 noname77

I poked around and it turned out that event though the requested stream had correct quality set, the track api response was set to LOSLESS regardless

here's my quick and dirty fix

TIDALDL-PY % git status
On branch lobra
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   tidal_dl/tidal.py
% git diff TIDALDL-PY/tidal_dl/tidal.py
diff --git a/TIDALDL-PY/tidal_dl/tidal.py b/TIDALDL-PY/tidal_dl/tidal.py
index b00e06b..2d872f3 100644
--- a/TIDALDL-PY/tidal_dl/tidal.py
+++ b/TIDALDL-PY/tidal_dl/tidal.py
@@ -265,7 +265,12 @@ class TidalAPI(object):
         videos = []
         for item in data:
             if item['type'] == 'track' and item['item']['streamReady']:
-                tracks.append(aigpy.model.dictToModel(item['item'], Track()))
+                # print(f"track model: {item['item']}")
+                # fix audio quality
+                track = aigpy.model.dictToModel(item['item'], Track())
+                track.audioQuality = self.getQualityStr(SETTINGS.audioQuality)
+                tracks.append(track)
+
             else:
                 videos.append(aigpy.model.dictToModel(item['item'], Video()))
         return tracks, videos
@@ -280,7 +285,7 @@ class TidalAPI(object):
         albums += list(aigpy.model.dictToModel(item, Album()) for item in data)
         return albums
 
-    def getStreamUrl(self, id, quality: AudioQuality):
+    def getQualityStr(self, quality: AudioQuality) -> str:
         squality = "HI_RES"
         if quality == AudioQuality.Normal:
             squality = "LOW"
@@ -289,6 +294,11 @@ class TidalAPI(object):
         elif quality == AudioQuality.HiFi:
             squality = "LOSSLESS"
 
+        return squality
+
+    def getStreamUrl(self, id, quality: AudioQuality):
+        squality = self.getQualityStr(quality)
+
         paras = {"audioquality": squality, "playbackmode": "STREAM", "assetpresentation": "FULL"}
         data = self.__get__(f'tracks/{str(id)}/playbackinfopostpaywall', paras)
         resp = aigpy.model.dictToModel(data, StreamRespond())

noname77 avatar Jun 17 '23 13:06 noname77