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 529873 - The artdisplay plugin should be able to supply metadata for arbitrary db entries
The artdisplay plugin should be able to supply metadata for arbitrary db entries
Status: RESOLVED FIXED
Product: rhythmbox
Classification: Other
Component: Plugins (other)
0.11.x
Other All
: Normal normal
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-04-25 11:51 UTC by Niels Vorgaard Christensen
Modified: 2009-07-06 21:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Proposed patch (1.18 KB, patch)
2008-04-26 09:09 UTC, Niels Vorgaard Christensen
committed Details | Review

Description Niels Vorgaard Christensen 2008-04-25 11:51:32 UTC
Please describe the problem:
The artdisplay plugin will only return metadata for the current entry on the entry-extra-metadata-request signals, and it will only emit entry-extra-metadata-notify signals when current entry is changed. Other plugins should be able to request and recieve notifications about metadata for arbitrary entries.

Steps to reproduce:


Actual results:


Expected results:


Does this happen every time?


Other information:
Comment 1 Niels Vorgaard Christensen 2008-04-26 09:09:15 UTC
Created attachment 109946 [details] [review]
Proposed patch

This tries to fix matters for the rb:coverArt tag. This is important for the iPod plugin.
Comment 2 Antonio Frediani 2008-05-04 16:24:34 UTC
(In reply to comment #1)
> Created an attachment (id=109946) [edit]
> Proposed patch
> 
> This tries to fix matters for the rb:coverArt tag. This is important for the
> iPod plugin.
> 

I lack the knowledge for a proper review, but this patch fixed the loading of covers on my iPod Nano 3g.

Thank you.
Comment 3 Paul Drain 2008-05-06 05:52:56 UTC
Works for me too -- puts the cover that is embedded into the song (see bug #345975) onto the ipod correctly for a video 5g and nano 3g.
Comment 4 Joshua Abell 2008-05-07 17:25:50 UTC
How can I apply this patch?
Comment 5 Adrien Asséré 2008-05-23 13:25:32 UTC
1. Install patch package (with synaptic)
2. Download art-metadata.patch in your /home/username/ directory

In the console :
3. sudo cp art-metadata.patch /usr/lib/rhythmbox/plugings/artdisplay/
4. cd /usr/lib/rhythmbox/plugings/artdisplay/
5. patch -p0 < art-metadata.patch

And it should be OK
Comment 6 dan 2008-06-14 10:36:09 UTC
No success for me :( Using nano 2g.
Comment 7 Jonathan Matthew 2008-07-01 13:27:50 UTC
This is a good idea, clearly, but I'm concerned that it would result in it issuing  cover art search requests too quickly.  We probably need to add a request queue with some simple rate limiting.
Comment 8 Colin Leroy 2008-07-01 14:34:11 UTC
(In reply to comment #7)
> This is a good idea, clearly, but I'm concerned that it would result in it
> issuing  cover art search requests too quickly.  We probably need to add a
> request queue with some simple rate limiting.

Indeed, it does. Looking at Wireshark's output when transferring a 12-track album to the ipod, I see there are 12 requests to Amazon and 12 (identical) answers.

Rate limiting would prevent hammering the server, but also, I think queuing requests so that lookup isn't started until the previous one finished would allow to make only one HTTP request per album (the next ones would be fetched from the local cache).

Unfortunately, I suck so much at Python that I won't be able to implement that.
Comment 9 Jonathan Matthew 2009-03-21 23:48:47 UTC
After this change:

2009-03-22  Jonathan Matthew  <jonathan@d14n.org>
                
        * plugins/artdisplay/artdisplay/CoverArtDatabase.py:
        Make the ticket system code a bit less inscrutable, remove items from
        the hash when the last ticket is forgotten, and add a method to search
        for existing items with live tickets that match a new item.

        When a new cover art search request arrives, look for already running
        searches for the same artist and album.  If an existing search is 
        found, just wait for the results rather than doing the same search 
        again.  From #529873.

we should only get one search request for each album (excluding compilations, which we can't handle correctly yet).  I'd still like to add a rate limit for the amazon search, since as far as I can tell, amazon has a loosely applied limit of one request per second per client IP address.

I can't test the behaviour of this patch as I don't have an ipod.  If someone reports that it does result in a single search per album, and the artwork still ends up on the ipod, I'll commit the patch here.
Comment 10 Christophe Fergeau 2009-03-22 18:45:10 UTC
I made a quick test, I recompiled rb trunk, applied the patch from this bug, transferred an album from rb library to the ipod and there was only 1 request to last.fm that I could see using wireshark. However, the artwork isn't shown on the ipod, but that's probably a different issue.
Comment 11 Jonathan Matthew 2009-03-27 00:09:21 UTC
Sounds like the patch is doing what it's supposed to, but there's a problem somewhere else.  Hopefully we can find that soon..

2009-03-27  Jonathan Matthew  <jonathan@d14n.org>

        patch by:  Niels Vorgaard Christensen  <vorgaard.c@math.ku.dk>

        * plugins/artdisplay/artdisplay/__init__.py:
        Answer cover art requests for any entry, not just the current playing
        one.  Fixes #529873.
Comment 12 Juan Rial 2009-07-06 21:58:59 UTC
(In reply to comment #5)
> 1. Install patch package (with synaptic)
> 2. Download art-metadata.patch in your /home/username/ directory
> 
> In the console :
> 3. sudo cp art-metadata.patch /usr/lib/rhythmbox/plugings/artdisplay/
> 4. cd /usr/lib/rhythmbox/plugings/artdisplay/
> 5. patch -p0 < art-metadata.patch
> 
> And it should be OK
> 

Thanks for these instructions, but please doublecheck next time. I find that in these cases it's best to apply the patch again (on a restored source file if needed), and copy/paste straight from your console instead of trying to remember what you did and typing it in manually. Less typos, and you know your instructions are correct.

Corrected instructions for step 3-5:

3. sudo cp art-metadata.patch /usr/lib/rhythmbox/plugins/
4. cd /usr/lib/rhythmbox/plugins/
5. sudo patch -p0 < art-metadata.patch