[kde-doc-english] [amarok] /: Some changes to the handling of cover art reading and writing

Daniel Faust hessijames at gmail.com
Sun Apr 29 10:05:17 UTC 2012


Git commit 69c568773360ba70f11334a1c70deae0cacb5eff by Daniel Faust.
Committed on 29/04/2012 at 11:51.
Pushed by dfaust into branch 'master'.

Some changes to the handling of cover art reading and writing

The maximum dimensions for embedded covers are now configurable.
When writing covers to files, all existing covers will be replaced.

FEATURE: 279493
FIXED-IN: 2.6
REVIEW: 104513
GUI: New configuration option for the maximum cover size in the 'local collection' tab

M  +2    -0    ChangeLog
M  +55   -213  shared/tag_helpers/ASFTagHelper.cpp
M  +34   -45   shared/tag_helpers/ID3v2TagHelper.cpp
M  +17   -17   shared/tag_helpers/MP4TagHelper.cpp
M  +2    -0    shared/tag_helpers/TagHelper.h
M  +4    -31   shared/tag_helpers/VorbisCommentTagHelper.cpp
M  +4    -0    src/amarokconfig.kcfg
M  +2    -2    src/core-impl/collections/db/sql/SqlMeta.cpp
M  +29   -12   src/dialogs/CollectionSetup.cpp
M  +7    -3    src/dialogs/CollectionSetup.h

http://commits.kde.org/amarok/69c568773360ba70f11334a1c70deae0cacb5eff

diff --git a/ChangeLog b/ChangeLog
index b1676c6..1e6145e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -4,6 +4,7 @@ Amarok ChangeLog
 
 Version 2.6-Beta 1
   FEATURES:
+    * The maximum dimensions for embedded covers are now configurable. (BR 279493)
     * Small configuration dialog for iPods that shows troubleshooting information
       and allows to change iPod name.
     * Improved usability of iPod playlists: iPod collection automatically transfers
@@ -38,6 +39,7 @@ Version 2.6-Beta 1
       vice versa. (BR 142579)
 
   CHANGES:
+    * When writing covers to files, all existing covers will be replaced.
     * PulseAudio status in diagnostic dialog.
     * optional libgpod dependency raised to 0.8.2 to support newest iPods.
     * Amarok now prevents accidental unmounting of iPods in (small) time-frames
diff --git a/shared/tag_helpers/ASFTagHelper.cpp b/shared/tag_helpers/ASFTagHelper.cpp
index 93e6031..2f4555b 100644
--- a/shared/tag_helpers/ASFTagHelper.cpp
+++ b/shared/tag_helpers/ASFTagHelper.cpp
@@ -28,167 +28,6 @@
 using namespace Meta::Tag;
 
 
