> On April 10, 2012, 4:57 p.m., Matěj Laitl wrote: > > I agree that current hard-coded value of 200px is a bit suboptimal. But I > > doubt there is a significant group of users that makes use of multiple > > per-file covers. When I order Amarok to replace track cover, I want Amarok > > to "make this picture the one and only cover". I would suggest raising the > > dafaut size to something more reasonable like 500px (storage is really > > cheap these days) and making it a hidden confing-file-only option > > documented in the handbook. Alternatively, I have a mockup of perhaps > > rather ugly extension to the config dialog that wouldn't get in the way. (I > > will attach soon) > > > > The work of cleaning up cover file reading and writing and fixing its bugs > > is of course welcome. > > Daniel Faust wrote: > >When I order Amarok to replace track cover, I want Amarok to "make this > picture the one and only cover" > That's what I want, too. But it's not the current behavior that's why I > implemented the configuration option. > > >making it a hidden confing-file-only option > The problem with hidden options is that nobody will know they are there. > But still better than no option at all. > > >Mockup of a possible gui for setting cover size. I must say I'm not a > big fan of this > You are right, it isn't perfect but it would be a shame to stop > implementing features because the config dialog gets overcrowded.
> > When I order Amarok to replace track cover, I want Amarok to "make this > > picture the one and only cover" > That's what I want, too. But it's not the current behavior that's why I > implemented the configuration option. I think we can assume that current behaviour is just wrong, or not well thought out. > > making it a hidden confing-file-only option > The problem with hidden options is that nobody will know they are there. But > still better than no option at all. > > Mockup of a possible gui for setting cover size. I must say I'm not a big > > fan of this > You are right, it isn't perfect but it would be a shame to stop implementing > features because the config dialog gets overcrowded. It is not a shame. I mean that well chosen default or some kind of good heuristics that works for 95% serves users better than a zillion of config options. But I'm leaving this up to you. Note that the mockup I presented cannot be used as is - it is basically untranslatable and won't work for right-to-left scripts. - Matěj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/104513/#review12303 ----------------------------------------------------------- On April 10, 2012, 5:01 p.m., Daniel Faust wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/104513/ > ----------------------------------------------------------- > > (Updated April 10, 2012, 5:01 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/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 > >
_______________________________________________ Amarok-devel mailing list Amarok-devel@kde.org https://mail.kde.org/mailman/listinfo/amarok-devel
