-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Cover Art Support R3
Last week: June 29 - July 05
This week I also pushed some fixes in the first branch.
We realized that using "guiTick50ms" in wtrackis better than "guiTickTime", because in this way we would get less updates.
I also updated the unit tests to work on Windows.
We found out that the dlgtrackinfo UI is to large to use with displays that have a height lower then 635px which is most netbooks. In this way, we would need to do bigger changes because just adding a placeholder would just increase the dlg height and it must be avoided.
I pushed a sketch and I got some feedback from jus. It helped me a lot to improve the layout and think about new possibilities.
Ideas and considerations:
-
Some fields, such as ARTIST, TITLE and ALBUM, need more space to show the information, so it has to be alone in a row.(done!) -
The FILENAME field can be dropped. It only contains redundant info, since it still available in the LOCATION field.(done!) - It could have a button to open the current track directory in a file browser.
- <del>The main purpose of showing covers in the dlgtrackinfo is to have a "brief idea" of which cover is loaded for the track. In this way, it should be as small as possible (maybe 70px). So, other fields would have more space... </del>(done!)
-
dlgtrackinfo should have a height lower than 500px.(done!) -
We could move COMMENTS to a new tab.(done!) -
Covers must be drawn in a QWidget or QGroupBox with a fixed size to avoid resizing the fields beside.(done!)
During the week, basically all main tasks of the second PR were implemented.
I spent some time to find out what is the best way to show options like EDIT and UNSET, Initially was thinking about paint a "X" icon when the mouser is hovering (like we use in wcoverart) and unset the cover if the user click on it. However, it is a bad idea because we would need to reimplement mouseEvent() and repaint(), which could bring many issues. In addition, it was not enough to affect other actions (edit, reload, ...).
Currently, the cover is being loaded as a icon of a QPushButton and it has a popup menu, which works as a context menu implemented using QMenu. In this popup menu we have initially actions to EDIT (change location), UNSET and RELOAD FROM METADATA.
In order to make it possible, I did some adaptions in the CoverCache, adding some methods to handle cover changes. Now it works fine and I also added an action in the popup menu to make users able to reload just the cover from metadata.
There are two cases when a box is showed:
**1 - ** a) Some cover is being displayed;
b) User clicks on "reload track from metadata"
c) the displayed_cover is different from the embedded_cover (new)
-> currently embedded_cover is our priority, so if the current cover is different from the embedded_cover, it is because the user changed it once... So, if we see that the md5_hashes are different, we will ask if he/she want to overwrite the cover...
**2 - ** a) Some cover is being displayed;
b) User clicks on "reload track from metadata"
c) the current track does not have an embedded cover
-> We are warning that there is no embedded_cover for the current track, and we ask if the user want to unset the current one (that was probably changed manually)...
Next week: July 13 - July 19
In the last week, we stop passing pixmaps in the SIGNAL queue to start passing it as reference in CoverCache::requestPixmap(...).
When I was working on the "add/edit" stuff, I realized that we can have some problems with this approach.
For example,
- user choose a new cover
- covercache load it and emit pixmapFound(trackId)
expected: all widgets load the same cover (new one)
current: all widgets&dlgtrackinfo will reload the cover, but just the dlgtrackinfo show the new cover, because other widgets would continue pointing to a old pixmap.
As passing pixmaps in the SIGNAL queue is not a big problem, because it probably will not bring performance issues (since we are using delay), I think that we should use revert it.
I saw that other softwares follow a different concept in their "dlgtrackinfo"...
Usually they show just basic info (COVER ART, artist, title and album) and info that are not editable (duration, bpm, location...) in the SUMMARY tab. All editable fields are showed in a different tab (METADATA).
It seems to be a really good strategy to avoid having a big dlg.
I'm waiting for more feedback about this issue, but maybe we should think about bigger restructuring and do something similar to other softwares...
Daniel detected that Mixxx crashes when he is trying to load .m4a files.
@Daniel
I am not sure about the warning when the cover is replaced with the embedded cover.
IMHO this is the same class of information like track name or artist. So it should be handled at the same > way. I have seen some tracks riped from CD where the Artist is "Unknown" overwriting the right Artist name with > "Unknown" is the same class of annoyance.
Please Issue the case if you reload the metadata from many files.
In this way, should we disable all warning/question boxes?
We already have a lot of big and small issues to sort out in the second PR during the next week. However, it is important think about what comes after that. In fact we have just more 2 big tasks to complete, which are implement a new widget to show covers AND make users able to download covers.
Mixxx is a free and open-source DJ software.
Manual
Hardware Compatibility
Reporting Bugs
Getting Involved
Contribution Guidelines
Coding Guidelines
Using Git
Developer Guide
Creating Skins
Contributing Mappings
Mixxx Controls
MIDI Scripting
Components JS
HID Scripting