-class ASFPicture
-{
-    public:
-        enum PictureType
-        {
-            Other                       = 0,
-            PNGIcon                     = 1,
-            OtherIcon                   = 2,
-            FrontCover                  = 3,
-            BackCover                   = 4,
-            Leaflet                     = 5,
-            Media                       = 6,
-            LeadArtist                  = 7,
-            Artist                      = 8,
-            Conductor                   = 9,
-            Band                        = 10,
-            Composer                    = 11,
-            Lyricist                    = 12,
-            RecordingStudioOrLocation   = 13,
-            RecordingSession            = 14,
-            Performance                 = 15,
-            CaptureFromMovieOrVideo     = 16,
-            BrightColoredFish           = 17,
-            Illustration                = 18,
-            BandLogo                    = 19,
-            PublisherLogo               = 20
-        };
-
-        ASFPicture();
-        ASFPicture( const TagLib::ByteVector &bv );
-
-        ASFPicture &operator=( const ASFPicture &picture );
-
-        PictureType type() const;
-        QString mimeType() const;
-        QString description() const;
-
-        QByteArray data() const;
-        int size() const;
-#ifndef UTILITIES_BUILD
-        ASFPicture( const QImage &image );
-        QImage image() const;
-        TagLib::ByteVector toByteVector() const;
-#endif  //UTILITIES_BUILD
-    private:
-        PictureType m_type;
-        QString m_mimeType;
-        QString m_description;
-        QByteArray m_data;
-};
-
-
-ASFPicture::ASFPicture()
-          : m_type( PublisherLogo )             // We won't use such
-{
-}
-
-ASFPicture::ASFPicture( const TagLib::ByteVector &bv )
-{
-    if( bv.isEmpty() || bv.isNull() )
-    {
-        m_type = PublisherLogo;
-        return;
-    }
-
-    qint32 pos = 0;
-    const char *data = bv.data();
-    m_type = PictureType( *data );
-    pos++;
-    quint32 pictSize = *( quint32 *)(data + pos );
-    pos += 4;
-    m_mimeType = QString::fromUtf16( ( const ushort *)( data + pos ) );
-    pos += 2 * ( m_mimeType.length() + 1 );
-    m_description = QString::fromUtf16( ( const ushort *)( data + pos ) );
-    pos += 2 * ( m_description.length() + 1 );
-    m_data = QByteArray( data + pos, pictSize );
-}
-
-ASFPicture &
-ASFPicture::operator=( const ASFPicture &picture )
-{
-    m_type = picture.m_type;
-    m_mimeType = picture.m_mimeType;
-    m_description = picture.m_description;
-    m_data = picture.m_data;
-
-    return *this;
-}
-
-ASFPicture::PictureType
-ASFPicture::type() const
-{
-    return m_type;
-}
-
-QString
-ASFPicture::mimeType() const
-{
-    return m_mimeType;
-}
-
-QString
-ASFPicture::description() const
-{
-    return m_description;
-}
-
-QByteArray
-ASFPicture::data() const
-{
-    return m_data;
-}
-
-int
-ASFPicture::size() const
-{
-    return m_data.size();
-}
-
-#ifndef UTILITIES_BUILD
-ASFPicture::ASFPicture( const QImage &image )
-          : m_type( FrontCover )
-          , m_mimeType( "image/jpeg" )
-{
-    QBuffer buffer( &m_data );
-
-    buffer.open( QIODevice::WriteOnly );
-    image.save( &buffer, "JPEG" );
-    buffer.close();
-}
-
-QImage
-ASFPicture::image() const
-{
-    return QImage::fromData( m_data );
-}
-
-TagLib::ByteVector
-ASFPicture::toByteVector() const
-{
-    QByteArray data;
-
-    uchar type = m_type;
-    qint32 pictSize = m_data.size();
-
-    QBuffer buffer( &data );
-    buffer.open( QIODevice::WriteOnly );
-
-    buffer.write( ( const char *)&type, 1 );
-    buffer.write( ( const char *)&pictSize, 4 );
-    buffer.write( ( const char *)m_mimeType.utf16(), ( m_mimeType.length() + 1 ) * 2 );
-    buffer.write( ( const char *)m_description.utf16(), ( m_description.length() + 1 ) * 2 );
-    buffer.write( m_data.data(), pictSize );
-
-    buffer.close();
-
-    return TagLib::ByteVector( data.data(), data.size() );
-}
-
-#endif  //UTILITIES_BUILD
-
 ASFTagHelper::ASFTagHelper( TagLib::Tag *tag, TagLib::ASF::Tag *asfTag, Amarok::FileType fileType )
             : TagHelper( tag, fileType )
             , m_tag( asfTag )
@@ -240,10 +79,10 @@ ASFTagHelper::tags() const
                     if( cover->type() != TagLib::ASF::Attribute::BytesType )
                         continue;
 
-                    ASFPicture pict( cover->toByteVector() );
-                    if( ( pict.type() == ASFPicture::FrontCover ||
-                          pict.type() == ASFPicture::Other ) &&
-                        pict.size() > 1024 )
+                    TagLib::ASF::Picture pict = cover->toPicture();
+                    if( ( pict.type() == TagLib::ASF::Picture::FrontCover ||
+                        pict.type() == TagLib::ASF::Picture::Other ) &&
+                        pict.dataSize() > MIN_COVER_SIZE )
                     {
                         data.insert( field, true );
                         break;
@@ -317,6 +156,7 @@ ASFTagHelper::hasEmbeddedCover() const
     TagLib::ASF::AttributeListMap map = m_tag->attributeListMap();
     TagLib::String name = fieldName( Meta::valHasCover );
     for( TagLib::ASF::AttributeListMap::ConstIterator it = map.begin(); it != map.end(); ++it )
+    {
         if( it->first == name )
         {
             TagLib::ASF::AttributeList coverList = it->second;
@@ -325,15 +165,16 @@ ASFTagHelper::hasEmbeddedCover() const
                 if( cover->type() != TagLib::ASF::Attribute::BytesType )
                     continue;
 
-                    ASFPicture pict( cover->toByteVector() );
-                    if( ( pict.type() == ASFPicture::FrontCover ||
-                          pict.type() == ASFPicture::Other ) &&
-                        pict.size() > 1024 )
-                    {
-                        return true;
-                    }
+                TagLib::ASF::Picture pict = cover->toPicture();
+                if( ( pict.type() == TagLib::ASF::Picture::FrontCover ||
+                      pict.type() == TagLib::ASF::Picture::Other ) &&
+                    pict.dataSize() > MIN_COVER_SIZE )
+                {
+                    return true;
+                }
             }
         }
+    }
 
     return false;
 }
@@ -344,76 +185,77 @@ ASFTagHelper::embeddedCover() const
     TagLib::ASF::AttributeListMap map = m_tag->attributeListMap();
     TagLib::String name = fieldName( Meta::valHasCover );
 
-    ASFPicture other, front;
-    bool hasFront = false, hasOther = false;
-    int maxSize = 1024;
+    TagLib::ASF::Picture cover, otherCover;
+    bool hasCover = false, hasOtherCover = false;
 
     for( TagLib::ASF::AttributeListMap::ConstIterator it = map.begin(); it != map.end(); ++it )
