After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 789601 - Fetching cover art with cover art search plugin doesn't work
Fetching cover art with cover art search plugin doesn't work
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Plugins (other)
3.4.x
Other Linux
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-28 20:41 UTC by christophe.vda
Modified: 2017-11-01 07:03 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description christophe.vda 2017-10-28 20:41:48 UTC
On an updated Arch linux, I don't get a cover art image when I try the fetch button in the Album Art tab. I am able to add a local image with the Browse button, but Fetch doesn't seem to work. I tested this for a .flac file of a rather well known album by The Black Keys called Brothers. I don't have a lot of experience posting bugs, so I will follow your directions on what output to post here.
Comment 1 André Klapper 2017-10-31 09:53:10 UTC

*** This bug has been marked as a duplicate of bug 783140 ***
Comment 2 gkrithi8 2017-10-31 11:56:54 UTC
(In reply to André Klapper from comment #1)
> 
> *** This bug has been marked as a duplicate of bug 783140 ***

Not sure if that is correct. The bug reporter has mentioned that the FLAC file has valid 'Album' name, and still cover art doesn't load, which is actually a bug.

Bug 783140 is about "Unknown" / empty album names, which is not an actual bug with the plugin, but some missing 'error handling' code. as nothing much can be done with empty 'Album' names by artsearch plugin.
Comment 3 gkrithi8 2017-10-31 19:00:29 UTC
Just a quick question. Why do we need Last.fm account for search api ? I removed the below check, and the cover art loaded fine.

--- a/plugins/artsearch/lastfm.py
+++ b/plugins/artsearch/lastfm.py
@@ -151,7 +151,7 @@ class LastFMSearch (object):
                        callback (args)
                        return
 
-               if user_has_account() == False:
+               if False:
                        print("can't search: no last.fm account details")
                        callback (args)
                        return
Comment 4 christophe.vda 2017-10-31 19:31:28 UTC
(In reply to gkrithi8 from comment #3)
> Just a quick question. Why do we need Last.fm account for search api ? I
> removed the below check, and the cover art loaded fine.
> 
> --- a/plugins/artsearch/lastfm.py
> +++ b/plugins/artsearch/lastfm.py
> @@ -151,7 +151,7 @@ class LastFMSearch (object):
>                         callback (args)
>                         return
>  
> -               if user_has_account() == False:
> +               if False:
>                         print("can't search: no last.fm account details")
>                         callback (args)
>                         return

I have no knowledge of the code of this project, but I do not have a Last.fm account and do not use that plugin. So that might be the cause of my problem!
Comment 5 Jonathan Matthew 2017-10-31 20:33:27 UTC
(In reply to gkrithi8 from comment #3)
> Just a quick question. Why do we need Last.fm account for search api ? I
> removed the below check, and the cover art loaded fine.

in order to comply with last.fm terms of service.
Comment 6 christophe.vda 2017-10-31 20:53:33 UTC
(In reply to Jonathan Matthew from comment #5)
> (In reply to gkrithi8 from comment #3)
> > Just a quick question. Why do we need Last.fm account for search api ? I
> > removed the below check, and the cover art loaded fine.
> 
> in order to comply with last.fm terms of service.

So if I understand it correctly, Cover Art Search plugin needs the Last.fm plugin to be turned on and the user to be logged in to a Last.fm account? I use linux for privacy reasons, so I won't make a Last.fm account. Is it possible to legally use some other database than Last.fm, like Cover Art Archive from Musicbrainz? This would of course need a lot of new code so I understand if it is not a priority.
Comment 7 Jonathan Matthew 2017-10-31 21:07:23 UTC
Of course it's possible to use other services.  Using coverartarchive.org would probably be a relatively simple addition to the existing musicbrainz search code, assuming its terms of service are acceptable and it won't block rhythmbox users to reduce load on its systems, as other services have done in the past.
Comment 8 gkrithi8 2017-10-31 21:24:41 UTC
(In reply to Jonathan Matthew from comment #7)
> Of course it's possible to use other services.  Using coverartarchive.org
> would probably be a relatively simple addition to the existing musicbrainz
> search code, assuming its terms of service are acceptable and it won't block
> rhythmbox users to reduce load on its systems, as other services have done
> in the past.

From: https://musicbrainz.org/doc/Cover_Art_Archive/API#Rate_limiting_rules

Rate limiting rules:

There are currently no rate limiting rules in place at http://coverartarchive.org.
Comment 9 gkrithi8 2017-10-31 21:32:42 UTC
From: https://www.discogs.com/developers/

Rate Limiting:

Requests are throttled by the server by source IP to 60 per minute for authenticated requests, and 25 per minute for unauthenticated requests, with some exceptions.
Comment 10 Jonathan Matthew 2017-10-31 21:39:18 UTC
(In reply to gkrithi8 from comment #8)
> From: https://musicbrainz.org/doc/Cover_Art_Archive/API#Rate_limiting_rules
> 
> Rate limiting rules:
> 
> There are currently no rate limiting rules in place at
> http://coverartarchive.org.

Which means they haven't thought about it and probably won't do a good job of it when they're forced to.

(In reply to gkrithi8 from comment #9)
> From: https://www.discogs.com/developers/
> 
> Rate Limiting:
> 
> Requests are throttled by the server by source IP to 60 per minute for
> authenticated requests, and 25 per minute for unauthenticated requests, with
> some exceptions.

https://git.gnome.org/browse/rhythmbox/commit/?id=eb3ec4c513e3b7427db6f3c47f556a6182729bed

I'm not going to try that again.
Comment 11 christophe.vda 2017-10-31 21:45:55 UTC
I have been quickly looking over the code and I see that Last.fm is only one of multiple sources being searched. Musicbrainz is allready implemented it seems and this is essentially cover art archive, so ignore my last comment.

Weirdly, after fidgeting around with the different plugin settings, it suddenly works perfectly! I activated Last.fm plugin, clicked on the Last.fm section in RB but did not login and now fetching works, even when Last.fm plugin is disabled. I honestly have no idea if this has anything to do with the (short-lived) bug I encountered.

On a side note, it seems that the album cover is not stored in the file itself, as other apps do not show it. Is that intentional or should I file another bug?
Comment 12 gkrithi8 2017-10-31 22:03:23 UTC
(In reply to Jonathan Matthew from comment #10)

> > Requests are throttled by the server by source IP to 60 per minute for
> > authenticated requests, and 25 per minute for unauthenticated requests, with
> > some exceptions.
> 
> https://git.gnome.org/browse/rhythmbox/commit/
> ?id=eb3ec4c513e3b7427db6f3c47f556a6182729bed
> 
> I'm not going to try that again.

1. I don't understand how would rhythmbox get blocked, and not every other application which uses the same API.

2. Also, the requests are tracked by client IP, which should give more bandwidth per rhythmbox user. 60 per minute and 25 per minute are more than enough, I guess.

3. As per https://www.discogs.com/developers/#page:home,header:home-rate-limiting:

We attach the following headers to responses to help you track your rate limit use:

X-Discogs-Ratelimit: The total number of requests you can make in a one minute window.
X-Discogs-Ratelimit-Used : The number of requests you’ve made in your existing rate limit window.
X-Discogs-Ratelimit-Remaining: The number of remaining requests you are able to make in the existing rate limit window.

With the above response headers, can't we rate limit rhythmbox for a while before submitting the request.
Comment 13 gkrithi8 2017-10-31 22:09:53 UTC
(In reply to christophe.vda from comment #11)
> I have been quickly looking over the code and I see that Last.fm is only one
> of multiple sources being searched. Musicbrainz is allready implemented it
> seems and this is essentially cover art archive, so ignore my last comment.

The current musicbrainz search code uses Amazon image URL ( which is kind of broken ) for image backend, and not coverartarchive.
Comment 14 gkrithi8 2017-11-01 07:03:59 UTC
(In reply to gkrithi8 from comment #12)

The reason for asking this is that some API's rate limit based on a user account. That would be a global rate limit ( few hundred / thousand requests per day per free account ). In that case, an app would not have much choice of rate limiting. But, here it doesn't seem to be the case.

Sorry, but am I missing something here.