[prev in list] [next in list] [prev in thread] [next in thread]
List: kde-core-devel
Subject: JuK feature/bugfix; thoughts?
From: Michael Pyne <mpyne () purinchu ! net>
Date: 2008-05-13 2:52:23
Message-ID: 200805122252.28109.mpyne () purinchu ! net
[Download RAW message or body]
[Attachment #2 (multipart/mixed)]
[Attachment #4 (multipart/alternative)]
Hi all,
I just wanted to ask if it was possible to get a review of a patch to JuK to
load cover art from an .mp3 file if present, or a image file in the same
directory.
JuK can already load cover art (which is currently managed by a database-type
scheme); the patch in question would expand the sources of displayed cover art
to include the actual .mp3 file and specially named image files in the same
directory. It would not allow for writing to the aforementioned image file or
.mp3 file; all changes to cover art would be made in the database.
The patch is attachment 2 to bug 103118 (
http://bugs.kde.org/show_bug.cgi?id=103118 ). It is also attached.
Reason I'm asking is because we're in feature freeze now (I always pick the
best times to come back in port :) and I don't want to simply try and stretch
the definition of "bugfix" that far.
The patch itself is relatively non-intrusive and read-only (although testing
is always appreciated). Is this OK, or should I wait for trunk to be open for
KDE 4.2?
Regards,
- Michael Pyne
[Attachment #7 (text/html)]
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN" \
"http://www.w3.org/TR/REC-html40/strict.dtd"> <html><head><meta name="qrichtext" \
content="1" /><style type="text/css"> p, li { white-space: pre-wrap; }
</style></head><body style=" font-family:'Consolas'; font-size:11pt; font-weight:400; \
font-style:normal;"> <p style=" margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;">Hi all,</p> \
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"></p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">I just wanted to ask if it was possible to get a review of a patch \
to JuK to load cover art from an .mp3 file if present, or a image file in the same \
directory.</p> <p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"></p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">JuK can already load cover art (which is currently managed by a \
database-type scheme); the patch in question would expand the sources of displayed \
cover art to include the actual .mp3 file and specially named image files in the same \
directory. It would not allow for writing to the aforementioned image file or .mp3 \
file; all changes to cover art would be made in the database.</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">The patch is attachment 2 to \
bug 103118 ( http://bugs.kde.org/show_bug.cgi?id=103118 ). It is also attached.</p> \
<p style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;"></p> <p style=" margin-top:0px; margin-bottom:0px; \
margin-left:0px; margin-right:0px; -qt-block-indent:0; text-indent:0px; \
-qt-user-state:0;">Reason I'm asking is because we're in feature freeze now (I always \
pick the best times to come back in port :) and I don't want to simply try and \
stretch the definition of "bugfix" that far.</p> <p style="-qt-paragraph-type:empty; \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">The patch itself is \
relatively non-intrusive and read-only (although testing is always appreciated). Is \
this OK, or should I wait for trunk to be open for KDE 4.2?</p> <p \
style="-qt-paragraph-type:empty; margin-top:0px; margin-bottom:0px; margin-left:0px; \
margin-right:0px; -qt-block-indent:0; text-indent:0px; -qt-user-state:0;"></p> <p \
style=" margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;">Regards,</p> <p style=" \
margin-top:0px; margin-bottom:0px; margin-left:0px; margin-right:0px; \
-qt-block-indent:0; text-indent:0px; -qt-user-state:0;"> - Michael \
Pyne</p></body></html>
["cover-art.patch" (text/x-patch)]
Index: coverinfo.h
===================================================================
--- coverinfo.h (revision 807128)
+++ coverinfo.h (working copy)
@@ -1,6 +1,9 @@
/***************************************************************************
copyright : (C) 2004 Nathan Toone
email : nathan@toonetown.com
+
+ copyright : (C) 2008 Michael Pyne
+ email : michael.pyne@kdemail.net
***************************************************************************/
/***************************************************************************
@@ -15,13 +18,12 @@
#ifndef COVERINFO_H
#define COVERINFO_H
-#include <qimage.h>
-//Added by qt3to4:
-#include <QPixmap>
-
#include "filehandle.h"
#include "covermanager.h"
+class QImage;
+class QPixmap;
+
class CoverInfo
{
friend class FileHandle;
@@ -58,9 +60,11 @@
private:
QString coverLocation(CoverSize size) const;
bool convertOldStyleCover() const;
+ QImage scaleCoverToThumbnail(const QImage &image) const;
FileHandle m_file;
bool m_hasCover;
+ bool m_hasAttachedCover;
bool m_haveCheckedForCover;
mutable coverKey m_coverKey;
mutable bool m_needsConverting;
Index: playlistitem.cpp
===================================================================
--- playlistitem.cpp (revision 807128)
+++ playlistitem.cpp (working copy)
@@ -21,10 +21,12 @@
#include <QPixmap>
#include <QFileInfo>
+
#include "collectionlist.h"
#include "musicbrainzquery.h"
#include "tag.h"
#include "coverinfo.h"
+#include "covermanager.h"
#include "tagtransactionmanager.h"
PlaylistItemList PlaylistItem::m_playingItems; // static
@@ -89,8 +91,15 @@
int offset = playlist()->columnOffset();
- if((column - offset) == CoverColumn && d->fileHandle.coverInfo()->hasCover())
+ // Don't use hasCover here because that may dig into the track itself.
+ // Besides, we really just want to know if the cover manager has a cover
+ // for the track.
+
+ if((column - offset) == CoverColumn &&
+ d->fileHandle.coverInfo()->coverId() != CoverManager::NoMatch)
+ {
return ℑ
+ }
if(column == playlist()->leftColumn() &&
m_playingItems.contains(const_cast<PlaylistItem *>(this)))
Index: coverinfo.cpp
===================================================================
--- coverinfo.cpp (revision 807128)
+++ coverinfo.cpp (working copy)
@@ -1,6 +1,6 @@
/***************************************************************************
copyright : (C) 2004 Nathan Toone
- : (C) 2005 Michael Pyne <michael.pyne@kdemail.net>
+ : (C) 2005, 2008 Michael Pyne <michael.pyne@kdemail.net>
email : nathan@toonetown.com
***************************************************************************/
@@ -29,13 +29,21 @@
#include <QHBoxLayout>
#include <QEvent>
#include <QFile>
+#include <QFileInfo>
#include <QDesktopWidget>
+#include <taglib/mpegfile.h>
+#include <taglib/tstring.h>
+#include <taglib/id3v2tag.h>
+#include <taglib/attachedpictureframe.h>
+
#include "collectionlist.h"
#include "playlistsearch.h"
#include "playlistitem.h"
#include "tag.h"
+using namespace TagLib;
+
struct CoverPopup : public QWidget
{
CoverPopup(const QPixmap &image, const QPoint &p) :
@@ -64,6 +72,7 @@
CoverInfo::CoverInfo(const FileHandle &file) :
m_file(file),
m_hasCover(false),
+ m_hasAttachedCover(false),
m_haveCheckedForCover(false),
m_coverKey(CoverManager::NoMatch),
m_needsConverting(false)
@@ -74,7 +83,7 @@
bool CoverInfo::hasCover()
{
if(m_haveCheckedForCover)
- return m_hasCover;
+ return m_hasCover || m_hasAttachedCover;
m_haveCheckedForCover = true;
@@ -98,15 +107,40 @@
m_needsConverting = true;
}
+ // Check if it's embedded in the file itself.
+
+ QByteArray filePath = QFile::encodeName(m_file.absFilePath());
+ MPEG::File mpegFile(filePath.constData(), true, AudioProperties::Accurate);
+ ID3v2::Tag *id3tag = mpegFile.ID3v2Tag(false);
+
+ if(!id3tag)
+ return m_hasCover;
+
+ // Look for attached picture frames.
+ ID3v2::FrameList frames = id3tag->frameListMap()["APIC"];
+ m_hasAttachedCover = !frames.isEmpty();
+
+ if(m_hasAttachedCover)
+ return true;
+
+ // Look for cover.jpg or cover.png in the directory.
+ QFileInfo fileInfo(m_file.absFilePath());
+ if(QFile::exists(fileInfo.absolutePath() + "/cover.jpg") ||
+ QFile::exists(fileInfo.absolutePath() + "/cover.png"))
+ {
+ m_hasCover = true;
+ }
+
return m_hasCover;
}
void CoverInfo::clearCover()
{
m_hasCover = false;
+ m_hasAttachedCover = false;
- // Yes, we have checked, and we don't have it. ;)
- m_haveCheckedForCover = true;
+ // Re-search for cover since we may still have a different type of cover.
+ m_haveCheckedForCover = false;
m_needsConverting = false;
@@ -186,13 +220,87 @@
if(m_needsConverting)
convertOldStyleCover();
- if(m_coverKey == CoverManager::NoMatch)
+ if(m_hasCover && m_coverKey != CoverManager::NoMatch) {
+ return CoverManager::coverFromId(m_coverKey,
+ size == Thumbnail
+ ? CoverManager::Thumbnail
+ : CoverManager::FullSize);
+ }
+
+ // If m_hasCover is still true we must have a directory cover image.
+ if(m_hasCover) {
+ QFileInfo fileInfo(m_file.absFilePath());
+ QString fileName = fileInfo.absolutePath() + "/cover.jpg";
+
+ QImage cover;
+ if(!cover.load(fileName)) {
+ fileName = fileInfo.absolutePath() + "/cover.png";
+
+ if(!cover.load(fileName))
+ return QPixmap();
+ }
+
+ if(size == Thumbnail)
+ cover = scaleCoverToThumbnail(cover);
+
+ return QPixmap::fromImage(cover);
+ }
+
+ // If we get here, see if there is an embedded cover.
+
+ QByteArray filePath = QFile::encodeName(m_file.absFilePath());
+ MPEG::File mpegFile(filePath.constData(), true, AudioProperties::Accurate);
+ ID3v2::Tag *id3tag = mpegFile.ID3v2Tag(false);
+
+ if(!id3tag)
return QPixmap();
+ // Look for attached picture frames.
+ ID3v2::FrameList frames = id3tag->frameListMap()["APIC"];
+
+ if(frames.isEmpty())
+ return QPixmap();
+
+ // According to the spec attached picture frames have different types.
+ // So we should look for the corresponding picture depending on what
+ // type of image (i.e. front cover, file info) we want. If only 1
+ // frame, just return that (scaled if necessary).
+
+ ID3v2::AttachedPictureFrame *selectedFrame = 0;
+
+ if(frames.size() != 1) {
+ ID3v2::FrameList::Iterator it = frames.begin();
+ for(; it != frames.end(); ++it) {
+ ID3v2::AttachedPictureFrame *frame =
+ static_cast<ID3v2::AttachedPictureFrame *>(*it);
+
+ // Both thumbnail and full size should use FrontCover, as
+ // FileIcon may be too small even for thumbnail.
+ if(frame->type() != ID3v2::AttachedPictureFrame::FrontCover)
+ continue;
+
+ selectedFrame = frame;
+ break;
+ }
+ }
+
+ // If we get here we failed to pick a picture, or there was only one,
+ // so just use the first picture.
+
+ if(!selectedFrame)
+ selectedFrame = static_cast<ID3v2::AttachedPictureFrame *>(frames.front());
+
+ QByteArray pictureData = QByteArray(selectedFrame->picture().data(),
+ selectedFrame->picture().size());
+ QImage attachedImage = QImage::fromData(pictureData);
+
+ kDebug() << "Embedded cover art of size" << attachedImage.size();
+
if(size == Thumbnail)
- return CoverManager::coverFromId(m_coverKey, CoverManager::Thumbnail);
- else
- return CoverManager::coverFromId(m_coverKey, CoverManager::FullSize);
+ attachedImage = scaleCoverToThumbnail(attachedImage);
+
+ QPixmap returnValue = QPixmap::fromImage(attachedImage);
+ return QPixmap::fromImage(attachedImage);
}
void CoverInfo::popup() const
@@ -225,6 +333,11 @@
new CoverPopup(image, QPoint(x, y));
}
+QImage CoverInfo::scaleCoverToThumbnail(const QImage &image) const
+{
+ return image.scaled(80, 80, Qt::KeepAspectRatio, Qt::SmoothTransformation);
+}
+
/**
* DEPRECATED
*/
Index: playlist.cpp
===================================================================
--- playlist.cpp (revision 807128)
+++ playlist.cpp (working copy)
@@ -2161,8 +2161,12 @@
m_rmbEdit->setEnabled(file.fileInfo().isWritable() || selectedItems().count() > 1);
+ // View cover is based on if there is a cover to see. We should only have
+ // the remove cover option if the cover is in our database (and not directly
+ // embedded in the file, for instance).
+
action("viewCover")->setEnabled(file.coverInfo()->hasCover());
- action("removeCover")->setEnabled(file.coverInfo()->hasCover());
+ action("removeCover")->setEnabled(file.coverInfo()->coverId() != CoverManager::NoMatch);
m_rmbMenu->popup(point);
m_currentColumn = column + columnOffset();
["signature.asc" (application/pgp-signature)]
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic