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 363694 - make cover/art configurable
make cover/art configurable
Status: RESOLVED OBSOLETE
Product: rhythmbox
Classification: Other
Component: Plugins (other)
0.9.6
Other Linux
: Normal enhancement
: ---
Assigned To: RhythmBox Maintainers
RhythmBox Maintainers
: 392160 424571 (view as bug list)
Depends on: 374653 398093 398102 398231 409673 410105 410125
Blocks: 384483 412853
 
 
Reported: 2006-10-20 15:46 UTC by Hubert Figuiere (:hub)
Modified: 2018-05-24 11:58 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
dnd-cover-art.patch (11.77 KB, patch)
2007-01-14 23:44 UTC, Ed Catmur
none Details | Review
dnd-cover-art.patch (4.92 KB, patch)
2007-01-18 18:36 UTC, Ed Catmur
none Details | Review
dnd-cover-art.patch (4.04 KB, patch)
2007-01-18 18:45 UTC, Ed Catmur
none Details | Review
dnd-cover-art.patch (4.35 KB, patch)
2007-01-19 00:53 UTC, Ed Catmur
none Details | Review
dnd-cover-art.patch (4.22 KB, patch)
2007-01-19 00:55 UTC, Ed Catmur
none Details | Review
dnd-cover-art.patch (4.22 KB, patch)
2007-01-19 00:56 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.patch (3.72 KB, patch)
2007-01-19 01:58 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.patch (3.72 KB, patch)
2007-01-19 02:15 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.patch (4.04 KB, patch)
2007-01-19 15:58 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.patch (4.02 KB, patch)
2007-01-20 13:46 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (16.65 KB, patch)
2007-01-20 13:49 UTC, Ed Catmur
none Details | Review
10namespace-gerror.patch (519 bytes, patch)
2007-01-21 18:12 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (16.88 KB, patch)
2007-01-21 19:40 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (16.89 KB, patch)
2007-01-21 21:40 UTC, Ed Catmur
none Details | Review
property pane with cover information (27.31 KB, image/jpeg)
2007-02-19 22:29 UTC, Christophe Dehais
  Details
30cover-manager.patch (22.86 KB, patch)
2007-02-20 04:11 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (23.68 KB, patch)
2007-02-20 19:20 UTC, Ed Catmur
none Details | Review
Intial layout of the file-dialog (69.34 KB, image/jpeg)
2007-02-20 20:39 UTC, Christophe Dehais
  Details
30cover-manager.patch (25.87 KB, patch)
2007-02-21 23:03 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (26.26 KB, patch)
2007-02-24 13:18 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (26.59 KB, patch)
2007-02-24 23:38 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (26.29 KB, patch)
2007-02-24 23:52 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (26.32 KB, patch)
2007-02-25 00:02 UTC, Ed Catmur
none Details | Review
30cover-manager.patch (30.57 KB, patch)
2007-02-25 20:26 UTC, Ed Catmur
none Details | Review
30cover-manager.patch.12 (30.57 KB, patch)
2007-02-28 23:31 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.5.patch (4.07 KB, patch)
2007-03-04 01:15 UTC, Ed Catmur
none Details | Review
30cover-manager.13.patch (30.83 KB, patch)
2007-03-04 01:16 UTC, Ed Catmur
none Details | Review
28dnd-cover-art.6.patch (4.07 KB, patch)
2007-06-24 20:37 UTC, Ed Catmur
committed Details | Review
30cover-manager.14.patch (30.75 KB, patch)
2007-06-24 20:37 UTC, Ed Catmur
none Details | Review
30cover-manager.15.patch (30.79 KB, patch)
2008-03-24 21:32 UTC, Ed Catmur
none Details | Review
30cover-manager.16.patch (31.12 KB, patch)
2008-04-06 23:42 UTC, Ed Catmur
none Details | Review
30cover-manager.17.patch (30.85 KB, patch)
2008-04-08 00:40 UTC, Ed Catmur
none Details | Review
30cover-manager.18.patch (30.84 KB, patch)
2008-04-08 23:18 UTC, Ed Catmur
none Details | Review

Description Hubert Figuiere (:hub) 2006-10-20 15:46:23 UTC
RB fetch the music art automatically. But often it gets it wrong and there is no way to change that. I think that there should be a "Cover Manager" like in amaroK
Comment 1 William Jon McCann 2006-10-20 15:56:05 UTC
Wouldn't just dragging the correct cover art onto the art display be enough?
Comment 2 Alex Lancaster 2006-10-20 16:37:27 UTC
If there wasn't an exact or close match (based on some definition of "close"), or if there was more than one match, this would be a gallery of covers that would pop-up from which you could select the correct one. Amarok has something like this (the "cover manager") and it's pretty useful.

I know that Gareth Murphy was working on something like this a while back.
Comment 3 Alex Lancaster 2006-10-21 04:50:30 UTC
Cc'ing Gareth on this bug.
Comment 4 Vincent Trouilliez 2006-12-19 04:31:45 UTC
(In reply to comment #1)
> Wouldn't just dragging the correct cover art onto the art display be enough?

I too am more than annoyed that half of my album return no cover, or worse, a non-sense/irrelevant cover. It really turns a nice plug-in into a useless gadget.

To fix this problem, dragging the correct pic straight onto RB's GUI would be sooo sweet.

This would cater for two use cases:

1) I go on the web, on some on-line shop that sells music CD's, and if I can find the correct cover, I just drag and drop it straight from the web browser onto RB, like "Muine" does.

2) I can't find the cover on the web, so I proceed to scan it myself, save it on my desktop, and then drag it again onto RB like in 1)

