Use id3v2 APIC tag for reading albumart

[myth@…]

The attached patch moves the albumart loading into MetaIO, and makes MetaIOID3v2 look for a APIC field to grab the album art from, and fallback to picking a random image from the directory.

It also adds the album art to the banner shown when visualisers are in fullscreen mode.

[myth@…]

the patch

[myth@…]

There's a memoryleak in the change for playbackbox.cpp, it needs to delete the tagger.

[stuartm]

The 'show in fullscreen banner' portion of this patch has been implemented by Paulh in [13235]

[stuartm]

I've attached a patch using taglib and updated for the new visualiser/albumart code. I would appreciate some testing from those people who have albumart embedded in ID3 tags (I don't). You will need to run a scan for albumart to be found and delete existing tracks from the database first if they contain the albumart.

The patch itself is functional, but needs a little work before it's committed.

[myth@…]

Nope, just a lot of ;

2007-04-21 01:35:35.157 Music Scanner - APIC tag found, but missing description.
2007-04-21 01:35:35.177 Music Scanner - APIC tag found, but missing description.
2007-04-21 01:35:35.192 Music Scanner - APIC tag found, but missing description.
2007-04-21 01:35:35.212 Music Scanner - APIC tag found, but missing description.
2007-04-21 01:35:35.256 Music Scanner - APIC tag found, but missing description.
2007-04-21 01:35:44.578 DB Error (music delete artwork):
Query was:
DELETE FROM music_albumart WHERE filename= 'cover.gif' AND directory_id= 0;
No error type from QSqlError?  Strange...
2007-04-21 01:35:44.583 DB Error (music delete artwork):
Query was:
DELETE FROM music_albumart WHERE filename= 'cover.jpg' AND directory_id= 0;
No error type from QSqlError?  Strange...
2007-04-21 01:35:44.586 DB Error (music delete artwork):
Query was:
DELETE FROM music_albumart WHERE filename= 'cover2.jpg' AND directory_id= 0;
No error type from QSqlError?  Strange...

during scanning.

I'd look into it, but not tonight.

[stuartm]

Ok, I can fix the description problem. The ID3v2 specs say that each APIC frame must have a unique description, I assumed that should mean that the field contained a value for a valid tag.

I've updated the patch with the description check disabled. It's not a permanent fix but should at least allow it to find the art.

The other check on which I suspect it may fail is the artwork type. At the moment we only accept 5 types as valid.

[myth@…]

Ah, many rippers/taggers will just add an APIC frame with the cover art, but with no description

Here's an addition to MetaIOTagLib::readAlbumArt (insert around line 327),

                default:
                    VERBOSE(VB_GENERAL, QString ("Music Scanner - APIC tag found "
                                                 "with unsupported type %1").arg(frame->type()));
                    continue;
            }

            artlist.append(art);
        }

        // We didn't get any entries in artlist. If there are any APIC
        // frames, many taggers/rippers just add the coverart but with no
        // description, so let's just grab the first (we could go for biggest?)
        if (artlist.isEmpty () && apicframes.size () >= 1) {
            VERBOSE(VB_GENERAL, "Music Scanner - no useable APIC tags found, "
                    "using first...");
            AlbumArtImage art;
            AttachedPictureFrame *frame = static_cast<AttachedPictureFrame*> (*apicframes.begin ());
            art.imageType = (ImageType)frame->type();
            art.typeName = "Front Cover";
            art.description = TStringToQString(frame->description());
            art.embedded = true;
            artlist.append(art);
        }

Pretty nasty, ie. casting the frame->type() directly to a ImageType? is going to be trouble, but it got me a bit closer. In most cases the 1 apic will have type 0 (other), so instead of horrorcasting, we could just pick the first type 0 frame and default to that if the artlist is empty. But now it crashes because AlbumArtImages::getImageAt(unsigned int) is missing...

'night...

[myth@…]

updated version that works on files with "Other" type images.

[myth@…]

Stuart, I've modified your patch so it works with files that have just a "Other" type image. Basically, anything that's not a known type just gets added as a IT_UNKNOWN, then I added the getImageAt, plus a isImageEmbedded (instead of requiring filename to be "embedded") and voila, all is well.

[paulh]

Just to point out editmetadata.cpp will also have to be changed. At the moment it uses the filename to get the images and to update the image type in the DB. Probably should use the albumart_id to change the type in AlbumArtImage::saveImageType()

Also line 374 in mainvisual.cpp should be

if (visMode != 2 && fullScreen && albumArt.isNull()) 

There's no point in 3 of us working on this so I will withdraw gracefully from this one :-).

[myth@…]

The more the merrier! I'll fix editmetadata (unless someone beats me to it).

[stuartm]

eskil - I'm working on it right now.

[stuartm]

Updated with several fixes and improvments

[stuartm]

I've attached the latest version of the patch, with a few fixes and changes. I'm not sure I've got all the required editmetadata changes so that might need a look. For the purposes of editmetadata I opted to ignore any embedded images, at least for now.

Before I commit it, there is one known issue to be addressed. I've got a few files which contain empty APIC frames and so I'll add something to ignore them, maybe even delete them from the tag.

[stuartm]

Added image size sanity check

[stuartm]

(In [13318]) Adds support for reading album art from ID3v2 tags.

Closes #2942