GNOME Bugzilla – Bug 457375
More flexibility for the artdisplay plugin (local cover art in subfolders, configuration dialog) (PATCH)
Last modified: 2018-05-24 12:42:32 UTC
Some people, including me, keep local cover images in /path/to/record/Cover and call them Front.jpg, Back.jpg and so on. Attached patch to LocalCoverArtSearch.py attacks this problem by allowing to search in certain subfolders. Other information:
Created attachment 91858 [details] [review] Little modification of LocalCoverArtSearch.py to allow searching in subfolders relative to track URI
Comment on attachment 91858 [details] [review] Little modification of LocalCoverArtSearch.py to allow searching in subfolders relative to track URI Please ignore the first part of this patch. The modification of rb-util is not necessary for the cover art issue. Only the second part, the patch to LocalCoverArtSearch.py is necessary for this.
Created attachment 93100 [details] [review] Patch to the artdisplay plugin allows for local covers in subdirs and adds a configuration dialog for the plugin
So you do something like this? (covers in a subdirectory of where the music file is): ~/mp3/artist_X/album_Y/track1.mp3 ~/mp3/artist_X/album_Y/track2.mp3 ~/mp3/artist_X/album_Y/track3.mp3 ~/mp3/artist_X/album_Y/Cover/Front.jpg ~/mp3/artist_X/album_Y/Cover/Back.jpg Out of interest, why do you do this? Have you checked how your patch this would affect people who do the following (e.g. me, and I suspect, many others): ~/mp3/artist_X/album_Y/track1.mp3 ~/mp3/artist_X/album_Y/track2.mp3 ~/mp3/artist_X/album_Y/track3.mp3 ~/mp3/artist_X/album_Y/cover.jpg ~/mp3/artist_X/single1.mp3 In the above example, there is no cover defined for "single1.mp3", but with your patch I think it would incorrectly pickup the cover file for album_Y (but I didn't actually try this out) -- The minor change of adding "Front" to the list of image filenames checked by default seems reasonable. Supporting more than one image per track (e.g. front and back) has been suggested before and I think the consensus was this was uncommon and trying to support it would make both the code and the GUI too complicated.
Peter, first to your question: Yes, I do arrange it like your example, ~/mp3/artist_X/album_Y/track1.mp3 ~/mp3/artist_X/album_Y/track2.mp3 ~/mp3/artist_X/album_Y/track3.mp3 ~/mp3/artist_X/album_Y/Cover/Front.jpg ~/mp3/artist_X/album_Y/Cover/Back.jpg The question why it is exactly this organisation pattern and not a different one can't really be answered. Subjective convenience maybe. I don't claim it is a superior arrangement from a technical point of view. It's a habit. Your second example is a good point, though. Picking up the wrong cover in this case would be prevented if the recursion depth of the subdirectory searching was restricted to 1. I will check if this problem occurs and apply the necessary changes in this case.
Created attachment 94310 [details] [review] Patch to the artdisplay plugin allows for local covers in subdirs and adds a configuration dialog for the plugin
Comment on attachment 94310 [details] [review] Patch to the artdisplay plugin allows for local covers in subdirs and adds a configuration dialog for the plugin This revision of the patch handles a minor flaw in the case of missing gconf keys correctly.
(In reply to comment #4) > In the above example, there is no cover defined for "single1.mp3", but with > your patch I think it would incorrectly pickup the cover file for album_Y (but > I didn't actually try this out) In the example you mentioned, the cover.jpg would not be picked up accidentally because 'album_Y' is not a valid name for a cover subfolder. My patch uses only a limited set of names relative to a file as valid subfolders to look for covers, currently only "cover" and "artwork". IMHO, the only thing that gets us in trouble is an artist who calls one of his records "Cover" or "Artwork" and you have: /path/to/evil/artist/Artwork/01-Title_01.mp3 /path/to/evil/artist/Artwork/01-Title_02.mp3 /path/to/evil/artist/Artwork/Cover.jpg /path/to/evil/artist/Single1.mp3 I really hope the possible coincidence of an album name and a supported subfolder name is not reason enough to refuse the whole idea (and therefore my patch...). Riggs
I would appreciate feedback concerning this patch. Any chance for acceptance?
Why should there be a configuration dialog for the art display plugin?
Because imho it is desirable to tell the art display plugin something like "use the local covers but don't download new ones from amazon."
I think the preferences you're adding here are too specific. What reasons do you think a user would have for disabling amazon cover art searching? Would those also apply to any other internet-based search? If we added a cover art search that used last.fm, for example, why would a user want to enable that but disable amazon? Why should the user care which website(s) we use to find cover art? Why would a user want to disable the local cover art search, or the use of the image specified in a podcast feed? I can definitely see a use for a "don't go looking for anything on the internet" preference that would disable internet-based cover art searches, lyrics searches, last.fm submissions, podcast updates and downloads, and anything else.
(In reply to comment #12) > I think the preferences you're adding here are too specific. Valid point. > What reasons do > you think a user would have for disabling amazon cover art searching? Would > those also apply to any other internet-based search? Actually that is what I initially had in mind. I simply wanted to use the local covers and prevent retrieving new ones from the net. And while I was on it, I figured that other people might have other specific reasons for other scenarios, so I assumed it might be a good idea to allow to turn the different engines of the artdisplay plugin on and off, just as the user desires. > Why should the user care which website(s) we use to find cover art? I don't know. Ideology maybe? I am not questioning a user's reasons. Some people choose RB over other media players offering the same functionality. Isn't this one of the key possibilities in open source? That we are not limited by some company's decision? That users can decide because there are the possibilities? Anyways, I totally see your points and all of them seem valid to me. In my particular case, I simply need the "use local cover arts, but don't retrieve new ones from the internet" function. This patch is not set in stone. I am absolutely fine with writing patches according to official RB development policy so that they can be accepted upstream. So, what should I change here? Configuration just for "Internet yes/no"?
*** Bug 434148 has been marked as a duplicate of this bug. ***
(In reply to comment #13) > > What reasons do > > you think a user would have for disabling amazon cover art searching? Would > > those also apply to any other internet-based search? > > Actually that is what I initially had in mind. I simply wanted to use the local > covers and prevent retrieving new ones from the net. > And while I was on it, I figured that other people might have other specific > reasons for other scenarios, so I assumed it might be a good idea to allow to > turn the different engines of the artdisplay plugin on and off, just as the > user desires. I would rather wait until someone actually argues for that preference. > > Why should the user care which website(s) we use to find cover art? > > I don't know. Ideology maybe? I am not questioning a user's reasons. I absolutely will. I'm also going to question whether these particular users even exist, and I'm going to assume they don't until they prove they do. > Some people choose RB over other media players offering the same functionality. > Isn't this one of the key possibilities in open source? That we are not limited > by some company's decision? That users can decide because there are the > possibilities? Yes and no. We have to figure out which choices are most important and useful, and if we're thinking "I don't care about this, but maybe someone somewhere does for some reason.." .. well, that's not very far up the list. I'd suggest that you read Havoc's ideas on free software UI: http://ometer.com/free-software-ui.html, particularly the sections on preferences. These ideas informed a lot of the design decisions in GNOME 2. > Anyways, I totally see your points and all of them seem valid to me. > In my particular case, I simply need the "use local cover arts, but don't > retrieve new ones from the internet" function. > > This patch is not set in stone. I am absolutely fine with writing patches > according to official RB development policy so that they can be accepted > upstream. > So, what should I change here? Configuration just for "Internet yes/no"? For this bug specifically, I'd say leave the preferences out of it for now and just focus on what we came here for: improving the local cover art search. I haven't had much of a look at the part of your patch that does this, since I got distracted by the preferences side of it, but the idea seems reasonable enough to me.
(In reply to comment #15) > I'd suggest that you read Havoc's ideas on free software UI: > http://ometer.com/free-software-ui.html, particularly the sections on > preferences. These ideas informed a lot of the design decisions in GNOME 2. I'll have a look, thanks for the heads up. > For this bug specifically, I'd say leave the preferences out of it for now and > just focus on what we came here for: improving the local cover art search. I > haven't had much of a look at the part of your patch that does this, since I > got distracted by the preferences side of it, but the idea seems reasonable > enough to me. Okay, agreed. I can live without a config dialog for this plugin. But we should implement the global "don't do online stuff" switch some time (is there already a bug report for this?). Should I update the patch just to not include the config dialog or can we agree on the improvement first? I described how my proposed improvement works in the earlier discussion. Would you like to provide feedback to it? Thanks in advance
-- 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/404.