Some cover art changes. #22
Conversation
Add <DefaultCover> configuration option to WCoverArt to let skin authors pick a default cover.
* Move cache key formatting to its own method. * Explicitly handle the no-size cache key.
Add helper to create a supported cover art extensions regex.
* Move cover search out of the CoverArtCache workers. * Get rid of dedicated table for cover art (and add more columns to library table) * Delete CoverArtDAO and test. * Add cover scanning to LibraryScanner. * Load and store cover art in TrackInfoObject via TrackDAO. * Hook cover columns up to cached TrackInfoObjects in BaseTrackCache. * Get rid of "ID3TAG" string to identify metadata. Use CoverInfo::Type instead. * Add source tracking for cover provenance (user-selected vs. auto-selected) so we know whether it's safe to change a cover without losing user data. * Add helper for reloading cover art from file. * Use relative paths to cover art so FILE art is not broken by folder moves.
* Communicate selected track instead of cover art. * Support connecting a WCoverArt to the track currently loaded to a deck.
| TrackPointer pTrack = TrackPointer( | ||
| new TrackInfoObject(filePath, pToken), | ||
| new TrackInfoObject(filePath, pToken, true, true), |
|
@rryan , you reintroduced the bug I solved with the parent issue. Here is a screenshot of what I mean, the cover should be above the trackInfo dialog.
|
Ah I see. I can't reproduce that. I thought it was a sizing bug (the full-size widget thinks the largest window is the track dialog). |
|
@kain88-de @cardinot @daschuer @ywwg The current image file search logic works well for all my album folders but for a folder like ~/Downloads/ (which is an unorganized mess of files) there are one or two image files in this folder. As a result, all my files in that folder have a randomly chosen image file as their cover. I propose a cut-off of something like 50 files. If the folder has more than 50 files in it, lets not do file-based cover guessing. Thoughts? Something better would be detecting coherence of the tracks on some dimension (same album, same artist, lack of metadata art, etc.) and doing file guessing when the tracks are something like 80% coherent or higher. The structure of the scanner makes that annoying right now. |
The QDialog docs suggest calling activateWindow() after show() and raise() for a modeless dialog. I added activateWindow() to DlgCoverArtFullSize. Could you test (since I can't repro the issue)? |
Occasionally labels will rerelease a giant collection of favorite tracks all under the same album name. I do think the better metric would be checking for a large number of album names in one folder. I'm happy with that being a TODO and leaving the cutoff at 50, though. |
Nope this issue is not fixed here. I'm actually wondering that you don't have it on OSX. This is what the qt docs say for
Because of this I went with giving dlgtrackinfo a real parent. This also solves the issue currently for me. But this is a bit strange because dlgcoverartfullsize is getting a I assume this is because different window managers handle some things differently. I'm currently using gnome-shell. I can try KDE when I'm at home tomorrow. |
Maybe we can fix it by distinguish it by the cover file case, not by the count of tracks.
It is just the decision about what is more annoying, to pick the wrong cover if there is a choice, or to pick non. I think the current version picks one. But on the second thought, it may look unprofessional to have the wrong cover so having non, might be better. |
* Adds CoverArtCache::guessCover(s) methods for calling CoverArtUtils::guessCoverArt in a background thread. * Removes processing of reset menu option from WCoverArtMenu and instead sends a signal that the owner of the menu should handle.
Nice idea @daschuer -- I'll do this. |
| QString trackAlbum = query.value(3).toString(); | ||
| CoverInfo::Source source = static_cast<CoverInfo::Source>( | ||
| query.value(4).toInt()); | ||
| if (source == CoverInfo::USER_SELECTED) { |
There was a problem hiding this comment.
Is that even possible given the query that we are using?
|
@rryan really nice job, thanks for doing this. Once you have addressed me last comments and Daniels I'll merge coverArt into master. |
|
I went ahead and changed the last remarks myself. Thanks again @rryan |
|
Cool -- thanks. I'm on vacation so won't have much computer access through On Wed, Nov 5, 2014, 8:14 AM kain88-de [email protected] wrote:
|
Hey @cardinot, @kain88-de,
Great work with this feature! It's super fun to use.
I made some changes -- I realized the set of things I was asking you to do were basically unreasonable to come to you this late in the game and ask for so I went ahead and did them myself this weekend.
I know I made some significant changes to the design but I think they were necessary. I'm happy to discuss any of them.
I'm really excited to have this in Mixxx 1.12.0. I think the branch is merge-able with these changes. There are some improvements I want to do (I left myself TODOs) but I think they're fine to do post-merge.