+    {
         if( it->first == name )
         {
             TagLib::ASF::AttributeList coverList = it->second;
-            for( TagLib::ASF::AttributeList::ConstIterator cover = coverList.begin(); cover != coverList.end(); ++cover )
+            for( TagLib::ASF::AttributeList::ConstIterator it = coverList.begin(); it != coverList.end(); ++it )
             {
-                if( cover->type() != TagLib::ASF::Attribute::BytesType )
+                if( it->type() != TagLib::ASF::Attribute::BytesType )
                     continue;
 
-                ASFPicture pict( cover->toByteVector() );
-                if( pict.size() < maxSize )
+                TagLib::ASF::Picture pict = it->toPicture();
+
+                if( pict.dataSize() < MIN_COVER_SIZE )
                     continue;
 
-                if( pict.type() == ASFPicture::FrontCover )
+                if( pict.type() == TagLib::ASF::Picture::FrontCover )
                 {
-                    front = pict;
-                    maxSize = pict.size();
-                    hasFront = true;
+                    cover = pict;
+                    hasCover = true;
                 }
-                else if( pict.type() == ASFPicture::Other )
+                else if( pict.type() == TagLib::ASF::Picture::Other )
                 {
-                    other = pict;
-                    maxSize = pict.size();
-                    hasOther = true;
+                    otherCover = pict;
+                    hasOtherCover = true;
                 }
             }
         }
+    }
 
-    if( !hasFront && !hasOther )
+    if( !hasCover && hasOtherCover )
+    {
+        cover = otherCover;
+        hasCover = true;
+    }
+
+    if( !hasCover )
         return QImage();
 
-    //If Front and Other covers have the same size, we should use the Front one.
-    if( hasFront && !hasOther )
-        return front.image();
-    else if( !hasFront && hasOther )
-        return other.image();
-    else if( front.size() >= other.size() )
-        return front.image();
-    //else
-    return other.image();
+    return QImage::fromData( ( uchar * ) cover.picture().data(), cover.picture().size() );
 }
 
 bool
 ASFTagHelper::setEmbeddedCover( const QImage &cover )
 {
-    TagLib::String name = fieldName( Meta::valHasCover );
+    QByteArray bytes;
+    QBuffer buffer( &bytes );
 
-    ASFPicture picture( cover );
-    bool stored = false;
+    buffer.open( QIODevice::WriteOnly );
 
-    TagLib::ASF::AttributeList coverList = m_tag->attributeListMap()[name];
-    for( uint i = 0; i < coverList.size(); i++ )
+    if( !cover.save( &buffer, "JPEG" ) )
     {
-        if( coverList[i].type() != TagLib::ASF::Attribute::BytesType )
-            continue;
-
-        ASFPicture pict( coverList[i].toByteVector() );
-        if( pict.type() == ASFPicture::FrontCover )
-        {
-            coverList[i] = TagLib::ASF::Attribute( picture.toByteVector() );
-            stored = true;
-        }
+        buffer.close();
+        return false;
     }
 
-    if( !stored )
-        m_tag->addAttribute( name, TagLib::ASF::Attribute( picture.toByteVector() ) );
+    buffer.close();
+
+    TagLib::String name = fieldName( Meta::valHasCover );
+
+    // remove all covers
+    m_tag->removeItem( name );
+
+    // add new cover
+    TagLib::ASF::Picture picture;
+    picture.setPicture( TagLib::ByteVector( bytes.data(), bytes.count() ) );
+    picture.setType( TagLib::ASF::Picture::FrontCover );
+    picture.setMimeType( "image/jpeg" );
+    m_tag->addAttribute( name, TagLib::ASF::Attribute( picture.render() ) );
 
     return true;
 }
diff --git a/shared/tag_helpers/ID3v2TagHelper.cpp b/shared/tag_helpers/ID3v2TagHelper.cpp
index 27e0cf0..f748901 100644
--- a/shared/tag_helpers/ID3v2TagHelper.cpp
+++ b/shared/tag_helpers/ID3v2TagHelper.cpp
@@ -102,7 +102,7 @@ ID3v2TagHelper::tags() const
 
                 if( ( frame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover ||
                       frame->type() == TagLib::ID3v2::AttachedPictureFrame::Other ) &&
-                    frame->picture().size() > 1024 ) // must be at least 1kb
+                    frame->picture().size() > MIN_COVER_SIZE ) // must be at least 1kb
                 {
                     data.insert( Meta::valHasCover, true );
                 }
@@ -331,15 +331,21 @@ ID3v2TagHelper::render() const
 bool
 ID3v2TagHelper::hasEmbeddedCover() const
 {
-    TagLib::ByteVector field = fieldName( Meta::valHasCover ).toCString();
-    if( m_tag->frameListMap().contains( field ) )
+    TagLib::ID3v2::FrameList apicList = m_tag->frameListMap()[fieldName( Meta::valHasCover ).toCString()];
+
+    for( TagLib::ID3v2::FrameList::ConstIterator it = apicList.begin(); it != apicList.end(); ++it )
     {
-        TagLib::ID3v2::AttachedPictureFrame *frame =
-                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( m_tag->frameListMap()[field].front() );
-        return frame && ( frame->picture().size() > 1024 ) &&
-               ( frame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover ||
-                 frame->type() == TagLib::ID3v2::AttachedPictureFrame::Other );
+        TagLib::ID3v2::AttachedPictureFrame *currFrame =
+                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( *it );
+
+        if( currFrame->picture().size() < MIN_COVER_SIZE )
+            continue;
+
+        if( currFrame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover ||
+            currFrame->type() == TagLib::ID3v2::AttachedPictureFrame::Other )
+            return true;
     }
+
     return false;
 }
 
