GNOME Bugzilla – Bug 363694
make cover/art configurable
Last modified: 2018-05-24 11:58:12 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
Wouldn't just dragging the correct cover art onto the art display be enough?
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.
Cc'ing Gareth on this bug.
(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)
*** Bug 392160 has been marked as a duplicate of this bug. ***
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.
>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
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.
(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.
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
Created attachment 80282 [details] [review] dnd-cover-art.patch Drag-and-drop. Against patched rhythmbox (bug 345592)
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 :-)
Created attachment 80624 [details] [review] dnd-cover-art.patch Reduced patch; decoupling work split out into bug 398093.
Created attachment 80627 [details] [review] dnd-cover-art.patch Split further (to bug 398102).
Created attachment 80664 [details] [review] dnd-cover-art.patch Use signals.
Created attachment 80665 [details] [review] dnd-cover-art.patch Oops.
Created attachment 80666 [details] [review] dnd-cover-art.patch Grr. Sorry for the spam.
Created attachment 80671 [details] [review] 28dnd-cover-art.patch After changes to bug 398093.
Created attachment 80673 [details] [review] 28dnd-cover-art.patch Fix apply.
Created attachment 80705 [details] [review] 28dnd-cover-art.patch fix spacing
Created attachment 80756 [details] [review] 28dnd-cover-art.patch Fix passing the db around (Hint: Don't.)
Created attachment 80758 [details] [review] 30cover-manager.patch Cover management. Cumulative on various other patches. Please test.
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:
+ Trace 103964
self.set_pixbuf (db, entry, self.image_data_load (data), callback)
except GError:
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.
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.
(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.
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.
(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?
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).
Created attachment 80831 [details] [review] 30cover-manager.patch Spotted another minor bug.
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.
Need to add bug 374653 to dependencies; could a sufficiently empowered person do so please?
Created attachment 82929 [details] [review] 30cover-manager.patch Progress. Nowhere near ready yet.
(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.
(In reply to comment #31) > Need to add bug 374653 to dependencies; could a sufficiently empowered person > do so please? Done.
(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.)
(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.
Created attachment 82987 [details] [review] 30cover-manager.patch Updated patch. This is getting there; please test.
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.
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.
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
You need to apply 28dnd-cover-art.patch first.
Is there any reason they couldn't be combined into a single patch? It's best to keep one current patch.
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.
Created attachment 83231 [details] [review] 30cover-manager.patch Make sure reply metadata notifies are emitted on next main loop iteration, to prevent weird reentrancy.
Created attachment 83275 [details] [review] 30cover-manager.patch Improve separation further, using 'rb:coverArt-with-uri' metadata field.
Created attachment 83276 [details] [review] 30cover-manager.patch Oops. Nah, this is better: set a GObject property on the pixbuf.
Created attachment 83278 [details] [review] 30cover-manager.patch Fix bug if two requests arrive before search is concluded.
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!
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
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
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?
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).
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.
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.
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.)
(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.
Attached is the backtrace upon PyErr_Print Breakpoint 2, 0x00338514 in PyErr_Print () from /usr/lib/libpython2.4.so.1.0 (gdb) bt
+ Trace 114878
So perhaps this is simply bug #374653? (on which this bug depends) Is it safe to ignore for the moment?
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.
Created attachment 83858 [details] [review] 28dnd-cover-art.5.patch
Created attachment 83859 [details] [review] 30cover-manager.13.patch Updated to latest svn, hopefully.
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 on attachment 83859 [details] [review] 30cover-manager.13.patch Or updating the filename in the "Filename" field under "Edit attachment" also seems to work.
New patches work fine with latest SVN (r4907). I think this is probably ready to commit to the trunk (development branch).
(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.
Requested also on https://bugs.launchpad.net/bugs/115514
Created attachment 90582 [details] [review] 28dnd-cover-art.6.patch to 0.11.0
Created attachment 90583 [details] [review] 30cover-manager.14.patch to 0.11.0
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.
(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.
Quick question: Why doesn't this plugin use cover art in the id3v2 tag if available rather than always searching on the net?
(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.
*** Bug 424571 has been marked as a duplicate of this bug. ***
Created attachment 107952 [details] [review] 30cover-manager.15.patch Update to 0.11.5.
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.
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.
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
(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.
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.
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):
+ Trace 194483
on_search_completed_cb (self, entry, results, *args)
for data, uri in uris:
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?
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):
+ Trace 194484
asd.begin_search (db, entries[0], rb.Loader ())
engine.search (db, entry, self.on_results, id, title)
on_search_completed_callback (self, self.entry, image_url, *self.args)
Previously I believe this worked and the "PodcastCoverArt" or somesuch appeared.
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):
+ Trace 194485
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
(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.
(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.
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.
(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.
(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. ;)
(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
Created attachment 108898 [details] [review] 30cover-manager.18.patch Oops. Fixed.
(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).
-- 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.