mixxx
mixxx copied to clipboard
Cover Art Fetcher
Hey everyone!
My goal is to get cover arts of the tracks by using coverartarchive
on the "Import Metadata from Musicbrainz" menu, aka Tag Fetcher Menu. I am working on this feature right now. This is the initial PR to get your ideas-thoughts and of course feedback about the GUI of this feature. More information about this PR can be found on our Zulip chat https://mixxx.zulipchat.com/#narrow/stream/109215-gsoc/topic/cover.20art.20lookup
This is the first interaction for now. I have displayed the existing cover art of the track on the Tag Fetcher Menu. The Tag Fetcher Menu seems like this.
Right now I can send the metadata of the Suggested track to coverartarchive and get a response via a button. It is better to first decide what should GUI look like? Then we can implement that (or something similar) later on.
For to decide about GUI, we should decide how this feature can be used?
- I think there should be a button called "look for cover art" which triggers the request when pressed, and can get cover art of the selected
Suggested Tags
. - Should we have separate buttons for the metadata and cover art? Can cover-art be updated separate then the metadata or apply button should update both the cover art and the metadata?
Do you have any other use-case comes to your mind? These few buttons would make this menu crowded and complicated? What do you think generally?
(This feature also needs your feedback about technical stuff, the first thing is how to store the cover-arts, cover-arts has thumbnails should we use them, if the cover art updated, should we save it as a file or embed in the file, should user choose between these storing options and so on...)
Nice!
The GUI can be as rough as necessary for testing the functionality, don't worry. That can be rearranged and polished later on.
Just some ideas/impressions regarding the GUI:
- show the current cover and new candidates in a vertical layout:
- "current:" at the top
- candidate (blank cover if no candidate has been fetched?)
- "Get cover art" button at the bottom?
- maybe it would be worth to rework the WCoverArt instantiation so the right-click menu can be disabled in the musicbrainz dialog?
I think we should fetch an adequate thumbnail first. Smallest is 250px, right? Then try choose bigger version if necessary, depending on the Mixxx GUI scalefactor, so images look sharp on HiDPI screens. Maybe users want to preview the image in full size. Usually left-click on the cover opens the separate resizable popup. Dunno how that would feel like if the full-size cover needs to be fetched first.
Thanks you for the feedback @ronso0
The GUI can be as rough as necessary for testing the functionality, don't worry. That can be rearranged and polished later on.
That's good! So we must decide the GUI at least for the testing.
show the current cover and new candidates in a vertical layout: "current:" at the top candidate (blank cover if no candidate has been fetched?) "Get cover art" button at the bottom?
I think that 2 cover arts is a great idea. Both for technical and UX way. So these two cover arts can have different functionality. One is simply can show the existing cover art, the other can show selected suggested tags
.
maybe it would be worth to rework the WCoverArt instantiation so the right-click menu can be disabled in the musicbrainz dialog?
Maybe a child class can do that. Which can only show the cover art and populate the full size of the cover art. Would something like that work?
I think we should fetch an adequate thumbnail first. Smallest is 250px, right? Then try choose bigger version if necessary, depending on the Mixxx GUI scalefactor, so images look sharp on HiDPI screens. Maybe users want to preview the image in full size. Usually left-click on the cover opens the separate resizable popup. Dunno how that would feel like if the full-size cover needs to be fetched first.
Yeah thumbnails are 250px. I think for displaying the suggested tag
cover arts we better use the 250px in order to reduce the performance issues. We thought users can choose the resolution of the cover art. We have a popup for existing cover-arts, maybe something similar can be done for the founded cover arts.
By the way, I was playing with the UI, so I did something like this. With this one button, maybe found cover arts can be displayed if the selected result triggered. What do you think?
The resolution/quality question has some aspects we need to consider.
- Internet bandwidth
- Display resolution
- Storage space
- Time for fetching
- rate limits
What is the best strategy?
We may either always fetch all thumbnail covers for all MusicBrainz results, so that they are immediately displayed when selecting a row. When the user has decided to integrate a cover, we may download the desired target size.
Another strategy may be to download only the desired target size on demand and rescale it before displaying.
What should be the default behaviour? What should be the default size?
@ronso0 what do you think?
I can think of a few checkboxes:
- [ ] Fetch Cover
- [ ] Use High-Res cover (1MB)
- [ ] Store as track-file-name.jpg
- [ ] Integrate in track file
Is this a good Idea to offer these options? Do we want to place them on the dialog or in preference?
Does Qt have some abstraction that we can query whether the system is on a metered connection? We could use that for deciding the quality option and eagerness of the fetching. So we can save bandwidth when the connection is metered.
Does Qt have some abstraction that we can query whether the system is on a metered connection? We could use that for deciding the quality option and eagerness of the fetching. So we can save bandwidth when the connection is metered.
I found this on QT Doc, can it work? I assume it is not because It says since 6.2.
Thats pretty much exactly what I wanted, but yes, we can't use it because we're still stuck on Qt5.15
Thats pretty much exactly what I wanted, but yes, we can't use it because we're still stuck on Qt5.15
So there should be some TO DO comment lines for the future.
The resolution/quality question has some aspects we need to consider.
- Internet bandwidth
- Display resolution
- Storage space
- Time for fetching
- rate limits
What is the best strategy?
We may either always fetch all thumbnail covers for all MusicBrainz results, so that they are immediately displayed when selecting a row. When the user has decided to integrate a cover, we may download the desired target size.
Another strategy may be to download only the desired target size on demand and rescale it before displaying.
What should be the default behaviour? What should be the default size?
@ronso0 what do you think?
There is no rate limits according to Coverartarchive API documentation.
That was exactly what I thought in the first place, we should fetch all thumbnails with the MusicBrainz results, and user can see by just clicking on the suggested tags, if they like they can get the cover art.
I have tried the API with the few songs, most of the results are not stable. What I mean by that is there is a main image with the high resolution which was 11MB. Then thumbnails come along, the sizes are generally starts from 1200 and then gets smaller to 500 and 250. There are also 2 keys with the small
and large
which is equivalent of 250px and 500px. Some Album Release ID's have only small and large. So maybe we might use to fetch small
for the first time and users can choose large
to integrate.
What do you think?
I can think of a few checkboxes: Fetch Cover Use High-Res cover (1MB) Store as track-file-name.jpg Integrate in track file Is this a good Idea to offer these options? Do we want to place them on the dialog or in preference?
I think we should offer this options later on after this feature is working. And if we do that we should do in the preference. On preferences there could be section for cover art fetcher. If the users want to fetch cover arts they can choose. I think this will affect the UI, I couldn't make sure how to deal with it at the first time but this can be added as a first thing. Then resolution could be chosen on that screen to, small or large. Storage can be added in that section too.
So maybe we might use to fetch small for the first time and users can choose large to integrate.
Yes, use the most appropriate size for the UI (depending how big the coverart will be displayed) but use the best quality available for actually storing in mixxx.
11 MB for a cover is probably overdone for a 5 MB mp3 file. I cannot effort the HDD to double up the size of my collection because of cover arts. So is there a size that fits to the screen size when double clicking the cover? I think I can effort 200 kB or such for a cover. @Swiftb0y: Do we also need the option to actually use the 11 MB cover art? If we store it in a separate folder, the user can delete the big once if he is running out of HDD. Would that be worth to consider?
Cover arts are usually only that big when their resolution is large (I'd estimate 11Mb to be about 5000x5000 pixels) or there is almost no compression... Rescaling/compressing the cover in mixxx is probably too complicated. I guess we can add a preference option for the upper limit in terms of size/resolution?
I have checked with the few cover art responses. The original Image had big resolution, and it can change album release id to album release id. Small sizes (250px) are approx 20-30 KB and large size (500px) are 80-100 KB. 1200 px are approx 500kb.
I think we should use small and large sizes for fetching and applying. I think large size enough for most use cases.
For the higher resolutions (1200px in this case), I'm not sure what to do. Also about the 11 Mb, I think that was only for an album, this can be changed and maybe could be more or less, so it is better not to use original image returned by the coverartarchive.
I want to use small and large sizes for normal use cases as @daschuer mentioned 200 KB is affordable for a cover art. I think that, we can say this could be the normal use case, but for the higher resolutions, (Let's say a user cares about the quality about the cover art a lot) we can fetch the actual image (we don't know how big it could be) by a button (but this time there will be a lot of buttons and that will affect the usability) or a preference setting such as get the highest-large-small. 1200 can be used too it is not in the responses always.
I think we should offer this options later on after this feature is working. And if we do that we should do in the preference. On preferences there could be section for cover art fetcher.
Yup. Btw I think we should try to split the Library page.
We could also make the size of the cover art to be depended on the filesize. Then HQ flacs get HQ cover arts but LQ mp3s get low res cover arts. I think someone with a flac collection cares more about quality in general. Also since the size increase is relative, it wouldn't be that noticeable.
If we store it in a separate folder, the user can delete the big once if he is running out of HDD. Would that be worth to consider?
I wouldn't make it that complicated. A pref option [small | medium | large] as well as "Store in folder or add to file" should suffice. For "medium" something between 800 and 1200 is okay, no? choose depending on what's available (if that info is available prior to fetching the image).
Then HQ flacs get HQ cover arts but LQ mp3s get low res cover arts. I think someone with a flac collection cares more about quality in general.
I do care about audio quality but I don't need 4k cover art : ) Thus I'd prefer a pref option.
Yup. Btw I think we should try to split the Library page.
I haven't understand what do you mean by split the library page, sorry. Could you please explain it more?
I do care about audio quality but I don't need 4k cover art : ) Thus I'd prefer a pref option.
Right... still I think it would be a shame if one was not able to leverage HQ cover arts when they are available. Since we already have a pref options for image size in pixels and in file size, a third option for specifying a ratio wouldn't be that much more complicated.
I haven't understand what do you mean by split the library page, sorry. Could you please explain it more?
Just a side note, actually irrelevant here. I mean splitting up Preferences > Library into at least two sub-pages, because we added more options, thus the Library page gets longer and longer and needs to be scrolled and it becomes harder to find certain options.
So, I have updated the GUI as I mentioned on this comment. I have left few comments in the new files/classes just for this commit to tell what I wanted to.
I think that on the Tag Fetcher menu it shouldn't be right clicked (cover art menu shouldn't be poped up). In order to do that, I needed another class. But the classes were almost the same, so in order to not duplicate the code, I have made a base class called "CoverArtLabel" (it is on the tag fetcher) and the widget one is derived from the base class. On my test it was working fine. But I'm not sure if I did something good.
I might also need that for dlgcoverartfullsize, because it can also trigger the cover art menu, but in the future this menu can be made specialized to cover art fetcher such as, set this cover art, download this cover art and etc.
Also about the GUI, since I am not an expert, when I resize it, it breaks.
I think that these three parts need a bit refactor before moving to the technical part of this PR.
Thanks in advance for the comments and feedback.
As we mentioned that, GUI can be addressed later on. So I wanted to get your ideas about the displaying fetched cover arts.
What I want to do is add another feature on the existing TagFetcher. So before I ask my questions, I want to basically tell how Tag Fetcher works. This is directly related for the future steps.
As mentioned On the comments in tagfetcher.h
There are 3 stages. When user clicks on "Import Metadata from Musicbrainz" these happens:
- Chromaprint creates AcoustID fingerprint.
- AcoustIDtask returns MusicBrainz UUID's so they can be used to get additional info about the track on Musicbrainz.
- MusicBrainzTask gets related metadata of the track with the given UUID from AcoustIDTask.
This system works perfectly, even there is no related info about the track, user is warned. Sometimes it returns just a result or up to 30 results according to my test tracks (Most result I had for a track was In The End by Linkin Park). This is an important information about fetching the cover arts. This is also a fast process comparing to the coverartarchive.
How do I get cover arts of the tracks? By using cover art archive. How this works? It only accepts release id.
- I will send request to CoverArtArchive with the existing id of the results, that returns all the links as a j son response. There is a interesting block for me, It might be failed because some ID's returning 404 because they don't have a cover art. But also we need an another step for to retrieve 'actual' cover art.
- I will send another request to coverartarchive with the URL of the Image itself. That will return me Byte Array of the image. I think that I can use this to display the cover art on the second
fetched cover art
cover art label. This task never fails on my tracks. It always returns an image, i think that because some ID's failed at first stage.
The ideas are:
-
The first idea is to fetch cover arts at first time and display right after the menu is opened. This can be achieved by adding these 2 steps after the musicbrainztask succeeded. This has advantages and disadvantages. As an advantage, users can see the cover arts without waiting one by one. As a disadvantage, this can make the time longer for to get results of the metadata. For the popular songs (as i mentioned earlier
In the End
) to get metadatas it takes noticeably longer already. If we fetch all the cover arts in the first place I'm afraid this will make to get results longer. Also, coverartarchive is a slower process compared to others. Fetching all the cover arts (I assume in the first place we wanted to get small ones) it is 25-30 kbs each, if we have a lot of results would it be a lot to fetch all of them at once? -
Second idea is, when a result selected via mouse click or hovered by keyboard events, we can start the coverartarchive process. This will not make this 3 stage task longer so menu will be displayed faster, the tasks will be separated. But also, user can sent a lot of request without noticing, for now when I sent few requests one after another, Mixxx crashes. I can fix this by adding aborted I guess. Also if we choose this way, I think we need to have a dummy cover art that can notice users, that fetching on progress, and also another dummy cover art that fetching didn't have a result (this no result also a must for first idea too)
-
I know that we wanted to make this menu without buttons (just apply) but. The last idea about it, (it is really similar to the second one). We add a button for to fetch cover arts, and if the result found user can apply it. It has also a good use-case for the users who doesn't want to get cover arts IMHO.
This became a huge update-question. Sorry for that :sweat_smile:
What do you think about these ideas?
Since #4864 is merged, I have updated this PR.
Hey everyone, this is an update about the cover art fetcher. It has been a long time since the last important update due to division into many PR's (some of them are merged - the others are still on progress) such as:
#4864 Cover art label composition with menu: Tag fetcher dialog the menu could be populated on the fetched cover art (which was unwanted behavior)
#4871 New Tag Fetcher Dialog : That was quite needed to show the cover art progress to the users. (Meanwhile working on this and playing with the tag fetcher a lot I found few related bugs with the tag fetcher which are -> #10795 and #10796)
#4887 Ask user after changing cover art: This is an implementation of a cover art copy worker, which is needed for this feature. More details can be found on the PR page.
#4909 Fix Reload From File/Folder: This was a small fix related with the library scan. Before this fix, after I updated all of my library with the new cover arts, the tracks cover art updated with the wrong cover art because the cover arts had many dots .
in their name.
Meanwhile I was working on these features - fixes, I was also working on this feature on my Local PR. Since the coding period is coming to the end, I decided to release a POC about the cover art fetcher. So I can work on the track suggestion feature. I roughly merged all of the related branches into one and decided to update on this PR. The code is not yet polished, waiting for the prerequisite PR's. But I would like to work on it any time and have a a rock solid feature.
Thanks to all reviewers who shared their ideas-feedback about this PR @daschuer @ronso0 and @Swiftb0y
We are able to get HQ cover arts (This can be done by selecting Highest option on the Preferences > Library) and GUI improved to show the cover arts (The full size can be populated perfectly for the Low-Medium and High options).
The user-case change is, after the fetching metadata is successful, when user select the suggested tags cover art fetching progress starts. If it is successful, after pressing Apply cover art is downloaded to same folder where the track located and applies it right away (This fetching progress should be turned on-off on the preferences IMHO). If no cover art found, apply button only applies the metadata.
There are still things to do in this PR, such as: Better scaling on full size for Highest resolution cover arts. A caching class. Preference to turn on this feature on-off (Might be needed might be not).
I would be really happy if you can try this feature and share your ideas.
I also want to share the final situation via some screenshots.
The preferences options are like this:
The first initial cover art fetching progress (finding if the cover art exists):
After the cover art found, fetching the actual cover art as an image:
After the cover art is found:
After applying:
Now this PR is rebased on #4871
I have implemented the worker into the DlgTagFetcher
.
It works fine according to my tests. As a use case, I didn't see any unwanted behaviour. But still, It needs a good refactoring.
I think there are still these things refactored-added:
- Full size population for fetched cover art not populating properly. With my setup, I only have problem with Highest Size, If the fetched cover art more bigger than 1200x1200, it is not populated correctly.
- More debug messages can be added just in case of any failure in cover art tasks.
- Code can be refactored a bit, I added comments to make it explanatory and tried better naming, but a new perspective would be very helpful.
What do you think?
Sorry for the delay, I tried to fix all the requested changes and also the conflicted files.
As far as I tested, it works just fine, but still it needs still a good polishing, and tested in any way.
What do you think? @daschuer @ronso0
Thank you for continuing her just after the prerequisites have landed in main.
U have tested this a bit, and it is really a great feature.
The jusability can be improved a bit. I am just not sure if we should hold this PR back over it. @ronso0 what do you think?
Here my findings:
- The cover size (pixel count) varies for differnt covers. I like to add covers roughly at the same size. Was this just a bad pick or is the cover size unpredictable? Can we give an estimation what the sizes mean in terms of pixel and bytes?
- The cover size setting is hard to discover. Can we make them accessible from the tag fetcher dialog?
- I think we need a different empty cover, for not available vs. not set.
- caching would be nice, for not fetching the same cover twice when scrolling though the results.
I took a look and have these findings from a quick test with two tracks.
- caching would indeed be good (at least for the time the dialog is open) Is QPixmapCache sufficient?
- no matter which image size I select in the preferences, clicking the cover preview has always the same size? If I select "high" or "highest" I expect a 1:1 preview so I can judge the image quality
- the coverart zoom is broken: the window resizes but the actual images doesn't (it's just cropped or padded with the background color, and the aspect ratio changes when zooming in - out)
the coverart zoom is broken: the window resizes but the actual images doesn't (it's just cropped or padded with the background color, and the aspect ratio changes when zooming in - out)
Fixed by 34c92ffa5239099649617049c9f6afcf4b529da1