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


The newest iteration looks good to me. I'd still suggest that someone at least 
a bit experienced in UI design should review the UI.

- Matěj Laitl


On April 13, 2012, 7:42 p.m., Daniel Faust wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104513/
> -----------------------------------------------------------
> 
> (Updated April 13, 2012, 7:42 p.m.)
> 
> 
> Review request for Amarok.
> 
> 
> 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
> -----
> 
>   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/
> Still not perfect but better, I hope
>   http://git.reviewboard.kde.org/r/104513/s/527/
> 
> 
> Thanks,
> 
> Daniel Faust
> 
>

_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel

Reply via email to