User Tools

Site Tools


cover_art_support_r2

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
cover_art_support_r2 [2014/07/06 20:21]
cardinot
cover_art_support_r2 [2014/07/06 22:29] (current)
cardinot
Line 1: Line 1:
 ====== Cover Art Support ====== ====== Cover Art Support ======
 [[cover_art_support|Cover Art Support]] [[cover_art_support|Cover Art Support]]
-====== Report: June 29 - July 05 ======+====== Report ​#2: June 29 - July 05 ======
 Last week: [[cover_art_support_r1|June 22 - June 28]] Last week: [[cover_art_support_r1|June 22 - June 28]]
  
Line 13: Line 13:
 https://​github.com/​cardinot/​mixxx/​commit/​7b5be7774cbb3d780796a02edd164d4f2ce4ee2b https://​github.com/​cardinot/​mixxx/​commit/​7b5be7774cbb3d780796a02edd164d4f2ce4ee2b
  
 +===== Duplicated covers in cache (MD5 Hash) =====
  
 +As I said last week, the main idea to sort it out would be using md5 hash. The problem is that MD5 calculation can take a long time and it could bring many performance issues, so we need to avoid doing it as much as possible.
 +
 +In this way, the best choice would be doing it just during the *search* and storing it in database. So, now we have a new column in cover_cache table (md5) and we also added it in the tableview, avoiding doing extra queries (as we were doing for cover location).
 +
 +As all covers have a md5hash, now we are using the md5hash as key for our covercache (QPixmapCache) instead of doing it using coverLocation,​ because just the covers loaded from track directory have coverLocation. So, we did not need a QHash to maintain the md5 hashes.
 +
 +===== Using a delay when the selection changes very fast =====
 +
 +Firstly I thought that it could happens just when the user is holding down an arrow key. So, the simplest and more efficient solution that I found was to use the *keyevent* and *keyeventrelease* to check if the user is holding the arrow key and it would be enough to handle the loadCoverArt signal.
 +
 +However, *daschuer* showed me that it would not work for Midi mapping.
 +https://​github.com/​mixxxdj/​mixxx/​blob/​831a1450a70ea5d19b6a44fed6b6eb71d8ba9d0b/​res/​controllers/​Hercules-DJ-Console-Mk4-scripts.js#​L59
 +
 +Thus, a better approach would be using some kind of timer to know if the user is "​stopped"​ in the same row for a definite time (I chose 0.1s). And it's also more efficient because it ensures that we'll only call loadCoverArt() after finish quick selections.
 +https://​github.com/​cardinot/​mixxx/​commit/​ab39f640619116c7ac07b21168e991aa9b121470
 +
 +===== Remove pixmap from pixmapFound() signal =====
 +
 +In order to avoid loading heavy pixmaps in the QT signal queue, the pixmap is being passed as reference in CoverCache::​requestPixmap().
 +
 +===== Unit test =====
 +
 +After the changes in the "​loading and searching"​ steps due to the implementation of the md5hash, all tests were  reviewed to make that everything was working properly. I also add some tests for the md5hash.
 +
 +As now we are using the “album” and “base track” names to try finding a cover, I also added a test to check if everything works with UTF8 chars.
 +
 +===== Redesign dlgtrackinfo to show covers =====
 +
 +As all other info about a track, the editions of covers will also be done in dlgtrackinfo. The problem is that the current layout is not ideal, because usually small screens (netbooks) have a very low height (600px) and now it has 635px, which is already too long and awful for these users. In this way, I think that the best thing to do is redesign the layout and try using more "​width"​ instead of "​height"​ (as other softwares do).
 +
 +It's also the first issue for the second pull request.
 +https://​github.com/​cardinot/​mixxx/​commits/​coverArtSupport_2
 +
 +====== Issues for the next week ======
 +
 +Next week: [[cover_art_support_r3|July 06 - July 12]]
 +
 +===== Second pull request =====
 +
 +  * redesign dlgtrackinfo to show covers
 +  * load cover in dlgtrackinfo
 +  * edit cover in dlgtrackinfo
 +  * warning box for “reload metadata” action
 +
 +==== Warning box for “reload metadata” action ====
 +
 +if user reload id3tags from file, and it has a different cover from the current cover, we could use a simple warning box to ask if the user really want to reload the cover art from id3tag. We could also show both covers in this box.
cover_art_support_r2.txt · Last modified: 2014/07/06 22:29 by cardinot