search

tehkillerbee / mopidy-tidal

Tidal Backend plugin for Mopidy
Apache License 2.0
93 stars 28 forks source link

Support for python-tidal 0.7.x #68

Closed blacklight closed 2 years ago

blacklight commented 2 years ago

There are still some details to iron out, but most of the features seem to work (tested with ncmpcpp, Iris and Platypush).

Some observations:

The problem with not using the new get_items function is that limit is null by default, and that results in the upstream API falling back on its default (and that's 1000 items max).

Of course, I could submit another PR to python-tidal that properly implements pagination for all the items that support it, but I don't know if/when it will be merged, and I don't feel like we should depend too much on python-tidal to do things for us at this point.

So if you folks agree, I'd go for implementing pagination on mopidy-tidal's side. It's not that difficult, it's just a matter of copying the get_items function from python-tidal to this side. We would then call it for all the API items where it makes sense - playlists most of all, maybe also albums?

The advantage of copying that logic on this side is that we have better control and we can easily do things like parallelization regardless of how python-tidal implements pagination upstream. My idea would be to initialize a pool of workers in get_items, each with a limit/offset parameter, so requests on large collections can be much faster by processing chunks in parallel.

blacklight commented 2 years ago

FYI moods aren't yet supported on the 0.7.x, so I'll skip their implementation for now. Accessing moods will raise an exception for now.

session.user.favourites.playlists() is also apparently broken (tidal:my_playlists URI), but playlists can still be browsed through lsplaylists.

blacklight commented 2 years ago

I have pushed a new commit that should wrap up most of the things. I don't get any more errors from Iris nor ncmpcpp - except for moods, that haven't been implemented on python-tidal 0.7.x yet.

Now we also have proper support for albums/tracks release dates btw.

Also, it adds a workers.get_items method that performs proper pagination with multitask support - it basically uses multiprocessing.Pool to parallelize a method, with each function instance that takes a chunk with limit..offset as arguments.

This should boost performance for some of the large queries, but I can't boost it that much yet for single track lookups: when a playlist is loaded, both Iris and ncmpcpp perform the lookup track by track, instead of doing it in one batch. This is a design decision that has always horrified me, and it should be fixed in the upstream clients.

blacklight commented 2 years ago

FYI I've just noticed that free-text search is broken. Working on it.

blacklight commented 2 years ago

I have fixed the issues with search on 0.7.x. I have been using this branch for a couple of days so far on multiple devices and, besides from search (and moods that are gone, but mostly because they haven't (yet?) been implemented on 0.7.x), I haven't experienced any issues.

Performance is also better, especially on playlists lookup, as we now have our own logic with a multiprocessing.Pool to process API requests with limit..offset in parallel, as well as look up playlists in parallel. And the issue with unreported albums/tracks dates should also be solved. I'd also like to implement support for playlist tracks count without having to download the whole playlist first, now that 0.7.x exposes num_tracks as a playlist attribute.

@tehkillerbee @fmarzocca feel free to test this branch at your convenience.

blacklight commented 2 years ago

I have noticed a bug with the search implementation and I'm on my way to fix it in this PR.

This is the current logic. It basically iterates over all the search fields (artist, album, track etc.) and it spawns for each of them a separate thread that searches for the specific field=value. Not only, but if exact=False then we spawn, for each of the search fields, THREE different threads - one to search for artists, one for albums, and one for tracks - but all of them actually search for the same term.

Not only, but the for loop stops after searching for the first field, and then it joins together the search results of artists, albums and tracks.

This is quite inefficient and messy. If we are specifically searching for a track by artist and title, it'll end up returning all the tracks from that artist too, as well as all the albums that match the search query.

The Tidal API doesn't provide a mechanism for structured search - you can't search by artist, album etc., you just submit a search query. My proposal is to send a query formatted as {artist} {album} {title} (where each of those fields can be unset), and searching in order of specificity - so if a track title is specified we search for tracks, otherwise if album is set we search for albums, otherwise we search for artists. This doesn't impact the any case, where we just specify no list of models.

blacklight commented 2 years ago

@tehkillerbee the last commit contains a rewrite of the search package. I have removed a lot of duplicate code, improved search accuracy (if I search for artist and album I won't get anymore other unrelated tracks from the same artists, or albums by other artists with similar names) and speed (tracks are now expanded through a pool of workers, no need to spawn different threads to search the API with different fields, and the overall number of API calls has been reduced).

tehkillerbee commented 2 years ago

@BlackLight Thanks for the update and great to hear you also took time to improve the search functionality and speed - I have always been annoyed by the subpar search results but never got around to do something about it :)

I will switch to your branch asap and start testing.

tehkillerbee commented 2 years ago

@BlackLight Initial tests shows a few issues. I will continue testing and perhaps suggest some quick fixes.

It also seems images are not displayed at all when browsing albums.

I have tested on a different PC this time so perhaps that is the cause of some of the issues I see. I will test again later today.

blacklight commented 2 years ago

OAuth session cannot load, it seems. Only solution is to login again using the URL. The json file is created and appears to contain the relevant json fields.

I also had OAuth session restoring issues on one of my devices after switching to 0.7.x, but on all the others it worked fine. In any case, the code should already catch the exception and return a login link - maybe it's worth adding to a "migration" section in the documentation?

When browsing artists, no art is shown and an error is thrown:

This should be fixed in the latest version. The API seems to support different sizes for different object types (750 for artists, 640 for albums, 480 for most of the items etc.). Instead of figuring it out from the type of object, I have now implemented a single try/catch logic that tries with all the sizes that we want, in decreasing order.

As a bonus, images lookup logic now happens in parallel, so it's ~4x faster.

fmarzocca commented 2 years ago

@tehkillerbee @fmarzocca feel free to test this branch at your convenience.

Sorry both of you, but I was offline for a long business trip.

Can you pls give me instructions on what version/branch I have to install (both python-tidal and mopidy-tidal)? I will be glad to test.

blacklight commented 2 years ago

@fmarzocca the mopidy-tidal branch that supports 0.7 is python-tidal-0.7-migration.

The vanilla 0.7.x branch for python-tidal should work out of the box. Most of my PRs have already been accepted into the branch, but if you have issues with pagination of long collections you may try my dev branch.

tehkillerbee commented 2 years ago

@BlackLight FYI, I tried restarting mopidy after re-authentication but the same issue occurred. I will test again with your latest changes ASAP.

blacklight commented 2 years ago

@tehkillerbee it seems indeed that sometimes it may be required to remove the stored OAuth credentials file upon upgrade. I'm not sure why it happens. I have tried the code on 6 devices, I encountered this issue on three of them, but the other half just did fine with the existing OAuth JSON file. I'm not sure if anything has changed on the authentication side in python-tidal.

tehkillerbee commented 2 years ago

@BlackLight Regarding upgrade detection; we could consider adding version info in the OAuth file - and delete/reauth it if version does not match.

blacklight commented 2 years ago

Sure, that can be an option if there was a clear pattern (i.e. logins with old OAuth credentials systematically don't work). But in my case only half of the devices were affected by this issue and the others could use the existing credentials file without problems. So I'm not entirely sure of what the issue could be.

fmarzocca commented 2 years ago

I have updated mopidy-tidal to the branch python-tidal-0.7-migration, then I updated python-tidal to 0.7.xbranch.

Restarted mopidy, I had to re-Auth it. And from now on, EVERY time I restart mopidy I need to re-Auth. Issue?

Jul 17 10:47:19 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. Quality = LOSSLESS
Jul 17 10:47:19 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. using default client id & client secret from python-tidal
Jul 17 10:47:19 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Loading OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json...
Jul 17 10:47:20 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Could not load OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json
Jul 17 10:47:20 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Creating new OAuth session...
Jul 17 10:47:20 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend Visit link.tidal.com/JODIX to log in, the code will expire in 300 seconds
Jul 17 10:48:07 pab mopidy[1245]: INFO     [TidalBackend-6] mopidy_tidal.backend TIDAL Login OK
Jul 17 10:48:28 pab mopidy[1300]: INFO     [MainThread] mopidy.commands Starting Mopidy backends: FileBackend, M3UBackend, StreamBackend, TidalBackend, LocalBackend, CdBackend, TuneInBackend, SpotifyBackend, SoundCloudBackend, PodcastBackend, iTunesPodcastBackend, InternetArchiveBackend
Jul 17 10:48:28 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. Quality = LOSSLESS
Jul 17 10:48:28 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. using default client id & client secret from python-tidal
Jul 17 10:48:28 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Loading OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json...
Jul 17 10:48:29 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Could not load OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json
Jul 17 10:48:29 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Creating new OAuth session...
Jul 17 10:48:29 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend Visit link.tidal.com/PCTXX to log in, the code will expire in 300 seconds
Jul 17 10:48:52 pab mopidy[1300]: INFO     [TidalBackend-6] mopidy_tidal.backend TIDAL Login OK
Jul 17 10:49:14 pab mopidy[1367]: INFO     [MainThread] mopidy.commands Starting Mopidy backends: FileBackend, M3UBackend, StreamBackend, TidalBackend, LocalBackend, CdBackend, TuneInBackend, SpotifyBackend, SoundCloudBackend, PodcastBackend, iTunesPodcastBackend, InternetArchiveBackend
Jul 17 10:49:14 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. Quality = LOSSLESS
Jul 17 10:49:14 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Connecting to TIDAL.. using default client id & client secret from python-tidal
Jul 17 10:49:14 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Loading OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json...
Jul 17 10:49:15 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Could not load OAuth session from /var/lib/mopidy/tidal/tidal-oauth.json
Jul 17 10:49:15 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Creating new OAuth session...
Jul 17 10:49:15 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend Visit link.tidal.com/NGGBK to log in, the code will expire in 300 seconds
Jul 17 10:49:38 pab mopidy[1367]: INFO     [TidalBackend-6] mopidy_tidal.backend TIDAL Login OK
fmarzocca commented 2 years ago

After few minutes of playing, mopidy hangs up, Iris no more reachable, without any error in log. Now way to stop mopidy service, I have to reboot the Raspi.

mopidy     961   495  0 11:36 ?        00:00:00 [mopidy] <defunct>
mopidy     962   495  0 11:36 ?        00:00:00 [mopidy] <defunct>
mopidy     963   495  0 11:36 ?        00:00:00 [mopidy] <defunct>

Reinstalling mopidy-tidal and python-tidal

Name: tidalapi
Version: 0.7.0rc1
Summary: Unofficial API for TIDAL music streaming service.
Home-page: https://github.com/tamland/python-tidal
Author: Thomas Amland
Author-email: thomas.amland@googlemail.com
License: LGPL
Location: /usr/local/lib/python3.7/dist-packages
Requires: python-dateutil, requests
Required-by: Mopidy-Tidal
Name: Mopidy-Tidal
Version: 0.2.8
Summary: Tidal music service integration
Home-page: https://github.com/tehkillerbee/mopidy-tidal
Author: Johannes Linde
Author-email: josaksel.dk@gmail.com
License: Apache License, Version 2.0
Location: /usr/local/lib/python3.7/dist-packages
Requires: Mopidy, Pykka, requests, setuptools, tidalapi
Required-by:
fmarzocca commented 2 years ago

No way, I can't use my device.

I'm sorry but I had to switch back to better-cache branch of mopidy-tidal and to 0.6.x of python-tidal, which I am using from months, without any problem at all.

blacklight commented 2 years ago

  1. Authentication: did you try to remove the existing file before restarting mopidy? If not, then the issue may require further investigation. Nothing has changed on the authentication side on mopidy-tidal in this PR, so you could maybe poke the python-tidal developer before he releases 0.7.x.

  2. Mopidy hanging: I have also experienced this issue, and I have solved it by disabling Iris - at least for now. Once Iris is disabled, everything runs smooth. I'm really not sure of what on the new mopidy-tidal branch causes Iris to hang - the only clue is that it could be related to multiprocessing.Pool now used to retrieve results in parallel clashing with Tornado's threads. The problem is that when it happens there are no log lines either to point to any obvious problems, not even in debug mode.

fmarzocca commented 2 years ago
  1. Authentication: did you try to remove the existing file before restarting mopidy?

No, I didn't.

2. Once Iris is disabled, everything runs smooth.

I didn't try to disable Iris, but this is an important issue. Not only Iris is blocked, but mopidy itself too. It is no more responding to commands.

tehkillerbee commented 2 years ago

@fmarzocca Yes, this was the issue I experienced too. Each time I restart mopidy, I am requested to reauthenticate. After reauth, everything appears to work although I did not stream music for many hours. I did use Iris and I will continue testing it and report my findings.

blacklight commented 2 years ago

The re-authentication issue should now be solved. It turns out that the signature of session.load_oauth_session has changed in 0.7 - it used to include session_id, but now it doesn't.

I have pushed a fix for it that checks the tidalapi version and passes the expected arguments.

I'm now taking a look into the mopidy freezes that occur when both mopidy-iris and mopidy-tidal are running.

The top candidate for the culprit may be the multiprocessing.Pool that I'm now using to search and paginate in parallel workers - using multiprocessing can mess things with Tornado, which uses its own single-process event pool.

If I'm correct then using concurrent.futures.ThreadPoolExecutor instead of multiprocessing.Pool should fix things.

blacklight commented 2 years ago

I have intensively tested the branch after the new changes for the past two days over several devices. No more re-authentication issues and no more application deadlocks when Iris is enabled. I guess that my suspicion that Tornado (used by Iris) and multiprocessing.Pool (used by the new parallelized pagination logic) weren't exactly best friends was true, because replacing processes with threads stopped the hanging issues. At some point I'd like to investigate the reason for these deadlocks, but for now I'm just happy that things work.

@tehkillerbee @fmarzocca feel free to test the branch after the new changes at your convenience.

tehkillerbee commented 2 years ago

Great work. I will test it on my mopidy clients tomorrow and see if it's working as expected.

fmarzocca commented 2 years ago

I've been trying it for a couple of hours now and it works great. No authentication issues and no deadlocks! Good job, thanks! If I'm not wrong, now installing mopidy-tidal it also installs tidalapi v.0.7.x, no need for a separate installation.

fmarzocca commented 2 years ago

After one full day of testing, I can confirm my previous report: very good.

blacklight commented 2 years ago

If I'm not wrong, now installing mopidy-tidal it also installs tidalapi v.0.7.x, no need for a separate installation.

That's correct, but it's a temporary solution. When the 0.7.x branch is merged we'll also need to change the setup requirements.

Also, no custom fork of python-tidal is required now. All of my PRs have been merged so the 0.7.x branch is up to date with our requirements. Support for pagination is still buggy and half-baked in python-tidal though, but that's fine - I have moved the pagination logic to this repo so we won't have to wait for python-tidal to fix it on their end.

blacklight commented 2 years ago

I have added a few more performance improvements:

As a result of these improvements, playlists lookup should be much faster - lsplaylists takes < 10 seconds on my account on a fresh start, and I have tons of playlists with tons of tracks. Any following call takes ~ 1 second.

tehkillerbee commented 2 years ago

@BlackLight Great work. Sync appears very, very fast indeed. Number of tracks is also displayed. Album sync took a long time the first time I manually clicked refresh in Iris but afterwards, it was very fast. Not sure why.

Authentication works great, no problems here.

Bugs:

FYI, I made sure to clear cache and install latest mopidy_tidal from this branch.

tehkillerbee commented 2 years ago

@BlackLight Browsing using a different frontend (in this case, I am using mopidy_mobile) appears to be broken:

Browsing My Albums, My Playlists, My Artists, My Tracks appears to work works fine but when browsing individual albums, artists, playlists, an error is thrown (see below). When browsing tracks, all tracks are displayed as usual.

Aug 09 09:29:00 jlinde mopidy[341935]: INFO     [TidalBackend-6] mopidy_tidal.library Browsing uri tidal:artist:3567061
Aug 09 09:29:00 jlinde mopidy[341935]: ERROR    [Core-7] mopidy.core.library TidalBackend backend caused an exception.
Aug 09 09:29:00 jlinde mopidy[341935]: Traceback (most recent call last):
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 17, in _backend_error_handling
Aug 09 09:29:00 jlinde mopidy[341935]:     yield
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 114, in _browse
Aug 09 09:29:00 jlinde mopidy[341935]:     result = backend.library.browse(uri).get()
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_threading.py", line 45, in get
Aug 09 09:29:00 jlinde mopidy[341935]:     _compat.reraise(*self._data['exc_info'])
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_compat/__init__.py", line 29, in reraise
Aug 09 09:29:00 jlinde mopidy[341935]:     raise value
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 193, in _actor_loop
Aug 09 09:29:00 jlinde mopidy[341935]:     response = self._handle_receive(envelope.message)
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 299, in _handle_receive
Aug 09 09:29:00 jlinde mopidy[341935]:     return callee(*message.args, **message.kwargs)
Aug 09 09:29:00 jlinde mopidy[341935]:   File "/usr/local/lib/python3.8/dist-packages/mopidy_tidal/library.py", line 224, in browse
Aug 09 09:29:00 jlinde mopidy[341935]:     session.get_artist_albums(parts[2]))
Aug 09 09:29:00 jlinde mopidy[341935]: AttributeError: 'Session' object has no attribute 'get_artist_albums'
Aug 09 09:29:06 jlinde mopidy[341935]: INFO     [TidalBackend-6] mopidy_tidal.library Browsing uri tidal:album:122861878
Aug 09 09:29:06 jlinde mopidy[341935]: ERROR    [Core-7] mopidy.core.library TidalBackend backend caused an exception.
Aug 09 09:29:06 jlinde mopidy[341935]: Traceback (most recent call last):
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 17, in _backend_error_handling
Aug 09 09:29:06 jlinde mopidy[341935]:     yield
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 114, in _browse
Aug 09 09:29:06 jlinde mopidy[341935]:     result = backend.library.browse(uri).get()
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_threading.py", line 45, in get
Aug 09 09:29:06 jlinde mopidy[341935]:     _compat.reraise(*self._data['exc_info'])
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_compat/__init__.py", line 29, in reraise
Aug 09 09:29:06 jlinde mopidy[341935]:     raise value
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 193, in _actor_loop
Aug 09 09:29:06 jlinde mopidy[341935]:     response = self._handle_receive(envelope.message)
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 299, in _handle_receive
Aug 09 09:29:06 jlinde mopidy[341935]:     return callee(*message.args, **message.kwargs)
Aug 09 09:29:06 jlinde mopidy[341935]:   File "/usr/local/lib/python3.8/dist-packages/mopidy_tidal/library.py", line 219, in browse
Aug 09 09:29:06 jlinde mopidy[341935]:     session.get_album_tracks(parts[2]))
Aug 09 09:29:06 jlinde mopidy[341935]: AttributeError: 'Session' object has no attribute 'get_album_tracks'
Aug 09 09:29:29 jlinde mopidy[341935]: INFO     [TidalBackend-6] mopidy_tidal.library Browsing uri tidal:playlist:1a7d78db-6f0f-4bbd-8223-bdd8826f7d7c
Aug 09 09:29:29 jlinde mopidy[341935]: ERROR    [Core-7] mopidy.core.library TidalBackend backend caused an exception.
Aug 09 09:29:29 jlinde mopidy[341935]: Traceback (most recent call last):
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 17, in _backend_error_handling
Aug 09 09:29:29 jlinde mopidy[341935]:     yield
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/mopidy/core/library.py", line 114, in _browse
Aug 09 09:29:29 jlinde mopidy[341935]:     result = backend.library.browse(uri).get()
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_threading.py", line 45, in get
Aug 09 09:29:29 jlinde mopidy[341935]:     _compat.reraise(*self._data['exc_info'])
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_compat/__init__.py", line 29, in reraise
Aug 09 09:29:29 jlinde mopidy[341935]:     raise value
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 193, in _actor_loop
Aug 09 09:29:29 jlinde mopidy[341935]:     response = self._handle_receive(envelope.message)
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/lib/python3/dist-packages/pykka/_actor.py", line 299, in _handle_receive
Aug 09 09:29:29 jlinde mopidy[341935]:     return callee(*message.args, **message.kwargs)
Aug 09 09:29:29 jlinde mopidy[341935]:   File "/usr/local/lib/python3.8/dist-packages/mopidy_tidal/library.py", line 229, in browse
Aug 09 09:29:29 jlinde mopidy[341935]:     session.get_playlist_tracks(parts[2]))
Aug 09 09:29:29 jlinde mopidy[341935]: AttributeError: 'Session' object has no attribute 'get_playlist_tracks'

