Review Request: Some changes to the handling of cover art reading and writing

Daniel Faust hessijames at gmail.com
Fri Apr 13 19:39:09 UTC 2012


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104513/
-----------------------------------------------------------

(Updated April 13, 2012, 7:39 p.m.)


Review request for Amarok.


Changes
-------

-Remove the overwrite option, all covers will be overwritten now
-Modify the cover size option ui
-Use the constant MIN_COVER_SIZE instead of 1024
-Remove the class ASFPicture, cover writing to asf files is supported by taglib since version 1.7 (which amarok git depends on)


Description
-------

This patch includes a list of small changes that are supposed to give the user more control over the cover writing process and unify it for all file formats.

1. The user can set the maximum cover size in the config dialog instead of having a fixed 200px. (BR 279493)
2. The user can set how existing covers should be handled:
   - 'Replace existing front covers' (almost the current behavior except for m4a files and some cases where more than one 'front' cover exists)
   - 'Replace all existing covers'
   - 'Append new cover' (actually it's a prepend, doesn't work with wma files, though; the current behavior for m4a files)
3. Unify the reading of covers. Instead of searching for the biggest cover like it was implemented for some file formats from now on the first 'front' cover will be taken. If there is no 'front' cover, the first 'other' cover will be taken (if present). The 1kb limit is still present.
4. Fix a potential bug where covers couldn't be found in mp3 files if the first cover was neither a 'front' cover nor 'other' or smaller than 1kb.


This addresses bug 279493.
    https://bugs.kde.org/show_bug.cgi?id=279493


Diffs (updated)
-----

  shared/tag_helpers/ASFTagHelper.cpp 93e6031 
  shared/tag_helpers/ID3v2TagHelper.cpp 27e0cf0 
  shared/tag_helpers/MP4TagHelper.cpp faeae0a 
  shared/tag_helpers/TagHelper.h f8e7fd9 
  shared/tag_helpers/VorbisCommentTagHelper.cpp 1fbb437 
  src/amarokconfig.kcfg 5610c4a 
  src/core-impl/collections/db/sql/SqlMeta.cpp e663adf 
  src/dialogs/CollectionSetup.h 3146f17 
  src/dialogs/CollectionSetup.cpp f1b7850 

Diff: http://git.reviewboard.kde.org/r/104513/diff/


Testing
-------

I have tested writing covers with flac, mp3, and wma files.


Screenshots
-----------


  http://git.reviewboard.kde.org/r/104513/s/508/
Mockup of a possible gui for setting cover size. I must say I'm not a big fan of this.
  http://git.reviewboard.kde.org/r/104513/s/514/
Mockup of a possible GUI to configure cover size. I'm not a big fan of this.
  http://git.reviewboard.kde.org/r/104513/s/515/


Thanks,

Daniel Faust

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20120413/851a2fab/attachment.html>


More information about the Amarok-devel mailing list