To make it easy for the user to handle covers, they should not be hidden in some oscure Gconf hidden folder and involve the use of any XML file (like the playlist). Instead, I think the following arrangement would be super easy for the user: there is a folder/directory for each and every album in the playlist. So, when RB starts playing a song, it just looks in the corresponding album folder/directory, if there is a picture file called, say, "cover.png" (let's standardize the file name to avoid problems in case the user puts more than one pic in the folder, for whatever good reason), then it displays this one, and doesn't even attempt to retrieve the cover from the net.
Loading the pic should be "fault tolerant". I mean, if the cover has been scanned manually, presumably the user resized it to whatever size/dimension RB requires, but if the pic has been dragged straight from a web browser, the picture is probably perfectly square, but sometimes it may be a little bit too small, sometimes a bit too large. So RB should resize it automatically/silently.

Obviously, all the ins and outs of the plug-ins should be documented in RB's help pages, so the user knows exactly how it all works.

I would rate this feature request as pretty important as it makes the plug-in "jsut works", henc eactually useful, unfortunately I don't have much cash to motivate the devs more than that ;o)
Comment 5 Alex Lancaster 2007-01-04 06:14:44 UTC
*** Bug 392160 has been marked as a duplicate of this bug. ***
Comment 6 Peter 2007-01-11 23:50:21 UTC
There is an (obsolete) patch on bug 307848 comment 71 which might provide some ideas for adding drag and drop support for covers.

Given bug 392160 which has been marked as duplicate, I assume that is the intent of this bug?

----------------------------------------------------------------------

Comment 4 is going a little off topic - I wonder which version Vincent was using?  But to answer a few of those questions:

> To make it easy for the user to handle covers, they should not be hidden
> in some oscure Gconf hidden folder ...

Covers which the plugin downloads itself in the following (hidden) location:

~/.gnome2/rhythmbox/covers

> ... and involve the use of any XML file (like the playlist). 

The rhythmbox database XML file is not used by the art display plugin.

> Instead, I think the following arrangement would be super easy for the
> user: there is a folder/directory for each and every album in the playlist.
> So, when RB starts playing a song, it just looks in the corresponding album
> folder/directory, if there is a picture file called, say, "cover.png"
> (let's standardize the file name to avoid problems in case the user puts
> more than one pic in the folder, for whatever good reason), then it displays
> this one, and doesn't even attempt to retrieve the cover from the net.

It does this already - assuming you store your library of music files in separate directories for each album.  It will accept several variations such as "Cover.jpg" which should cover most cases.

> Loading the pic should be "fault tolerant". I mean, if the cover has been
> scanned manually, presumably the user resized it to whatever size/dimension
> RB requires, but if the pic has been dragged straight from a web browser,
> the picture is probably perfectly square, but sometimes it may be a little
> bit too small, sometimes a bit too large. So RB should resize it
> automatically/silently.

The image is resized automatically when displayed - so I don't understand this question.
Comment 7 Vincent Trouilliez 2007-01-12 00:09:32 UTC
>is going a little off topic - I wonder which version Vincent was using? 

Hi, I am using 0.9.7 as found in Ubuntu Feisty.

> It does this already - assuming you store your library of music files in
> separate directories for each album.  It will accept several variations 
> such as "Cover.jpg" which should cover most cases.

Oh that's great ! I will fiddle with that soon ! :-)
I hope/assume that wen RB finds a cover in an album directory, it favours it rather any cover it might fetched automatically through the plug-in ? I will figure it out soon ;-)

>The image is resized automatically when displayed - 
> so I don't understand this question.

I noticed that sometimes when I switch album/covers, sometimes the covers "grows" a little bit (a few pixels) vertically, during the transition. 
So I suspect that not all album covers have exactly the same aspect ratio, and that RB only resizes the width of the pic, not the height, so if the aspect ration of two covers aren't exactly the same... it looks like the second covers expands vertically.
I am not a nitve English speaker, does any of the above make sense ?! If not, I can explain again, or make a screencast a last resort ! ;-)

--
Vince
Comment 8 Vincent Trouilliez 2007-01-12 00:21:48 UTC
Hmm yes, I just checked all the covers in  ~/.gnome2/rhythmbox/covers, and indeed many of them are slightly off the 1:1 aspect ratio, for instance

300x278 pixels
300x296
300x297
300x298
300x301
300x308