@@ -347,47 +353,34 @@ QImage
 ID3v2TagHelper::embeddedCover() const
 {
     TagLib::ID3v2::FrameList apicList = m_tag->frameListMap()[fieldName( Meta::valHasCover ).toCString()];
-    TagLib::ID3v2::AttachedPictureFrame *frontCover = NULL;
+    TagLib::ID3v2::AttachedPictureFrame *cover = NULL;
     TagLib::ID3v2::AttachedPictureFrame *otherCover = NULL;
-    TagLib::ID3v2::AttachedPictureFrame *coverToUse = NULL;
 
-    uint maxSize = 1024;     // ignore images that are too small
-
-    for( TagLib::ID3v2::FrameList::ConstIterator iter = apicList.begin(); iter != apicList.end(); ++iter )
+    for( TagLib::ID3v2::FrameList::ConstIterator it = apicList.begin(); it != apicList.end(); ++it )
     {
         TagLib::ID3v2::AttachedPictureFrame *currFrame =
-                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( *iter );
+                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( *it );
 
-        if( currFrame->picture().size() < maxSize )
+        if( currFrame->picture().size() < MIN_COVER_SIZE )
             continue;
 
         if( currFrame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover )
         {
-            frontCover = currFrame;
-            maxSize = currFrame->picture().size();
+            cover = currFrame;
         }
         else if( currFrame->type() == TagLib::ID3v2::AttachedPictureFrame::Other )
         {
             otherCover = currFrame;
-            maxSize = currFrame->picture().size();
         }
     }
 
-    if( !frontCover && !otherCover )
+    if( !cover && otherCover )
+        cover = otherCover;
+
+    if( !cover )
         return QImage();
 
-    //If Front and Other covers have the same size, we should use the Front one.
-    if( frontCover && !otherCover )
-        coverToUse = frontCover;
-    else if( !frontCover && otherCover )
-        coverToUse = otherCover;
-    else if( frontCover->picture().size() >= otherCover->picture().size() )
-        coverToUse = frontCover;
-    else
-        coverToUse = otherCover;
-
-    return QImage::fromData( ( uchar * )( coverToUse->picture().data() ),
-                             coverToUse->picture().size() );
+    return QImage::fromData( ( uchar * )( cover->picture().data() ), cover->picture().size() );
 }
 
 bool
@@ -410,27 +403,23 @@ ID3v2TagHelper::setEmbeddedCover( const QImage &cover )
     TagLib::ID3v2::FrameList apicList = m_tag->frameListMap()[field];
     TagLib::ID3v2::AttachedPictureFrame *frontCover = NULL;
 
-    for( TagLib::ID3v2::FrameList::ConstIterator iter = apicList.begin(); iter != apicList.end(); ++iter )
+    // remove covers
+    TagLib::List<TagLib::ID3v2::AttachedPictureFrame*> backedUpPictures;
+    for( TagLib::ID3v2::FrameList::ConstIterator it = apicList.begin(); it != apicList.end(); ++it )
     {
         TagLib::ID3v2::AttachedPictureFrame *currFrame =
-                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( *iter );
+                dynamic_cast< TagLib::ID3v2::AttachedPictureFrame * >( *it );
 
-        if( currFrame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover )
-        {
-            frontCover = currFrame;
-            break;
-        }
-    }
-
-    if( !frontCover )
-    {
-        frontCover = new TagLib::ID3v2::AttachedPictureFrame( field );
-        frontCover->setType( TagLib::ID3v2::AttachedPictureFrame::FrontCover );
-        m_tag->addFrame( frontCover );
+        m_tag->removeFrame( currFrame, false );
     }
 
+    // add new cover
+    frontCover = new TagLib::ID3v2::AttachedPictureFrame( field );
     frontCover->setMimeType( "image/jpeg" );
     frontCover->setPicture( TagLib::ByteVector( bytes.data(), bytes.count() ) );
+    frontCover->setType( TagLib::ID3v2::AttachedPictureFrame::FrontCover );
+    m_tag->addFrame( frontCover );
+
     return true;
 }
 #endif  //UTILITIES_BUILD
