| Summary: | OSD album cover/embedded image bug | ||
|---|---|---|---|
| Product: | [Applications] amarok | Reporter: | Jeff Kerr <jeff.kerr> |
| Component: | general | Assignee: | Amarok Developers <amarok-bugs-dist> |
| Status: | RESOLVED UNMAINTAINED | ||
| Severity: | normal | CC: | s_reuillon |
| Priority: | NOR | ||
| Version: | 1.4.7 | ||
| Target Milestone: | --- | ||
| Platform: | Ubuntu | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | Patch for the fix | ||
|
Description
Jeff Kerr
2007-10-31 14:20:05 UTC
Proposed fix (tested, works fine):
On save, test to make sure the extracted image is useable by amarok - if it's not, delete it.
metabundle.cpp
MetaBundle::EmbeddedImage::save
...
const Q_LONG s = file.writeBlock( m_data.data(), m_data.size() );
if( s >= 0 && Q_ULONG( s ) == m_data.size() ) {
debug() << "EmbeddedImage::save " << file.name() << endl;
//NEW LINES START HERE...
//Verify that the saved embedded image will work in Amarok
QImage image( file.name() );
if(image.isNull()){
debug() << "EmbeddedImage::save - embedded image saved but unusable - deleting..." << endl;
file.remove();
return false;
}
//NEW LINES END HERE...
return true;
}
...
Please attach the patch as a diff. You can do this by issuing `svn diff > embedded_artwork.patch`. Thanks Created attachment 21964 [details]
Patch for the fix
Created with command:
svn diff > embedded_artwork.patch
I don't think this patch should go in. The problem there might be that Qt wasn't compiled with whatever format the album art is in. For instance, it could be a gif but Qt wasn't compiled with gif support. In this case the "bug" would be rather distribution dependent, and you may "solve" it simply by recompiling Qt into a new package. But deleting the album art is permanent, and we shouldn't be assuming that the user wants that -- what if they want to play that image in iTunes later, from whence it came? A better solution is the following: when <wherever covers are used> is determining the image to use for the cover, check there to see if there is image data there but the image is null according to QImage. If so, force it to use the "no cover" cover. Actually, the fix isn't deleting the image out of the Mp3 file, just from the folder where it extracted it, so it wouldn't affect iTunes. Also, it was my understanding that album art is stored in another folder as well - if you clear all files from your tagfolder folder, it still exists when you run amarok and run the cover manager. On 31 Oct 2007 21:31:00 -0000, Jeff Mitchell <kde-dev@emailgoeshere.com> wrote: [bugs.kde.org quoted mail] Actually, the fix isn't deleting the image out of the Mp3 file, just from the folder where it extracted it, so it wouldn't affect iTunes.<br><br>Also, it was my understanding that album art is stored in another folder as well - if you clear all files from your tagfolder folder, it still exists when you run amarok and run the cover manager. <br><br><div><span class="gmail_quote">On 31 Oct 2007 21:31:00 -0000, <b class="gmail_sendername">Jeff Mitchell</b> <<a href="mailto:kde-dev@emailgoeshere.com">kde-dev@emailgoeshere.com</a>> wrote:</span><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"> ------- You are receiving this mail because: -------<br>You reported the bug, or are watching the reporter.<br><br><a href="http://bugs.kde.org/show_bug.cgi?id=151609">http://bugs.kde.org/show_bug.cgi?id=151609</a><br><br> <br><br><br>------- Additional Comments From kde-dev emailgoeshere com 2007-10-31 22:30 -------<br>I don't think this patch should go in.<br><br>The problem there might be that Qt wasn't compiled with whatever format the album art is in. For instance, it could be a gif but Qt wasn't compiled with gif support. In this case the "bug" would be rather distribution dependent, and you may "solve" it simply by recompiling Qt into a new package. But deleting the album art is permanent, and we shouldn't be assuming that the user wants that -- what if they want to play that image in iTunes later, from whence it came? <br><br>A better solution is the following: when <wherever covers are used> is determining the image to use for the cover, check there to see if there is image data there but the image is null according to QImage. If so, force it to use the "no cover" cover. <br></blockquote></div><br> afaict, no the patch only stops the writing of embedded artwork to our cache if it is invalid. *** Bug 151657 has been marked as a duplicate of this bug. *** Thank you for taking the time to report this bug for Amarok. Amarok 1.4 is now unmaintained and will no longer see any improvements. Because of this, and the massive amount of changes Amarok has undergone throughout the 2.x series of releases, we are closing bugs that no longer apply to the 2.x series due to changes in functionality, the underlying architecture, or a conflict for the vision of Amarok 2. We appreciate the time you took to provide feedback about Amarok 1.4 and will look forward to any feedback you may provided about Amarok 2. Thanks. |