So in order to look tidy, RB should resize the height too. The resulting distortion will be un-noticeble to the naked eye but it will make transitions from  cover to cover, perfect everytime.
Comment 9 Alex Lancaster 2007-01-12 03:13:35 UTC
(In reply to comment #6)
> There is an (obsolete) patch on bug 307848 comment 71 which might provide some
> ideas for adding drag and drop support for covers.
> 
> Given bug 392160 which has been marked as duplicate, I assume that is the
> intent of this bug?

Yes, one of them.  Ideally it would handle the drag and drop as described, but also when there are several very close matches, or no good matches, from Amazon (say) or elsewhere, it could prompt the user with a gallery from which the user can select.
Comment 10 Peter 2007-01-12 11:37:09 UTC
Regarding Vincent's questions about the aspect ratio / resizing of the cover in comment 4, comment 7 and comment 8 - that is best discussed on Bug 346679 – album covers - deal with atypical aspect ratio
Comment 11 Ed Catmur 2007-01-14 23:44:31 UTC
Created attachment 80282 [details] [review]
dnd-cover-art.patch

Drag-and-drop.  Against patched rhythmbox (bug 345592)
Comment 12 Vincent Trouilliez 2007-01-15 05:34:51 UTC
Sadly I don't know how to apply patches, if someone is using Ubuntu Feisty and apply these two patches, could you send me the deb file (using "checkinstall") so I can test it ? Or alternatively if you can e-mail me a how-to to apply the patches on the RB source package from Feisty, I have my ears wid opened :-)

Comment 13 Ed Catmur 2007-01-18 18:36:40 UTC
Created attachment 80624 [details] [review]
dnd-cover-art.patch

Reduced patch; decoupling work split out into bug 398093.
Comment 14 Ed Catmur 2007-01-18 18:45:54 UTC
Created attachment 80627 [details] [review]
dnd-cover-art.patch

Split further (to bug 398102).
Comment 15 Ed Catmur 2007-01-19 00:53:28 UTC
Created attachment 80664 [details] [review]
dnd-cover-art.patch

Use signals.
Comment 16 Ed Catmur 2007-01-19 00:55:39 UTC
Created attachment 80665 [details] [review]
dnd-cover-art.patch

Oops.
Comment 17 Ed Catmur 2007-01-19 00:56:52 UTC
Created attachment 80666 [details] [review]
dnd-cover-art.patch

Grr. Sorry for the spam.
Comment 18 Ed Catmur 2007-01-19 01:58:15 UTC
Created attachment 80671 [details] [review]
28dnd-cover-art.patch

After changes to bug 398093.
Comment 19 Ed Catmur 2007-01-19 02:15:26 UTC
Created attachment 80673 [details] [review]
28dnd-cover-art.patch

Fix apply.
Comment 20 Ed Catmur 2007-01-19 15:58:08 UTC
Created attachment 80705 [details] [review]
28dnd-cover-art.patch

fix spacing
Comment 21 Ed Catmur 2007-01-20 13:46:17 UTC
Created attachment 80756 [details] [review]
28dnd-cover-art.patch

Fix passing the db around (Hint: Don't.)
Comment 22 Ed Catmur 2007-01-20 13:49:40 UTC
Created attachment 80758 [details] [review]
30cover-manager.patch

Cover management.  Cumulative on various other patches.  Please test.
Comment 23 James "Doc" Livingston 2007-01-21 07:59:41 UTC
The DND patch looks fairly good to me, but we probably need better handling of things like the iradio source, which don't really support art (or could we allow local art for them?). Incidently, trying to drop art when playing from iradio gives the following:

  • File "/home/doc/programming/sources/rhythmbox/plugins/artdisplay/artdisplay/CoverArtDatabase.py", line 96 in loader_cb
    self.set_pixbuf (db, entry, self.image_data_load (data), callback)
  • File "/home/doc/programming/sources/rhythmbox/plugins/artdisplay/artdisplay/CoverArtDatabase.py", line 200 in image_data_load
    except GError:
NameError: global name 'GError' is not defined




With the art manager, I think the best place for this would be an "Art" tab of the song-properties window; this would also let you look up things for non-playing songs. If we wanted a menu item for it, displaying the properties window for the playing song, and selecting the Art tab might work.
Comment 24 Alex Lancaster 2007-01-21 12:57:48 UTC
Works for me great!  This makes a very useful addition.  Ditto comment #23 regarding adding a tab in the song properties list, with a menu item that takes you to that tab.  

It would would definitely be useful to view (and set) the art for non-playing trakcs.  Ideally the art could also appear in the default "Basic" tab as well so that all the basic info could be viewed at a glance, maybe with a separate "Art" tab for actually editing it.  iTunes does something like that.
Comment 25 Ed Catmur 2007-01-21 18:12:06 UTC
(In reply to comment #23)
> The DND patch looks fairly good to me, but we probably need better handling of
> things like the iradio source, which don't really support art (or could we
> allow local art for them?). 

Well, it makes sense to be able to dnd an album cover, have it cached in ~/.gnome/rhythmbox/covers, and have it come up next time the same album is played.

Of course, LocalCoverArtSearch.py won't be able to do find or save cover art; hopefully it won't try to do anything too stupid.  It currently checks whether the uri scheme is defined and not http.
Comment 26 Ed Catmur 2007-01-21 18:12:49 UTC
Created attachment 80822 [details] [review]
10namespace-gerror.patch

> Incidently, trying to drop art when playing from iradio gives the following:
> ...

Yup, GError is in gobject module. Patch here.
Comment 27 Ed Catmur 2007-01-21 18:30:54 UTC
(In reply to comment #24)
> Works for me great!  This makes a very useful addition.  Ditto comment #23
> regarding adding a tab in the song properties list, with a menu item that takes
> you to that tab.  

OK, I'll look at how to do that; seems like lyrics.py should be worth copying.

Menu item: Edit->Cover art..., yes?

There isn't room in a properties pane for any serious widgetry, so I'd imagine we want a GtkImage (or derived; maybe the same widget as the main art display) with two buttons for Set (from filesystem) and Search (from internet).  This should mean we can do a11y for copy-paste, as well.

I think the popup menu on the main art display should still go directly to the search/set dialogs, though; popup menus are for getting direct to functionality.

> It would would definitely be useful to view (and set) the art for non-playing
> trakcs.  Ideally the art could also appear in the default "Basic" tab as well
> so that all the basic info could be viewed at a glance, maybe with a separate
> "Art" tab for actually editing it.  iTunes does something like that.

There's definitely enough separation in the code that it won't get confused by non-playing tracks.  I'm not sure that there's room in the "Basic" tab for a cover art widget, though; do you feel like doing a mockup?
Comment 28 Ed Catmur 2007-01-21 19:40:43 UTC
Created attachment 80828 [details] [review]
30cover-manager.patch

minor: in SearchLayout, don't display iconview if model is empty.  This means that when there are no results for an engine, clicking on the expander won't display a nasty gap.

major: fix adding results to the SearchLayout model (got got by a python scoping gotcha).

minor: add gtk.gdk.BUTTON_PRESS_MASK on the art widget.  This fixes a bug where, if the first song played didn't have any art, the popup menu wouldn't work until a song with art was played (because setting it as a dnd source adds gtk.gdk.BUTTON_PRESS_MASK).
Comment 29 Ed Catmur 2007-01-21 21:40:08 UTC
Created attachment 80831 [details] [review]
30cover-manager.patch

Spotted another minor bug.
Comment 30 Christophe Dehais 2007-02-19 22:29:47 UTC
Created attachment 82921 [details]
property pane with cover information

Some thought:
* it could be good to be able to set a cover for a whole album easily. Maybe when setting the cover of a song, the plugin could automatically assign that cover to all songs of the same album. But sometimes one's wont't want that...
* what if we could associate more than one image for a song ? Something like a digital version of the CD booklet.
* attaching a mockup of the property pane. "Define cover ..." would launch the cover manager.
Comment 31 Ed Catmur 2007-02-20 01:46:56 UTC
Need to add bug 374653 to dependencies; could a sufficiently empowered person
do so please?
Comment 32 Ed Catmur 2007-02-20 04:11:45 UTC
Created attachment 82929 [details] [review]
30cover-manager.patch

Progress.  Nowhere near ready yet.
Comment 33 Ed Catmur 2007-02-20 04:17:40 UTC
(In reply to comment #30)
> Some thought:
> * it could be good to be able to set a cover for a whole album easily. Maybe
> when setting the cover of a song, the plugin could automatically assign that
> cover to all songs of the same album. But sometimes one's wont't want that...
Currently this happens for free; images are keyed by album.  When we start to support embedded cover art, that'll be written per file; however, it's possible to select a set of songs and set properties on them.  No reason the same shouldn't work with cover art.

> * what if we could associate more than one image for a song ? Something like a
> digital version of the CD booklet.
What's the use case on that?  Seems a bit overcomplicated.

> * attaching a mockup of the property pane. "Define cover ..." would launch the
> cover manager.
Very nice.  My current patch uses a separate tab, but that mockup is far nicer.  I'll look at implementing that layout.
Comment 34 Alex Lancaster 2007-02-20 05:27:24 UTC
(In reply to comment #31)
> Need to add bug 374653 to dependencies; could a sufficiently empowered person
> do so please?

Done. 

Comment 35 Christophe Dehais 2007-02-20 09:50:34 UTC
(In reply to comment #33)
> (In reply to comment #30)
> > Some thought:
> > * it could be good to be able to set a cover for a whole album easily. Maybe
> > when setting the cover of a song, the plugin could automatically assign that
> > cover to all songs of the same album. But sometimes one's wont't want that...
> Currently this happens for free; images are keyed by album.

Oh... When I checked yesterday, I had to drag the album cover for each song of an album. Dragging to one song didn't affect the others.

 
> > * what if we could associate more than one image for a song ? Something like a
> > digital version of the CD booklet.
> What's the use case on that?  Seems a bit overcomplicated.

Well, pretty much the same as a real CD booklet: you can have some goody images attached to an album. I admit I mentionned that because I extrapolated on the electronic distribution of music: an album could be downloaded with a booklet and we might want to make the code scalable to that right now. Yet I understand that handling a list raises a lot more problems than with just one image.
(Another use case that come to mind: you could have an album image and the cover of the single version of the song.)



Comment 36 Ed Catmur 2007-02-20 19:17:45 UTC
(In reply to comment #35)
> (In reply to comment #33)
> > (In reply to comment #30)
> > Currently this happens for free; images are keyed by album.
> Oh... When I checked yesterday, I had to drag the album cover for each song of
> an album. Dragging to one song didn't affect the others.
There are issues for albums with multiple artists; see bug 403959.

> > > * what if we could associate more than one image for a song ? Something like a
> > > digital version of the CD booklet.
> > What's the use case on that?  Seems a bit overcomplicated.
> Well, pretty much the same as a real CD booklet: you can have some goody images
> attached to an album. I admit I mentionned that because I extrapolated on the
> electronic distribution of music: an album could be downloaded with a booklet
> and we might want to make the code scalable to that right now. Yet I understand
> that handling a list raises a lot more problems than with just one image.
For that we'd want to scale not just to multiple images but to arbitrary rich content (formatted text as well).  Not really in the scope of this bug, or indeed of Rhythmbox till we see it happening.
> (Another use case that come to mind: you could have an album image and the
> cover of the single version of the song.)
I think if anyone's that completist they'll have separate tracks for the album version and the single version.  (Why, yes..)

> * attaching a mockup of the property pane. "Define cover ..." would launch the
> cover manager.
Looked at implementing it; not possible bar extreme hackery.  Bug 410125 added to request API.
Comment 37 Ed Catmur 2007-02-20 19:20:08 UTC
Created attachment 82987 [details] [review]
30cover-manager.patch

Updated patch.  This is getting there; please test.
Comment 38 Christophe Dehais 2007-02-20 20:39:53 UTC
Created attachment 82993 [details]
Intial layout of the file-dialog


* 
> There are issues for albums with multiple artists; see bug 403959.
you're right, and sadly I tested only on O.S.T albums...


* Something strange happens here. Yesterday I had the popup on the cover widget and now it doesn't appear anymore (this is not due to the most recent patch, the previous -only one I tested- add the same problem). I have this message on the terminal, each time I right click:

AttributeError: 'gtk.Clipboard' object has no attribute 'wait_is_image_available'

Is there a particular version of python gtk that I need ? (presently I have 2.8.6 on debian)

* Another minor issue is the intial layout of the file-dialog: the file area itself is quite small. see capture.


*
>
> [...] This is getting there; [...]
> 
indeed.
Comment 39 Ed Catmur 2007-02-21 23:03:38 UTC
Created attachment 83074 [details] [review]
30cover-manager.patch

> AttributeError: 'gtk.Clipboard' object has no attribute
> 'wait_is_image_available'
> 
> Is there a particular version of python gtk that I need ? (presently I have
> 2.8.6 on debian)
Yeah, you need 2.10.x for pygtk to handle images on the clipboard properly.  This version uses an Appalling Hack to work around the issue for pygtk 2.8 (though copying images won't work; it'll just copy the path/URL)

> * Another minor issue is the intial layout of the file-dialog: the file area
> itself is quite small. see capture.
Hm.  Perhaps a slightly smaller preview widget might help.  This version sets it to 150x150.
Comment 40 Alex Lancaster 2007-02-22 02:49:37 UTC
Patch above doesn't apply cleanly for me with SVN r4840:

$ svn up
At revision 4840.
$ patch -p0 < 00patches/30cover-manager.patch 
patching file plugins/artdisplay/artdisplay/__init__.py
Hunk #4 FAILED at 289.
Hunk #5 FAILED at 346.
Hunk #6 succeeded at 298 with fuzz 1 (offset -64 lines).
Hunk #7 FAILED at 684.
Hunk #8 succeeded at 794 (offset -2 lines).
Hunk #9 succeeded at 742 (offset -64 lines).
Hunk #10 succeeded at 819 (offset -2 lines).
Hunk #11 succeeded at 780 (offset -64 lines).
Hunk #12 succeeded at 856 with fuzz 2 (offset -2 lines).
3 out of 12 hunks FAILED -- saving rejects to file plugins/artdisplay/artdisplay/__init__.py.rej
Comment 41 Ed Catmur 2007-02-22 03:32:05 UTC
You need to apply 28dnd-cover-art.patch first.
Comment 42 Alex Lancaster 2007-02-22 05:09:17 UTC
Is there any reason they couldn't be combined into a single patch?  It's best to keep one current patch.
Comment 43 Alex Lancaster 2007-02-22 05:25:06 UTC
Now that 0.9.8 is out the door, and we're at the beginning of a new release cycle, I propose that this be committed to SVN to get more testing.  We can still change the layout (e.g. including the cover on the main "Basic" tab) later.
Comment 44 Ed Catmur 2007-02-24 13:18:12 UTC
Created attachment 83231 [details] [review]
30cover-manager.patch

Make sure reply metadata notifies are emitted on next main loop iteration, to prevent weird reentrancy.
Comment 45 Ed Catmur 2007-02-24 23:38:24 UTC
Created attachment 83275 [details] [review]
30cover-manager.patch

Improve separation further, using 'rb:coverArt-with-uri' metadata field.
Comment 46 Ed Catmur 2007-02-24 23:52:54 UTC
Created attachment 83276 [details] [review]
30cover-manager.patch

Oops.  Nah, this is better: set a GObject property on the pixbuf.
Comment 47 Ed Catmur 2007-02-25 00:02:18 UTC
Created attachment 83278 [details] [review]
30cover-manager.patch

Fix bug if two requests arrive before search is concluded.
Comment 48 Ed Catmur 2007-02-25 20:26:11 UTC
Created attachment 83324 [details] [review]
30cover-manager.patch

Improve separation and refactor; ArtDisplayWidget is now a proper standalone widget:
  gobject.new (gobject.type_from_name('artdisplay+ArtDisplayWidget'), shell=shell)
that can be used anywhere. w00t!
Comment 49 Ed Catmur 2007-02-25 20:33:43 UTC
nb. Demo: (with python console plugin)

import gtk, gobject
adw = gobject.new (gobject.type_from_name('artdisplay+ArtDisplayWidget'),
shell=shell)
w = gtk.Window ()
w.set_default_size(400, 400)
w.add (adw)
w.show_all ()
adw.props.entries = [shell.get_player().get_playing_entry()]
print adw.props.uri
Comment 50 Alex Lancaster 2007-02-27 01:05:10 UTC
Applying patch #80756, followed by patch #83324, fails for me with SVN r4865:

$ patch -p0 < 00patches/28dnd-cover-art.patch 
patching file plugins/artdisplay/artdisplay/__init__.py
Hunk #1 FAILED at 234.
Hunk #2 FAILED at 326.
Hunk #3 FAILED at 411.
3 out of 3 hunks FAILED -- saving rejects to file plugins/artdisplay/artdisplay/__init__.py.rej

$ patch -p0 < 00patches/30acover-manager.patch 
patching file plugins/artdisplay/artdisplay/__init__.py
Hunk #1 FAILED at 18.
Hunk #2 FAILED at 94.
Hunk #3 FAILED at 231.
Hunk #4 FAILED at 286.
Hunk #5 FAILED at 314.
Hunk #6 FAILED at 432.
Hunk #7 FAILED at 797.
Hunk #8 FAILED at 842.
Hunk #9 FAILED at 852.
9 out of 9 hunks FAILED -- saving rejects to file plugins/artdisplay/artdisplay/__init__.py.rej
Comment 51 Ed Catmur 2007-02-27 23:52:44 UTC
10namespace-gerror.patch no longer needed; it's been fixed.  Can't mark it obsolete, though; see bug 412847.

As for the patch application failures for 28 and 30, I tried it on a new checkout and had no problem (no fuzz and a few constant offsets).  Try checking out a clean tree?
Comment 52 Alex Lancaster 2007-02-28 02:28:42 UTC
Odd, it failed on my work computer, but worked on my laptop.  I did clean out plugins/artdisplay and update that subdirectory.  Odd.

By the way, could you possibly give each patch a unique file name,  by incrementing the patch number maybe something like:

30a-cover-manager.patch 
30b-cover-manager.patch 

Currently downloading the patch will overwrite the old one, and I need to keep the old one around until I revert it before applying the new one.  It's also good to be able to keep old one's around so you can fallback to them without having to redownload them.  Other devs (e.g. James and Jonathan do this).
Comment 53 Alex Lancaster 2007-02-28 02:32:55 UTC
This latest patch doesn't work for me.  The right-click menu appears on the art display but all the options are greyed-out.  I also get a lot of console messages like this:

TypeError: can't convert return value to desired type
TypeError: can't convert return value to desired type
TypeError: can't convert return value to desired type
TypeError: can't convert return value to desired type
TypeError: can't convert return value to desired type

I'm using python-2.4.4-1.fc6.
Comment 54 Alex Lancaster 2007-02-28 04:12:34 UTC
The "Cover art" tab in the "Properties" widget seems to be working, however.  So the problem seems to be only in the pop-up menu being greyed out.
Comment 55 Ed Catmur 2007-02-28 23:31:01 UTC
Created attachment 83604 [details] [review]
30cover-manager.patch.12

> By the way, could you possibly give each patch a unique file name,  by
> incrementing the patch number maybe something like: (...)
Done, though wouldn't it be easier to store your patches in svn?

(In reply to comment #53)
> This latest patch doesn't work for me.  The right-click menu appears on the
> art display but all the options are greyed-out.  
Bad copy-paste job.  Fixed.

> I also get a lot of console messages like this:
> TypeError: can't convert return value to desired type
> I'm using python-2.4.4-1.fc6.

2.4.4-gentoo.  How about pygobject; which patches are you using for that?  Could you attach to rhythmbox in gdb and break on PyErr_Print?  (alternatively the relevant line in pyg_closure_marshal() in pygtype.c, but you'll have to look at the source to find which line it is for you.)
Comment 56 Alex Lancaster 2007-03-01 05:26:04 UTC
(In reply to comment #55)
> Created an attachment (id=83604) [edit]
> 30cover-manager.patch.12
> 
> > By the way, could you possibly give each patch a unique file name,  by
> > incrementing the patch number maybe something like: (...)
> Done, though wouldn't it be easier to store your patches in svn?

Hmm, odd for some reason bugzilla strips off the "12" when saving that, it might be better to put that before the ".patch".

> (In reply to comment #53)
> Bad copy-paste job.  Fixed.

Thanks, right-clicking seems to work now.

> 2.4.4-gentoo.  How about pygobject; which patches are you using for that? 
> Could you attach to rhythmbox in gdb and break on PyErr_Print?  (alternatively
> the relevant line in pyg_closure_marshal() in pygtype.c, but you'll have to
> look at the source to find which line it is for you.)

/usr/lib/python2.4/site-packages/gst-0.10/gst/extend/pygobject.py seems to come from:

gstreamer-python-0.10.7-2.fc6 

I'll try to get a stack trace upon the error and attach it to this bug.
Comment 57 Alex Lancaster 2007-03-01 11:23:14 UTC
Attached is the backtrace upon PyErr_Print

Breakpoint 2, 0x00338514 in PyErr_Print () from /usr/lib/libpython2.4.so.1.0
(gdb) bt
  • #0 PyErr_Print
    from /usr/lib/libpython2.4.so.1.0
  • #1 ??
    from /usr/lib/python2.4/site-packages/gtk-2.0/gobject/_gobject.so
  • #2 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #3 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #4 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #5 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #6 rhythmdb_entry_request_extra_metadata
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/rhythmdb/rhythmdb.c line 3665
  • #7 _wrap_rhythmdb_entry_request_extra_metadata
    at rhythmdb.override line 568
  • #8 PyCFunction_Call
    from /usr/lib/libpython2.4.so.1.0
  • #9 PyEval_EvalFrame
    from /usr/lib/libpython2.4.so.1.0
  • #10 PyEval_EvalCodeEx
    from /usr/lib/libpython2.4.so.1.0
  • #11 PyClassMethod_New
    from /usr/lib/libpython2.4.so.1.0
  • #12 PyObject_Call
    from /usr/lib/libpython2.4.so.1.0
  • #13 PyClass_IsSubclass
    from /usr/lib/libpython2.4.so.1.0
  • #14 PyObject_Call
    from /usr/lib/libpython2.4.so.1.0
  • #15 PyObject_CallMethod
    from /usr/lib/libpython2.4.so.1.0
  • #16 ??
    from /usr/lib/python2.4/site-packages/gtk-2.0/gobject/_gobject.so
  • #17 g_object_set_property
    from /lib/libgobject-2.0.so.0
  • #18 ??
    from /usr/lib/python2.4/site-packages/gtk-2.0/gobject/_gobject.so
  • #19 ??
    from /usr/lib/python2.4/site-packages/gtk-2.0/gobject/_gobject.so
  • #20 PyObject_SetAttr
    from /usr/lib/libpython2.4.so.1.0
  • #21 PyEval_EvalFrame
    from /usr/lib/libpython2.4.so.1.0
  • #22 PyEval_EvalFrame
    from /usr/lib/libpython2.4.so.1.0
  • #23 PyEval_EvalCodeEx
    from /usr/lib/libpython2.4.so.1.0
  • #24 PyClassMethod_New
    from /usr/lib/libpython2.4.so.1.0
  • #25 PyObject_Call
    from /usr/lib/libpython2.4.so.1.0
  • #26 PyClass_IsSubclass
    from /usr/lib/libpython2.4.so.1.0
  • #27 PyObject_Call
    from /usr/lib/libpython2.4.so.1.0
  • #28 PyEval_CallObjectWithKeywords
    from /usr/lib/libpython2.4.so.1.0
  • #29 PyObject_CallObject
    from /usr/lib/libpython2.4.so.1.0
  • #30 ??
    from /usr/lib/python2.4/site-packages/gtk-2.0/gobject/_gobject.so
  • #31 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #32 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #33 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #34 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #35 reemit_playing_signal
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell-player.c line 545
  • #36 g_cclosure_marshal_VOID__PARAM
    from /lib/libgobject-2.0.so.0
  • #37 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #38 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #39 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #40 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #41 g_object_class_override_property
    from /lib/libgobject-2.0.so.0
  • #42 g_enum_register_static
    from /lib/libgobject-2.0.so.0
  • #43 g_object_notify
    from /lib/libgobject-2.0.so.0
  • #44 rb_shell_player_set_playing_entry
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell-player.c line 1194
  • #45 rb_shell_player_entry_activated_cb
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/rb-shell-player.c line 2189
  • #46 g_cclosure_marshal_VOID__BOXED
    from /lib/libgobject-2.0.so.0
  • #47 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #48 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #49 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #50 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #51 rb_entry_view_row_activated_cb
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/widgets/rb-entry-view.c line 1690
  • #52 _gtk_marshal_VOID__BOXED_OBJECT
    at gtkmarshalers.c line 1422
  • #53 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #54 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #55 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #56 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #57 IA__gtk_tree_view_row_activated
    at gtktreeview.c line 11465
  • #58 gtk_tree_view_button_press
    at gtktreeview.c line 2695
  • #59 _gtk_marshal_BOOLEAN__BOXED
    at gtkmarshalers.c line 84
  • #60 g_value_set_static_boxed
    from /lib/libgobject-2.0.so.0
  • #61 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #62 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #63 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #64 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #65 gtk_widget_event_internal
    at gtkwidget.c line 3915
  • #66 IA__gtk_propagate_event
    at gtkmain.c line 2335
  • #67 rb_tree_dnd_button_release_event_cb
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/lib/rb-tree-dnd.c line 325
  • #68 _gtk_marshal_BOOLEAN__BOXED
    at gtkmarshalers.c line 84
  • #69 g_closure_invoke
    from /lib/libgobject-2.0.so.0
  • #70 g_signal_chain_from_overridden
    from /lib/libgobject-2.0.so.0
  • #71 g_signal_emit_valist
    from /lib/libgobject-2.0.so.0
  • #72 g_signal_emit
    from /lib/libgobject-2.0.so.0
  • #73 gtk_widget_event_internal
    at gtkwidget.c line 3915
  • #74 IA__gtk_propagate_event
    at gtkmain.c line 2335
  • #75 IA__gtk_main_do_event
    at gtkmain.c line 1569
  • #76 gdk_event_dispatch
    at gdkevents-x11.c line 2318
  • #77 g_main_context_dispatch
    from /lib/libglib-2.0.so.0
  • #78 g_main_context_check
    from /lib/libglib-2.0.so.0
  • #79 g_main_loop_run
    from /lib/libglib-2.0.so.0
  • #80 IA__gtk_main
    at gtkmain.c line 1148
  • #81 main
    at /home/alex/src/remote-cvs/gnome.org/rhythmbox/shell/main.c line 383

Comment 58 Alex Lancaster 2007-03-01 11:44:04 UTC
So perhaps this is simply bug #374653? (on which this bug depends)  Is it safe to ignore for the moment?
Comment 59 Ed Catmur 2007-03-03 23:38:18 UTC
Yeah, probably, and if so, yes; the nature of the bug (when it crops up in a closure) is that when it prints that error it's already done the correct thing.
Comment 60 Ed Catmur 2007-03-04 01:15:56 UTC
Created attachment 83858 [details] [review]
28dnd-cover-art.5.patch
Comment 61 Ed Catmur 2007-03-04 01:16:28 UTC
Created attachment 83859 [details] [review]
30cover-manager.13.patch

Updated to latest svn, hopefully.
Comment 62 Alex Lancaster 2007-03-04 02:52:55 UTC
Comment on attachment 83858 [details] [review]
28dnd-cover-art.5.patch

It seems that in bugzilla updating the description, doesn't change the file name the patch will have when it downloads, you need to change the file before uploading.
Comment 63 Alex Lancaster 2007-03-04 02:54:25 UTC
Comment on attachment 83859 [details] [review]
30cover-manager.13.patch

Or updating the filename in the "Filename" field under "Edit attachment" also seems to work.
Comment 64 Alex Lancaster 2007-03-04 03:09:56 UTC
New patches work fine with latest SVN (r4907).  I think this is probably ready to commit to the trunk (development branch).
Comment 65 Derek Harding 2007-04-12 20:56:01 UTC
(In reply to comment #0)
> RB fetch the music art automatically. But often it gets it wrong and there is
> no way to change that. I think that there should be a "Cover Manager" like in
> amaroK

listen does a pretty good job of this too. It has three key features IMHO:

1. You can alter the patterns of what constitutes close. So you can tell it to ignore things like "(Disk 1)" or "(Limited Edition)".

2. You can choose between Amazon or Google for art searches.

3. Where necessary you can manually select the item you want.


Comment 66 Sebastien Bacher 2007-05-20 12:37:57 UTC
Requested also on https://bugs.launchpad.net/bugs/115514
Comment 67 Ed Catmur 2007-06-24 20:37:34 UTC
Created attachment 90582 [details] [review]
28dnd-cover-art.6.patch

to 0.11.0
Comment 68 Ed Catmur 2007-06-24 20:37:56 UTC
Created attachment 90583 [details] [review]
30cover-manager.14.patch

to 0.11.0
Comment 69 Jonathan Matthew 2007-08-26 02:34:36 UTC
I've committed the DnD patch to svn.  I don't have enough of an opinion about the cover manager stuff to do anything with it.
Comment 70 Alex Lancaster 2007-08-26 11:49:24 UTC
(In reply to comment #69)
> I've committed the DnD patch to svn.  I don't have enough of an opinion about
> the cover manager stuff to do anything with it.

It works nicely for me and seems to do a decent job.  Why not commit to the development branch and announce it on the mailing list to get wider feedback/testing?  I think a slightly suboptimal cover manager is better than none at all.  The patch will also get more stale if it isn't committed soon (it currently doesn't quite apply against SVN trunk).

Also being able to browse cover images of tracks that aren't currently playing (via the song properties window) is very useful.

Comment 71 Mike 2007-09-27 01:10:05 UTC
Quick question: Why doesn't this plugin use cover art in the id3v2 tag if available rather than always searching on the net?
Comment 72 Jeff Craig 2007-09-27 01:50:10 UTC
(In reply to comment #71)
> Quick question: Why doesn't this plugin use cover art in the id3v2 tag if
> available rather than always searching on the net?
> 

Because no one has implemented it yet, though apparently gstreamer supports it.  There is an open bug (bug 345975) for it.
Comment 73 Jonathan Matthew 2008-03-18 10:23:24 UTC
*** Bug 424571 has been marked as a duplicate of this bug. ***
Comment 74 Ed Catmur 2008-03-24 21:32:40 UTC
Created attachment 107952 [details] [review]
30cover-manager.15.patch

Update to 0.11.5.
Comment 75 Alex Lancaster 2008-03-27 08:44:04 UTC
Thanks for the updates.  I really would like to get these in, but they fail against SVN:

patch -p0 < patches-manual/active/28dnd-cover-art.6patch 
patching file plugins/artdisplay/artdisplay/__init__.py
Reversed (or previously applied) patch detected!  Assume -R? [n] y
Hunk #2 succeeded at 267 with fuzz 2 (offset 6 lines).
Hunk #3 succeeded at 365 with fuzz 2 (offset 12 lines).
[alex@delpy rhythmbox]$ patch -p0 < patches-manual/active/30cover-manager.patch 
patching file plugins/artdisplay/artdisplay/__init__.py
Hunk #4 FAILED at 286.
Hunk #5 FAILED at 433.
Hunk #6 FAILED at 804.
Hunk #7 succeeded at 786 (offset -67 lines).
Hunk #9 FAILED at 938.
4 out of 9 hunks FAILED -- saving rejects to file plugins/artdisplay/artdisplay/__init__.py.rej

Can you possibly regenerate against SVN trunk?  Otherwise it's unlikely to get reviewed much.
Comment 76 Alex Lancaster 2008-03-27 08:53:36 UTC
My bad.  Apparently the 28dnd-cover-art.patch had already been committed.  The remaining current 30cover-manager.15.patch *does* apply cleanly against SVN.  

Clicking on the cover tab in Properties pops-up the image fine, and clicking "search for cover art" launches the cover art manager which starts and shows the search, but then freezes all of rhythmbox taking up 100% CPU.  No obvious console message failures.   Will investigate further.
Comment 77 Alex Lancaster 2008-03-27 09:00:45 UTC
Here's a debug session (fuzzed out file names and product IDs):

rhythmbox -D artdisplay
-D artdisplay
Registering plugin SampleValaPlugin
Hello world
WARN  coherence                   Mar 27 01:55:14  Coherence UPnP framework version 0.5.0 starting... (coherence/base.py:165)
WARN  coherence                   Mar 27 01:55:14  hostname can't be resolved, maybe a system misconfiguration? (coherence/base.py:194)
WARN  webserver                   Mar 27 01:55:14  WebServer on port 34113 ready (coherence/base.py:103)
Rhythmbox: could not connect to socket
Rhythmbox: No such file or directory
(01:55:31) [0x9797478] [LocalCoverArtSearch.search] /usr/local//lib/rhythmbox/plugins/artdisplay/LocalCoverArtSearch.py:108: searching for local art for file:///path-to-foobar.mp3
(01:55:32) [0x9797478] [AmazonCoverArtSearch.__valid_match] /usr/local//lib/rhythmbox/plugins/artdisplay/AmazonCoverArtSearch.py:454: http://www.amazon.com/gp/product/B000XXXXX%3ftag=webservices-20%26link_code=xm2%26camp=2025%26dev-t=18C3VZN9HCECM5G3HQG2 doesn't have image URLs; ignoring
(01:55:32) [0x9797478] [AmazonCoverArtSearch.__valid_match] /usr/local//lib/rhythmbox/plugins/artdisplay/AmazonCoverArtSearch.py:454: http://www.amazon.com/gp/product/B000YYYYY%3ftag=webservices-20%26link_code=xm2%26camp=2025%26dev-t=18C3VZN9HCECM5G3HQG2 doesn't have image URLs; ignoring
(01:55:32) [0x9797478] [AmazonCoverArtSearch.__valid_match] /usr/local//lib/rhythmbox/plugins/artdisplay/AmazonCoverArtSearch.py:454: http://www.amazon.com/gp/product/B000NNNNN%3ftag=webservices-20%26link_code=xm2%26camp=2025%26dev-t=18C3VZN9HCECM5G3HQG2 doesn't have image URLs; ignoring
Comment 78 Ed Catmur 2008-04-06 22:06:37 UTC
(In reply to comment #76)
> Clicking on the cover tab in Properties pops-up the image fine, and clicking
> "search for cover art" launches the cover art manager which starts and shows
> the search, but then freezes all of rhythmbox taking up 100% CPU.  No obvious
> console message failures.   Will investigate further.

Yes, I'm getting this now since I upgraded to Gnome 2.22.  This appears to be a gtk layout bug - it's spinning in gtk_container_idle_sizer().  Some of the layout code in the art manager is a little screwy, so it could be my fault, though I'm a bit puzzled.  Investigating.
Comment 79 Ed Catmur 2008-04-06 23:42:32 UTC
Created attachment 108745 [details] [review]
30cover-manager.16.patch

This appears to fix the spinning.  The problem appears to be that I was making a size-request smaller than one of my children's allocations.
Comment 80 Alex Lancaster 2008-04-07 12:32:32 UTC
Thanks, the patch now seems to not make rhythmbox spin.

Now I've got a new problem, which is probably unrelated: Amazon search isn't working.  I tried deleting covers from albums that were previously found by amazon, and now the search returns nothing.  At one point I got the following traceback:

Traceback (most recent call last):
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/LocalCoverArtSearch.py", line 44 in _load_dir_cb
    on_search_completed_cb (self, entry, results, *args)
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 729 in on_results
    for data, uri in uris:
TypeError: 'gnomevfs.URI' object is not iterable

I removed the patch and found that this still happens.  Is this something to do with the fact that Amazon switched off one of their old webservices?
Comment 81 Alex Lancaster 2008-04-07 12:34:31 UTC
Another issue that is definitely related to the cover art patch.  When I attempt to right-click and "search for cover art" when playing a podcast, the cover manager doesn't appear and I get the following:

Traceback (most recent call last):
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 416 in do_search_cover
    asd.begin_search (db, entries[0], rb.Loader ())
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 719 in begin_search
    engine.search (db, entry, self.on_results, id, title)
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/PodcastCoverArtSearch.py", line 52 in search
    on_search_completed_callback (self, self.entry, image_url, *self.args)
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 729 in on_results
    for data, uri in uris:
ValueError: too many values to unpack

Previously I believe this worked and the "PodcastCoverArt" or somesuch appeared.
Comment 82 Alex Lancaster 2008-04-07 12:40:16 UTC
Yet another issue, in the cover art manager when I search a track that I know has it's cover art stored in the local directory "LocalCoverArtSearch" just shows "searching" and no thumbnails appear, and I also get the following traceback:

Traceback (most recent call last):
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/LocalCoverArtSearch.py", line 44 in _load_dir_cb
    on_search_completed_cb (self, entry, results, *args)
  • File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 729 in on_results
    for data, uri in uris:
TypeError: 'gnomevfs.URI' object is not iterable

Is this more churn in gnomevfs etc?  These are my versions:

rpm -q gnome-vfs2  gnome-python2
gnome-vfs2-2.20.1-1.fc8
gnome-python2-2.20.1-1.fc8
Comment 83 Jonathan Matthew 2008-04-07 12:42:08 UTC
(In reply to comment #80)
> I removed the patch and found that this still happens.  Is this something to do
> with the fact that Amazon switched off one of their old webservices?

That's why the amazon search isn't working, yes, but the tracebacks you're pasting in here are not related. 

Comment 84 Alex Lancaster 2008-04-07 13:22:30 UTC
(In reply to comment #83)
> (In reply to comment #80)
> > I removed the patch and found that this still happens.  Is this something to do
> > with the fact that Amazon switched off one of their old webservices?
> 
> That's why the amazon search isn't working, yes, 

Is it easy to switch to the new webservice?  I seem to recall some updates done on amarok recently to make it work.  Is there an open bug on this?

> but the tracebacks you're pasting in here are not related. 

Right, I figured.  But they are related to the cover art patch itself.

Comment 85 Ed Catmur 2008-04-08 00:40:30 UTC
Created attachment 108829 [details] [review]
30cover-manager.17.patch

Sorry, there was a bad interaction with an old version of the embedded-art patch.

This should address the "GnomeVFS.URI is not iterable" and other bug - though of course it doesn't fix the Amazon art search.
Comment 86 Ed Catmur 2008-04-08 00:46:48 UTC
(In reply to comment #84)
> Is it easy to switch to the new [Amazon] webservice?  I seem to recall some updates done
> on amarok recently to make it work.  Is there an open bug on this?

Bug 513851.
Comment 87 Alex Lancaster 2008-04-08 01:00:39 UTC
(In reply to comment #86)
> (In reply to comment #84)
> > Is it easy to switch to the new [Amazon] webservice?  I seem to recall some updates done
> > on amarok recently to make it work.  Is there an open bug on this?

> Bug 513851.


Thanks.  Odd that a search of "amazon" in the rhythmbox bugzilla product page doesn't find that bug, see:

http://bugzilla.gnome.org/buglist.cgi?query=product%3Arhythmbox+amazon

Sounds like I need to file a bugzilla bug. ;)
Comment 88 Alex Lancaster 2008-04-08 11:02:34 UTC
(In reply to comment #85)
> Created an attachment (id=108829) [edit]
> 30cover-manager.17.patch
> 
> Sorry, there was a bad interaction with an old version of the embedded-art
> patch.
> 
> This should address the "GnomeVFS.URI is not iterable" and other bug - though
> of course it doesn't fix the Amazon art search.

Looks like there is another problem, this time a syntax error:

 /usr/bin/install -c -m 644 '/home/alex/src/remote-cvs/gnome.org/rhythmbox/plugins/artdisplay/artdisplay/__init__.py' '/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py'
Byte-compiling python modules...
PodcastCoverArtSearch.py AmazonCoverArtSearch.py LocalCoverArtSearch.py CoverArtDatabase.py __init__.py  File "/usr/local//lib/rhythmbox/plugins/artdisplay/__init__.py", line 723
    if not uris:
               ^
SyntaxError: invalid syntax


Byte-compiling python modules (optimized versions) ...
Po
Comment 89 Ed Catmur 2008-04-08 23:18:37 UTC
Created attachment 108898 [details] [review]
30cover-manager.18.patch

Oops.  Fixed.
Comment 90 Alex Lancaster 2008-04-09 10:15:46 UTC
(In reply to comment #89)
> Created an attachment (id=108898) [edit]
> 30cover-manager.18.patch
> 
> Oops.  Fixed.

Great, thanks.  Appears to work fine now (modulo the Amazon search not currently working).

Comment 91 GNOME Infrastructure Team 2018-05-24 11:58:12 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/rhythmbox/issues/257.