[Amarok] 610b5a8 Revert "Add embedded cover art support"
Leo Franchi
lfranchi at kde.org
Fri Feb 26 20:54:15 CET 2010
commit 610b5a84cf367fc217c605fcd8bf9f304b7909fe
Author: Leo Franchi <lfranchi at kde.org>
Date: Fri Feb 26 14:51:57 2010 -0500
Revert "Add embedded cover art support"
As discussed on the mailing list, there are multiple problems with this patch. Besides the several technical issues, which Dan, Bart, and Jeff all bring up, there is the fact that we have been in feature freeze for 2 weeks, and this does not belong right now.
Andreas, we still want this feature, and we will be merging it back in as soon as 2.3 final is tagged. However, you can see the multiple comments that people have had, and things that we would like to be changed (e.g. no taglib code in sqlmeta).
Anyway, regardless of the technical quality, this still breaks our feature freeze for the 2.3.0 release, and given the ease with which we can develop in parallel in git, we have no reason to violate our freeze for this.
This reverts commit a0c4a6c706d6f4f6cf22740bfa4aec768a7c4b87.
Revert "One more for you, good friend ChangeLog."
This reverts commit 3be4fe9b8f4adca3d2f67db14b1f55240e679c80.
CCMAIL: amarok-devel at kde.org
CCMAIL: ecroy at gmx.net
diff --git a/ChangeLog b/ChangeLog
index 8d1f739..cc6f64f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,8 +5,6 @@ Amarok ChangeLog
VERSION 2.3
FEATURES:
- * Support for embedded cover art. Patch by Andreas L. <ecroy at gmx.net>.
- (BR 176402)
* Podcast channels and episodes can be dragged to add them to other
providers. (BR 195704)
* Trackaction buttons are now available in the label for the current track
diff --git a/src/collection/sqlcollection/ScanManager.cpp b/src/collection/sqlcollection/ScanManager.cpp
index 8fb4a3f..a21c6e5 100644
--- a/src/collection/sqlcollection/ScanManager.cpp
+++ b/src/collection/sqlcollection/ScanManager.cpp
@@ -799,15 +799,6 @@ XmlParseJob::run()
directoryData.append( data );
currentDir = url.directory();
}
-
- //the boolean attribute 'apic' indicates that the file contains an embedded cover-art image
- //we therefore add the path of the audio file to the list of images for this album
- if( !attrs.value( "apic" ).isEmpty() )
- {
- QList<QPair<QString, QString> > covers;
- covers += qMakePair( attrs.value( "artist" ).toString(), attrs.value( "album" ).toString() );
- processor.addImage( attrs.value( "path" ).toString(), covers );
- }
}
else if( localname == "folder" )
{
diff --git a/src/collection/sqlcollection/ScanResultProcessor.cpp b/src/collection/sqlcollection/ScanResultProcessor.cpp
index b9bd7b6..ce12d48 100644
--- a/src/collection/sqlcollection/ScanResultProcessor.cpp
+++ b/src/collection/sqlcollection/ScanResultProcessor.cpp
@@ -188,10 +188,6 @@ ScanResultProcessor::findBestImagePath( const QList<QString> &paths )
QString goodPath;
foreach( const QString &path, paths )
{
- // skip embedded images
- if( SqlAlbum::isEmbeddedImage( path ) )
- continue;
-
QString file = QFileInfo( path ).fileName();
//prioritize "front"
@@ -238,26 +234,21 @@ ScanResultProcessor::findBestImagePath( const QList<QString> &paths )
if( !goodPath.isEmpty() )
return goodPath;
- //next: pick largest non-embedded image -- often a high-quality blowup of the front
+ //finally: pick largest image -- often a high-quality blowup of the front
//so that people can print it out
- qint64 size = -1;
+ qint64 size = 0;
QString current;
foreach( const QString &path, paths )
{
QFileInfo info( path );
- if( info.size() > size &&
- ! SqlAlbum::isEmbeddedImage( path ) )
+ if( info.size() > size )
{
size = info.size();
current = path;
}
}
-
- //finally: if all available images are embedded, simply pick the first one
- if( size == -1 )
- return paths.first();
-
return current;
+
}
void
diff --git a/src/collection/sqlcollection/SqlMeta.cpp b/src/collection/sqlcollection/SqlMeta.cpp
index 96b9d44..dacce04 100644
--- a/src/collection/sqlcollection/SqlMeta.cpp
+++ b/src/collection/sqlcollection/SqlMeta.cpp
@@ -46,11 +46,6 @@
#include <KLocale>
#include <KSharedPtr>
-//Taglib:
-#include <mpegfile.h>
-#include <id3v2tag.h>
-#include <attachedpictureframe.h>
-
using namespace Meta;
QString
@@ -1118,12 +1113,7 @@ SqlAlbum::image( int size )
//FIXME this cache doesn't differentiate between shadowed/unshadowed
if( m_images.contains( size ) )
- {
- if (isEmbeddedImage( m_images.value( size ) ) )
- return QPixmap::fromImage( loadImageFromTag( m_images.value( size ) ) );
- else
- return QPixmap( m_images.value( size ) );
- }
+ return QPixmap( m_images.value( size ) );
QString result;
@@ -1162,10 +1152,7 @@ SqlAlbum::image( int size )
{
m_hasImage = true;
m_images.insert( size, result );
- if (isEmbeddedImage( m_images.value( size ) ) )
- return QPixmap::fromImage( loadImageFromTag( result ) );
- else
- return QPixmap( result );
+ return QPixmap( result );
}
// Cover fetching runs in another thread. If there is a retrieved cover
@@ -1424,13 +1411,8 @@ SqlAlbum::createScaledImage( QString path, int size ) const
// Don't overwrite if it already exists
if( !QFile::exists( cachedImagePath ) )
{
- QImage img;
- if ( isEmbeddedImage( path ) )
- img = loadImageFromTag( path );
- else
- img.load( path );
-
- if( img.isNull() )
+ QImage img( path );
+ if( img.isNull() )
return QString();
// resize and save the image
@@ -1442,49 +1424,6 @@ SqlAlbum::createScaledImage( QString path, int size ) const
return QString();
}
-QImage
-SqlAlbum::loadImageFromTag( const QString path ) const
-{
-#ifdef COMPLEX_TAGLIB_FILENAME
- const wchar_t * encodedName = reinterpret_cast<const wchar_t *>(path.utf16());
-#else
- QByteArray fileName = QFile::encodeName( path );
- const char * encodedName = fileName.constData(); // valid as long as fileName exists
-#endif
-
- TagLib::FileRef fileref;
- TagLib::Tag *tag = 0;
- fileref = TagLib::FileRef( encodedName, true );
-
- if( !fileref.isNull() )
- {
- tag = fileref.tag();
- if ( tag )
- {
- if ( TagLib::MPEG::File *file = dynamic_cast<TagLib::MPEG::File *>( fileref.file() ) )
- {
- if ( !file->ID3v2Tag()->frameListMap()["APIC"].isEmpty() )
- {
- TagLib::ID3v2::FrameList apicList = file->ID3v2Tag()->frameListMap()["APIC"];
- for ( TagLib::ID3v2::FrameList::ConstIterator it = apicList.begin(), end = apicList.end(); it != end; ++it )
- {
- TagLib::ID3v2::AttachedPictureFrame *apicFrame = (TagLib::ID3v2::AttachedPictureFrame *)(*it);
- // take first APIC frame which is a FrontCover
- if ( apicFrame->type() == TagLib::ID3v2::AttachedPictureFrame::FrontCover)
- {
- return QImage::fromData((uchar*)(apicFrame->picture().data()), apicFrame->picture().size());
- }
- }
- // if there was no FrontCover, just take the first picture available
- TagLib::ID3v2::AttachedPictureFrame *apicFrame = (TagLib::ID3v2::AttachedPictureFrame *)(apicList[0]);
- return QImage::fromData((uchar*)(apicFrame->picture().data()), apicFrame->picture().size());
- }
- }
- }
- }
- return QImage::QImage();
-}
-
QString
SqlAlbum::findImage( int size )
{
@@ -1734,20 +1673,6 @@ SqlAlbum::createCapabilityInterface( Meta::Capability::Type type )
return ( m_delegate ? m_delegate->createCapabilityInterface( type, this ) : 0 );
}
-bool
-SqlAlbum::isEmbeddedImage( const QString& path )
-{
- //Entries in the images table are currently either image files or audio files with embedded cover-art images.
- //This function simply checks the file extension and determines whether it is an audio file or not, using the
- //same suffix criteria as in ScanResultProcessor::addTrack for now.
- //Future versions could extend the database scheme to include image type information.
- //No additional checks are made - we trust that the entry was put into the images table for a reason.
-
- QStringList audioFiletypes;
- audioFiletypes << "mp3" << "ogg" << "oga" << "flac" << "wma" << "m4a" << "m4b";
- return audioFiletypes.contains( QFileInfo( path ).suffix().toLower() );
-}
-
//---------------SqlComposer---------------------------------
SqlComposer::SqlComposer( SqlCollection* collection, int id, const QString &name ) : Composer()
@@ -1913,3 +1838,4 @@ SqlYear::tracks()
return TrackList();
}
+
diff --git a/src/collection/sqlcollection/SqlMeta.h b/src/collection/sqlcollection/SqlMeta.h
index 82edd9f..9c4d9cd 100644
--- a/src/collection/sqlcollection/SqlMeta.h
+++ b/src/collection/sqlcollection/SqlMeta.h
@@ -292,8 +292,6 @@ class SqlAlbum : public Meta::Album
virtual Meta::Capability* createCapabilityInterface( Meta::Capability::Type type );
- static bool isEmbeddedImage( const QString& path );
-
//SQL specific methods
int id() const { return m_id; }
@@ -311,7 +309,6 @@ class SqlAlbum : public Meta::Album
void updateImage( const QString path ) const; // Updates the database to ensure the album has the correct path
// Finds or creates a magic value in the database which tells Amarok not to auto fetch an image since it has been explicitly unset.
int unsetImageId() const;
- QImage loadImageFromTag( const QString path ) const;
private:
SqlCollection* m_collection;
diff --git a/utilities/collectionscanner/CollectionScanner.cpp b/utilities/collectionscanner/CollectionScanner.cpp
index 5488b1e..9299ccf 100644
--- a/utilities/collectionscanner/CollectionScanner.cpp
+++ b/utilities/collectionscanner/CollectionScanner.cpp
@@ -43,7 +43,6 @@
#include <QTextStream>
#include <QTime>
#include <QTimer>
-#include <qpixmap.h>
//Taglib:
#include <apetag.h>
@@ -62,8 +61,6 @@
#include <tlist.h>
#include <tstring.h>
#include <vorbisfile.h>
-#include <attachedpictureframe.h>
-#include <tbytevector.h>
#include <audiblefiletyperesolver.h>
#include <realmediafiletyperesolver.h>
@@ -458,6 +455,8 @@ CollectionScanner::scanFiles( const QStringList& entries )
else
{
+ //FIXME: PORT 2.0
+// QList<EmbeddedImage> images;
const AttributeHash attributes = readTags( path );
if( !attributes.empty() )
@@ -468,6 +467,24 @@ CollectionScanner::scanFiles( const QStringList& entries )
if( !covers.contains( cover ) )
covers += cover;
+
+ //FIXME: PORT 2.0
+// foreach( EmbeddedImage image, images )
+// {
+// AttributeHash attributes;
+// if( m_batch && !m_rpath.isEmpty() )
+// {
+// QString rpath = path;
+// rpath.remove( QDir::cleanPath( QDir::currentPath() ) );
+// rpath.prepend( QDir::cleanPath( m_rpath + '/' ) );
+// attributes["path"] = rpath;
+// }
+// else
+// attributes["path"] = path;
+// attributes["hash"] = image.hash();
+// attributes["description"] = image.description();
+// writeElement( "embed", attributes );
+// }
}
}
@@ -604,8 +621,9 @@ CollectionScanner::readTags( const QString &path, TagLib::AudioProperties::ReadS
if ( !file->ID3v2Tag()->frameListMap()["TCMP"].isEmpty() )
compilation = TStringToQString( file->ID3v2Tag()->frameListMap()["TCMP"].front()->toString() ).trimmed();
- if ( !file->ID3v2Tag()->frameListMap()["APIC"].isEmpty() )
- attributes["apic"] = QString("true");
+ //FIXME: Port 2.0
+// if( images )
+// loadImagesFromTag( *file->ID3v2Tag(), *images );
}
// HACK: charset-detector disabled, so all tags assumed utf-8
// TODO: fix charset-detector to detect encoding with higher accuracy
More information about the Amarok-devel
mailing list