Issue should be easy to replicate when installing Mopidy-Mobile frontend

pip3 install Mopidy-Mobile
blacklight commented 2 years ago

@tehkillerbee

Browsing My Albums, My Playlists, My Artists, My Tracks appears to work works fine but when browsing individual albums, artists, playlists, an error is thrown (see below). When browsing tracks, all tracks are displayed as usual.

This should now be fixed. Different clients seem to trigger different paths in the code to retrieve the same things and not all the session method names had been updated.

Since I was working on that code, I have also:

  1. Fixed the support for genres and moods (moods in particular have only been added recently to the python-tidal branch)
  2. Added support for mixes (also newly added in 0.7.x)

Excessive syncing still causes JSONDecodeError

This would be a pain to solve. The error in this case it's most likely caused by an exceeded rate, but the code is actually failing because the API is returning HTML instead of a JSON - and, since we are failing in case of non-success HTTP codes, the API is actually even returning a 200 response. I don't want to go down the rabbit hole of parsing the HTML to infer if the error is caused by an exceeded rate (in that case probably we have to implement a retry-with-throttle logic), or by something else (e.g. server error, and in that case it's ok to fail).

Artist artwork and playlist artwork does not get displayed when browsing My playlists, My artists in Iris. The artist artwork gets displayed when browsing/refreshing each artist/playlist individually and then going back.

It actually gets displayed if you go through Browse -> Tidal, but not from the root level. Most likely, again, different paths in the code exist to basically retrieve the same thing, and we need to make sure that the references are always completely populated.

Manually clicking "Refresh" in My Playlists does nothing. Selecting a playlist and clicking "... menu -> Refresh" again works fine. This might be a problem, in case the user needs to sync all playlists manually if the first sync failed.

I tried and it works. In your case probably the extension probably had recently refreshed the playlists - there is now a 300 seconds timer upon new refresh to prevent calls to as_list triggered by playlist_modified from triggering a new refresh. But I should probably find a way to reset the event if a manual refresh event is received.

There's another problem with manual refresh though. When playlists are refreshed through Iris and then you click on one of them, you'll see that it contains a bunch of tracks with tidal:track:0:0:0 URI - basically the new mock tracks used to get the counters to display the right numbers, without having to retrieve all the tracks. It seems that something in the path there uses the cached playlist metadata instead of triggering a full playlist retrieval. Things work well if you open the playlists in Iris from -> Browse -> Tidal -> My Playlists instead of -> Playlists.

fmarzocca commented 2 years ago

Installed and tested latest version. The playlist refresh is really FAST! Great. I rarely use Iris (I use my own package) but I tested it too and I don't find any important issue. The first time all Playlists show "0 tracks", but as soon as you get access to them, the tracks are counted very fast, and from then on they all show the number of tracks. No errors in log.

tehkillerbee commented 2 years ago

Everything looks like its working as expected and VERY fast indeed! Really great work. All artwork works as expected (Although not from the root level, as you mentioned). The "Tidal -> My Playlists" directory was completely empty for me initially but I deleted the cache and then it worked fine.

Great that you fixed the issue with other frontends. Mopidy-mobile now works again. I have not tested any other frontends, as these are the ones I usually use.

Only issue I noticed was selecting individual mixes in "My Mixes" results in empty lists (0 tracks are listed when selecting a mix). Not sure why that is? EDIT: Mixes work in mopidy-mobile but not in Iris, for some reason.

I will update the other issue regarding Iris specific issues and mention the issues with browsing the root folders.

Excessive syncing should be mentioned in the README so the user knows how to deal with it. Naturally, the error message could be more descriptive but as you say, it is quite a bit of work to infer what caused the error. I've only seen it on a freshly configured system with no cache. For now, lets mention it as a known issue and tell the user to try again ;)

@BlackLight One thing I noticed when using the old version of tidalapi (i.e. using the existing version without upgrading):

Aug 10 17:44:13 rpimedia mopidy[6263]: Traceback (most recent call last):
Aug 10 17:44:13 rpimedia mopidy[6263]:   File "/usr/lib/python3/dist-packages/mopidy/__main__.py", line 129, in main
Aug 10 17:44:13 rpimedia mopidy[6263]:     extension.setup(registry)
Aug 10 17:44:13 rpimedia mopidy[6263]:   File "/usr/local/lib/python3.7/dist-packages/mopidy_tidal/__init__.py", line 36, in setup
Aug 10 17:44:13 rpimedia mopidy[6263]:     from .backend import TidalBackend
Aug 10 17:44:13 rpimedia mopidy[6263]:   File "/usr/local/lib/python3.7/dist-packages/mopidy_tidal/backend.py", line 11, in <module>
Aug 10 17:44:13 rpimedia mopidy[6263]:     from tidalapi import Config, Session, Quality, __version__ as tidal_api_version
Aug 10 17:44:13 rpimedia mopidy[6263]: ImportError: cannot import name '__version__' from 'tidalapi' (/usr/local/lib/python3.7/dist-packages/tidalapi-0.6.10-py3.7.egg/tidalapi/__init__.py)

Since version 0.7.x does not get installed automatically, it might be necessary to add a few steps to the README describing the steps needed to upgrade. Or we should change the dependency so versions below 0.7.x wont be accepted.

tehkillerbee commented 2 years ago

@fmarzocca Out of curiosity, what package do you use this plugin with?

fmarzocca commented 2 years ago

@fmarzocca Out of curiosity, what package do you use this plugin with?

I have a Node-Red server with which I am controlling my house devices. Last year I decided to embed Mopidy in it, too. So I developed a package (Mopiqtt) to handle Mopidy through MQTT messages.

This is the final result in the Dashboard (I am not using Node-Red's standard Dashboard, but I have developed my own with Vue and Quasar):

mopiqtt

blacklight commented 2 years ago

Mixes work in mopidy-mobile but not in Iris, for some reason.

This is fixed in https://github.com/tehkillerbee/mopidy-tidal/pull/68/commits/6966e41da3a5bd05b334ab8ed0d3be904e5731e2.

Apparently mopidy-iris uses core.playlists.lookup for everything that is exposed as a playlist, instead of core.library.lookup, and mixes were not exposed by the playlist provider.

When playlists are refreshed through Iris and then you click on one of them, you'll see that it contains a bunch of tracks with tidal:track:0:0:0 URI

This is fixed in https://github.com/tehkillerbee/mopidy-tidal/pull/68/commits/90e42b352ee7d1a7e53f49c83c9a93b369a87e35

The first time all Playlists show "0 tracks"

I'm well aware of this - at least from Browser -> Tidal -> My Playlist; everything works fine now if you access directly through Playlists. But I'm not sure of how to fix it. mopidy-iris uses core.playlists.as_list there, which returns a list of references, which in turn only contains uri and name. I'm not sure how it expects to retrieve the tracks count from references.

One thing I noticed when using the old version of tidalapi

How nice, they forgot to include the __version__ constant until now...well, this should now be fixed in https://github.com/tehkillerbee/mopidy-tidal/pull/68/commits/6e6a311f8f2a6689344ee3b3adcb98168bfab731

Since version 0.7.x does not get installed automatically, it might be necessary to add a few steps to the README describing the steps needed to upgrade.

The setup.py now explicitly specifies that we need the 0.7.x branch of python-tidal. At least in my case doing a pip install . from the extension's source folder automatically retrieved the right version of python-tidal. This should be considered temporary though - once the new branch gets merged into main we need to specify a minimum stable version for python-tidal. Which installation steps did you try to replicate a case where python-tidal 0.7 wasn't installed automatically?

Manually clicking "Refresh" in My Playlists does nothing.

This is still the case if the list has already been updated in the past 5 minutes. I've decided to leave it like this because mopidy-iris just calls mopidy.core.lookup("tidal:my_playlists") when a refresh is triggered. So there's no way of telling if it's a normal browse activity or a refresh action, and I don't want to increase the loading time every time one retrieves the list of playlists just to be consistent with mopidy-iris' refresh mechanism. If there was a way to tell whether a lookup of tidal:my_playlists is just the result of browse activity or a forced refresh, I'd be happy to implement a more consistent behaviour.

tehkillerbee commented 2 years ago

@fmarzocca Very cool! It looks like a good user experience too, probably a better WAF than using a mopidy-mobile android app to control the stereo ;)

@BlackLight I just tested everything again. Looks like Moods etc. works exactly as expected and syncing is really fast. Mixes appear empty in Iris (0 tracks when selecting each mix) but after refreshing manually, they are populated with tracks.. It looks like I need to refresh each time I select a new mix. I'll merge this PR anyways, since it will take a few days before I can process PR's again due to holidays.

Which installation steps did you try to replicate a case where python-tidal 0.7 wasn't installed automatically? When installing directly from your branch, the existing tidalapi version (i.e. 0.6.x) was used automatically, without upgrading. With the above change to setup.py, I think issue would be fixed and the correct version will always be installed. I think this is a better solution so we can move away from the 0.6.x releases once and for all.

fmarzocca commented 2 years ago

Today I had this issue. Restarting mopidy fixed it:

2022-08-17 19:00:33,164 INFO [489:TidalBackend-3] mopidy_tidal.backend: Creating new OAuth session...
2022-08-17 19:00:33,207 ERROR [489:SoundCloudBackend-9] mopidy_soundcloud.soundcloud: Invalid "auth_token" used for SoundCloud authentication!
2022-08-17 19:00:33,320 ERROR [489:TidalBackend-3] pykka: Unhandled exception in TidalBackend (urn:uuid:1b2c8de0-fd13-4e00-b116-edfb642658dc):
Traceback (most recent call last):
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 699, in urlopen
    httplib_response = self._make_request(
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 382, in _make_request
    self._validate_conn(conn)
  File "/usr/lib/python3/dist-packages/urllib3/connectionpool.py", line 1012, in _validate_conn
    conn.connect()
  File "/usr/lib/python3/dist-packages/urllib3/connection.py", line 411, in connect
    self.sock = ssl_wrap_socket(
  File "/usr/lib/python3/dist-packages/urllib3/util/ssl_.py", line 449, in ssl_wrap_socket
    ssl_sock = _ssl_wrap_socket_impl(
  File "/usr/lib/python3/dist-packages/urllib3/util/ssl_.py", line 493, in _ssl_wrap_socket_impl
    return ssl_context.wrap_socket(sock, server_hostname=server_hostname)
  File "/usr/lib/python3.9/ssl.py", line 500, in wrap_socket
    return self.sslsocket_class._create(
  File "/usr/lib/python3.9/ssl.py", line 1040, in _create
    self.do_handshake()
  File "/usr/lib/python3.9/ssl.py", line 1309, in do_handshake
    self._sslobj.do_handshake()
ssl.SSLCertVerificationError: [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed: certificate is not yet valid (_ssl.c:1123)