diff --git a/shared/tag_helpers/MP4TagHelper.cpp b/shared/tag_helpers/MP4TagHelper.cpp
index faeae0a..5052cfb 100644
--- a/shared/tag_helpers/MP4TagHelper.cpp
+++ b/shared/tag_helpers/MP4TagHelper.cpp
@@ -65,7 +65,7 @@ MP4TagHelper::tags() const
             {
                 TagLib::MP4::CoverArtList coverList = it->second.toCoverArtList();
                 for( TagLib::MP4::CoverArtList::ConstIterator it = coverList.begin(); it != coverList.end(); ++it )
-                    if( it->data().size() > 1024 )
+                    if( it->data().size() > MIN_COVER_SIZE )
                     {
                         data.insert( field, true );
                         break;
@@ -151,13 +151,17 @@ MP4TagHelper::hasEmbeddedCover() const
     TagLib::MP4::ItemListMap map = m_tag->itemListMap();
     TagLib::String name = fieldName( Meta::valHasCover );
     for( TagLib::MP4::ItemListMap::ConstIterator it = map.begin(); it != map.end(); ++it )
+    {
         if( it->first == name )
         {
             TagLib::MP4::CoverArtList coverList = it->second.toCoverArtList();
             for( TagLib::MP4::CoverArtList::ConstIterator cover = coverList.begin(); cover != coverList.end(); ++cover )
-                if( cover->data().size() > 1024 )
+            {
+                if( cover->data().size() > MIN_COVER_SIZE )
                     return true;
+            }
         }
+    }
 
     return false;
 }
@@ -167,24 +171,20 @@ MP4TagHelper::embeddedCover() const
 {
     TagLib::MP4::ItemListMap map = m_tag->itemListMap();
     TagLib::String name = fieldName( Meta::valHasCover );
-
-    TagLib::ByteVector coverData;
-    bool foundCover = false;
-    quint64 maxSize = 1024;
     for( TagLib::MP4::ItemListMap::ConstIterator it = map.begin(); it != map.end(); ++it )
+    {
         if( it->first == name )
         {
             TagLib::MP4::CoverArtList coverList = it->second.toCoverArtList();
             for( TagLib::MP4::CoverArtList::Iterator cover = coverList.begin(); cover != coverList.end(); ++cover )
-                if( cover->data().size() > maxSize )
-                {
-                    maxSize = cover->data().size();
-                    foundCover = true;
-                    coverData = cover->data();
-                }
+            {
+                if( cover->data().size() > MIN_COVER_SIZE )
+                    return QImage::fromData( ( uchar * ) cover->data().data(), cover->data().size() );
+            }
         }
+    }
 
-    return foundCover ? QImage::fromData( ( uchar * ) coverData.data(), coverData.size() ) : QImage();
+    return QImage();
 }
 
 bool
@@ -203,12 +203,12 @@ MP4TagHelper::setEmbeddedCover( const QImage &cover )
 
     buffer.close();
 
-    //Or just replace all covers with the new one?
-    TagLib::MP4::CoverArtList covers = m_tag->itemListMap()[fieldName( Meta::valHasCover )].toCoverArtList();
-    covers.prepend( TagLib::MP4::CoverArt( TagLib::MP4::CoverArt::JPEG,
-                                           TagLib::ByteVector( bytes.data(), bytes.count() ) ) );
+    TagLib::MP4::CoverArtList covers;
+
+    covers.append( TagLib::MP4::CoverArt( TagLib::MP4::CoverArt::JPEG, TagLib::ByteVector( bytes.data(), bytes.count() ) ) );
 
     m_tag->itemListMap()[fieldName( Meta::valHasCover )] = TagLib::MP4::Item( covers );
+
     return true;
 }
 #endif  //UTILITIES_BUILD
diff --git a/shared/tag_helpers/TagHelper.h b/shared/tag_helpers/TagHelper.h
index f8e7fd9..b1afc7c 100644
--- a/shared/tag_helpers/TagHelper.h
+++ b/shared/tag_helpers/TagHelper.h
@@ -24,6 +24,8 @@
     #define AMAROK_EXPORT
 #endif
 
+#define MIN_COVER_SIZE 1024 // Minimum size for an embedded cover to be loaded
+
 #include "MetaValues.h"
 #include "FileType.h"
 
diff --git a/shared/tag_helpers/VorbisCommentTagHelper.cpp b/shared/tag_helpers/VorbisCommentTagHelper.cpp
index 1fbb437..450679b 100644
--- a/shared/tag_helpers/VorbisCommentTagHelper.cpp
+++ b/shared/tag_helpers/VorbisCommentTagHelper.cpp
@@ -116,7 +116,7 @@ VorbisCommentTagHelper::tags() const
 
             if( ( picture->type() == TagLib::FLAC::Picture::FrontCover ||
                 picture->type() == TagLib::FLAC::Picture::Other ) &&
-                picture->data().size() > 1024 ) // must be at least 1kb
+                picture->data().size() > MIN_COVER_SIZE ) // must be at least 1kb
             {
                 data.insert( Meta::valHasCover, true );
                 break;
@@ -183,7 +183,7 @@ VorbisCommentTagHelper::hasEmbeddedCover() const
 
             if( ( picture->type() == TagLib::FLAC::Picture::FrontCover ||
                 picture->type() == TagLib::FLAC::Picture::Other ) &&
-                picture->data().size() > 1024 ) // must be at least 1kb
+                picture->data().size() > MIN_COVER_SIZE ) // must be at least 1kb
             {
                 return true;
             }
