[Amarok] a0c4a6c: Add embedded cover art support

[Jeff Mitchell]

I've just looked through the patch.

I have a couple of issues with it.

* A fundamental question is: why do it this way, without modifying the
database? Adding a column or two is pretty trivial. If modifying the
database gives us a far more robust and properly architected solution,
we should do it. I haven't re-looked at the 1.4 code, but my
recollection was that it worked pretty well and used the database to
determine whether an image was embedded or not, instead of the more
hacky "is it an audio file?" check.

* The TagLib stuff does not belong in SqlAlbum. It belongs in Meta::File
or some other place. There isn't any reason that SqlMeta should be tied
to TagLib.

* Some code is copied and pasted from elsewhere, where it was already
hacky. We shouldn't have hacky code twice.

* We're two weeks into feature and string freeze. The whole point of all
of us agreeing to respect these freezes is to prevent unseen errors
creeping in at the last moment, even in innocuous code. Regardless of
how nice it might be to have this feature, this violates that principle
and should wait until trunk is open again.

I think this code needs to be reverted, the design of it needs to be
discussed, and we need to adhere to our freezes and target it at 2.3.1
instead when there can be more testing.

--Jeff

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 196 bytes
Desc: OpenPGP digital signature
Url : http://mail.kde.org/pipermail/amarok-devel/attachments/20100226/9ac1ae32/attachment.sig