@@ -215,7 +215,7 @@ VorbisCommentTagHelper::embeddedCover() const
 
             if( ( picture->type() == TagLib::FLAC::Picture::FrontCover ||
                 picture->type() == TagLib::FLAC::Picture::Other ) &&
-                picture->data().size() > 1024 ) // must be at least 1kb
+                picture->data().size() > MIN_COVER_SIZE ) // must be at least 1kb
             {
                 QByteArray image_data( picture->data().data(), picture->data().size() );
 
@@ -248,7 +248,7 @@ VorbisCommentTagHelper::embeddedCover() const
             QByteArray image_data_b64( coverList[i].toCString() );
             QByteArray image_data = QByteArray::fromBase64(image_data_b64);
 
-            if( image_data.size() <= 1024 ) // must be at least 1kb
+            if( image_data.size() <= MIN_COVER_SIZE ) // must be at least 1kb
                 continue;
 
             bool success = false;
@@ -287,22 +287,6 @@ VorbisCommentTagHelper::setEmbeddedCover( const QImage &cover )
 
         buffer.close();
 
-        // back up covers
-        TagLib::List<TagLib::FLAC::Picture*> backedUpPictures;
-        const TagLib::List<TagLib::FLAC::Picture*> picturelist = m_flacFile->pictureList();
-        for( TagLib::List<TagLib::FLAC::Picture*>::ConstIterator it = picturelist.begin(); it != picturelist.end(); it++ )
-        {
-            const TagLib::FLAC::Picture *picture = *it;
-
-            TagLib::FLAC::Picture *backedUpPicture = new TagLib::FLAC::Picture();
-            backedUpPicture->setData( picture->data() );
-            backedUpPicture->setMimeType( picture->mimeType() );
-            backedUpPicture->setType( picture->type() );
-            backedUpPicture->setDescription( picture->description() );
-
-            backedUpPictures.append( backedUpPicture );
-        }
-
         // remove all covers
         m_flacFile->removePictures();
 
@@ -313,17 +297,6 @@ VorbisCommentTagHelper::setEmbeddedCover( const QImage &cover )
         newPicture->setType( TagLib::FLAC::Picture::FrontCover );
         m_flacFile->addPicture( newPicture );
 
-        // re-add all the backed-up covers except the front cover
-        for( TagLib::List<TagLib::FLAC::Picture*>::Iterator it = backedUpPictures.begin(); it != backedUpPictures.end(); it++ )
-        {
-            TagLib::FLAC::Picture *picture = *it;
-
-            if( picture->type() != TagLib::FLAC::Picture::FrontCover )
-            {
-                m_flacFile->addPicture( picture );
-            }
-        }
-
         return true;
     }
 
diff --git a/src/amarokconfig.kcfg b/src/amarokconfig.kcfg
index 5610c4a..f2efc0b 100644
--- a/src/amarokconfig.kcfg
+++ b/src/amarokconfig.kcfg
@@ -445,6 +445,10 @@
         <label>Whether changes to the album cover are written back</label>
         <default>false</default>
     </entry>
+    <entry key="Write Back Cover Dimensions" type="Int">
+        <label>Maximum embedded cover dimensions</label>
+        <default>400</default>
+    </entry>
     <entry key="Collection Folders" type="PathList">
         <label>List of folders in the Collection</label>
     </entry>
diff --git a/src/core-impl/collections/db/sql/SqlMeta.cpp b/src/core-impl/collections/db/sql/SqlMeta.cpp
index e663adf..3413a75 100644
--- a/src/core-impl/collections/db/sql/SqlMeta.cpp
+++ b/src/core-impl/collections/db/sql/SqlMeta.cpp
@@ -1631,8 +1631,8 @@ SqlAlbum::setImage( const QImage &image )
     {
         // - scale to cover to a sensible size
         QImage scaledImage( image );
-        if( scaledImage.width() > 200 || scaledImage.height() > 200 )
-            scaledImage = scaledImage.scaled( 200, 200, Qt::KeepAspectRatio, Qt::SmoothTransformation );
+        if( scaledImage.width() > AmarokConfig::writeBackCoverDimensions() || scaledImage.height() > AmarokConfig::writeBackCoverDimensions() )
+            scaledImage = scaledImage.scaled( AmarokConfig::writeBackCoverDimensions(), AmarokConfig::writeBackCoverDimensions(), Qt::KeepAspectRatio, Qt::SmoothTransformation );
 
         // - set the image for each track
         Meta::TrackList myTracks = tracks();
diff --git a/src/dialogs/CollectionSetup.cpp b/src/dialogs/CollectionSetup.cpp
index f1b7850..cc3b756 100644
--- a/src/dialogs/CollectionSetup.cpp
+++ b/src/dialogs/CollectionSetup.cpp
@@ -103,7 +103,7 @@ CollectionSetup::CollectionSetup( QWidget *parent )
     setObjectName( "CollectionSetup" );
     s_instance = this;
 
-    QLabel* descriptionLabel = new QLabel( i18n( 
+    QLabel* descriptionLabel = new QLabel( i18n(
         "These folders will be scanned for "
         "media to make up your collection. You can "
         "right-click on a folder to individually "
@@ -133,13 +133,20 @@ CollectionSetup::CollectionSetup( QWidget *parent )
     m_monitor   = new QCheckBox( i18n("&Watch folders for changes"), this );
     m_writeBack = new QCheckBox( i18n("Write metadata to file"), this );
     m_writeBackStatistics = new QCheckBox( i18n("Write statistics to file"), this );
-    m_writeBackCover = new QCheckBox( i18n("Write covers to file"), this );
+    KHBox* writeBackCoverDimensionsBox = new KHBox( this );
+    m_writeBackCover = new QCheckBox( i18n("Write covers to file, maximum size:"), writeBackCoverDimensionsBox );
+    m_writeBackCoverDimensions = new KComboBox( writeBackCoverDimensionsBox );
+    m_writeBackCoverDimensions->addItem( i18nc("Maximum cover size option","Small (200 px)"), QVariant(200) );
+    m_writeBackCoverDimensions->addItem( i18nc("Maximum cover size option","Medium (400 px)"), QVariant(400) );
+    m_writeBackCoverDimensions->addItem( i18nc("Maximum cover size option","Large (800 px)"), QVariant(800) );
+    m_writeBackCoverDimensions->addItem( i18nc("Maximum cover size option","Huge (1600 px)"), QVariant(1600) );
     m_charset   = new QCheckBox( i18n("&Enable character set detection in ID3 tags"), this );
     connect( m_recursive, SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
     connect( m_monitor  , SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
     connect( m_writeBack, SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
     connect( m_writeBackStatistics, SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
     connect( m_writeBackCover, SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
+    connect( m_writeBackCoverDimensions, SIGNAL( currentIndexChanged( int )), this, SIGNAL( changed() ) );
     connect( m_charset  , SIGNAL( toggled( bool ) ), this, SIGNAL( changed() ) );
 
     m_recursive->setToolTip( i18n( "If selected, Amarok will read all subfolders." ) );
@@ -147,6 +154,7 @@ CollectionSetup::CollectionSetup( QWidget *parent )
     m_writeBack->setToolTip( i18n( "Write meta data changes (including 'stars' rating) back to the original file.\nYou can also prevent writing back by write protecting the file.\nThis might be a good idea if you are currently\nsharing those files via the Internet." ) );
     m_writeBackStatistics->setToolTip( i18n( "Write play-changing statistics (e.g. score, lastplayed, playcount)\nas tags back to the file." ) );
     m_writeBackCover->setToolTip( i18n( "Write changed covers back to the file.\nThis will replace existing embedded covers." ) );
+    m_writeBackCoverDimensions->setToolTip( i18n( "Scale covers down if necessary." ) );
     m_charset->setToolTip(   i18n( "If selected, Amarok will use Mozilla's\nCharacter Set Detector to attempt to automatically guess the\ncharacter sets used in ID3 tags." ) );
 
     m_recursive->setChecked( AmarokConfig::scanRecursively() );
@@ -157,6 +165,11 @@ CollectionSetup::CollectionSetup( QWidget *parent )
     m_writeBackStatistics->setEnabled( writeBack() );
     m_writeBackCover->setChecked( AmarokConfig::writeBackCover() );
     m_writeBackCover->setEnabled( writeBack() );
+    if( m_writeBackCoverDimensions->findData( AmarokConfig::writeBackCoverDimensions() ) != -1 )
+        m_writeBackCoverDimensions->setCurrentIndex( m_writeBackCoverDimensions->findData( AmarokConfig::writeBackCoverDimensions() ) );
+    else
+        m_writeBackCoverDimensions->setCurrentIndex( 1 );
+    m_writeBackCoverDimensions->setEnabled( m_writeBackCover->isEnabled() && m_writeBackCover->isChecked() );
     m_charset->setChecked( AmarokConfig::useCharsetDetector() );
 
     // set the model _after_ constructing the checkboxes
@@ -167,11 +180,11 @@ CollectionSetup::CollectionSetup( QWidget *parent )
     #else
     m_view->setRootIndex( m_model->setRootPath( m_model->myComputer().toString() ) );
     #endif
-    
+
     Collections::Collection *primaryCollection = CollectionManager::instance()->primaryCollection();
     QStringList dirs = primaryCollection ? primaryCollection->property( "collectionFolders" ).toStringList() : QStringList();
     m_model->setDirectories( dirs );
-    
+
     // make sure that the tree is expanded to show all selected items
     foreach( const QString &dir, dirs )
     {
@@ -190,14 +203,16 @@ CollectionSetup::hasChanged() const
 
     m_writeBackStatistics->setEnabled( writeBack() );
     m_writeBackCover->setEnabled( writeBack() );
+    m_writeBackCoverDimensions->setEnabled( m_writeBackCover->isEnabled() && m_writeBackCover->isChecked() );
 
     return
         m_model->directories() != collectionFolders ||
-        m_recursive->isChecked() != recursive() ||
-        m_monitor->isChecked() != monitor() ||
-        m_writeBack->isChecked() != writeBack() ||
-        m_writeBackStatistics->isChecked() != writeBackStatistics() ||
-        m_writeBackCover->isChecked() != writeBackCover() ||
+        m_recursive->isChecked() != AmarokConfig::scanRecursively() ||
+        m_monitor->isChecked() != AmarokConfig::monitorChanges() ||
+        m_writeBack->isChecked() != AmarokConfig::writeBack() ||
+        m_writeBackStatistics->isChecked() != AmarokConfig::writeBackStatistics() ||
+        m_writeBackCover->isChecked() != AmarokConfig::writeBackCover() ||
+        m_writeBackCoverDimensions->itemData(m_writeBackCoverDimensions->currentIndex()).toInt() != AmarokConfig::writeBackCoverDimensions() ||
         m_charset->isChecked() != AmarokConfig::useCharsetDetector();
 }
 
@@ -211,6 +226,8 @@ CollectionSetup::writeConfig()
     AmarokConfig::setWriteBack( writeBack() );
     AmarokConfig::setWriteBackStatistics( writeBackStatistics() );
     AmarokConfig::setWriteBackCover( writeBackCover() );
+    if( writeBackCoverDimensions() > 0 )
+        AmarokConfig::setWriteBackCoverDimensions( writeBackCoverDimensions() );
     AmarokConfig::setUseCharsetDetector( charset() );
 
     Collections::Collection *primaryCollection = CollectionManager::instance()->primaryCollection();
@@ -260,7 +277,7 @@ namespace CollectionFolder {
         const QString path = filePath( index );
         if( isForbiddenPath( path ) )
             flags ^= Qt::ItemIsEnabled; //disabled!
-       
+
         flags |= Qt::ItemIsUserCheckable;
 
         return flags;
@@ -353,7 +370,7 @@ namespace CollectionFolder {
 
         qSort( dirs.begin(), dirs.end() );
 
-        // we need to remove any children of selected items as 
+        // we need to remove any children of selected items as
         // they are redundant when recursive mode is chosen
         if( recursive() )
         {
@@ -426,7 +443,7 @@ namespace CollectionFolder {
     /**
      * Check the logical recursive difference of root and excludePath.
      * For example, if excludePath is a grandchild of root, then this method
-     * will check all of the children of root except the one that is the 
+     * will check all of the children of root except the one that is the
      * parent of excludePath, as well as excludePath's siblings.
      */
     void
diff --git a/src/dialogs/CollectionSetup.h b/src/dialogs/CollectionSetup.h
index 3146f17..23d717a 100644
--- a/src/dialogs/CollectionSetup.h
+++ b/src/dialogs/CollectionSetup.h
@@ -21,6 +21,7 @@
 #define AMAROK_COLLECTIONSETUP_H
 
 #include <KVBox>      //baseclass
+#include <KComboBox>
 
 #include <QCheckBox>
 #include <QFileSystemModel>
@@ -61,16 +62,17 @@ class CollectionSetup : public KVBox
         static CollectionSetup* instance() { return s_instance; }
 
         CollectionSetup( QWidget* );
-        
+
         void writeConfig();
         bool hasChanged() const;
-         
+
         QStringList dirs() const { return m_dirs; }
         bool recursive() const { return m_recursive && m_recursive->isChecked(); }
         bool monitor() const { return m_monitor && m_monitor->isChecked(); }
         bool writeBack() const { return m_writeBack&& m_writeBack->isChecked(); }
         bool writeBackStatistics() const { return m_writeBackStatistics && m_writeBackStatistics->isChecked(); }
         bool writeBackCover() const { return m_writeBackCover && m_writeBackCover->isChecked(); }
+        int writeBackCoverDimensions() const { return m_writeBackCoverDimensions ? m_writeBackCoverDimensions->itemData(m_writeBackCoverDimensions->currentIndex()).toInt() : 0; }
         bool charset() const { return m_charset && m_charset->isChecked(); }
 
         const QString modelFilePath( const QModelIndex &index ) const;
@@ -92,7 +94,9 @@ class CollectionSetup : public KVBox
         QCheckBox *m_writeBack;
         QCheckBox *m_writeBackStatistics;
         QCheckBox *m_writeBackCover;
+        KComboBox *m_writeBackCoverDimensions;
         QCheckBox *m_charset;
+
 };
 
 
@@ -102,7 +106,7 @@ namespace CollectionFolder //just to keep it out of the global namespace
     {
         public:
             Model();
-        
+
             virtual Qt::ItemFlags flags( const QModelIndex &index ) const;
             QVariant data( const QModelIndex& index, int role = Qt::DisplayRole ) const;
             bool setData( const QModelIndex& index, const QVariant& value, int role = Qt::EditRole );


More information about the kde-doc-english mailing list