From: Todd Z. <tm...@po...> - 2006-09-24 16:19:25
Attachments:
gtkpod-coverart.diff
|
It seems that id3_read_apic() just looks for the first APIC frame and uses that for cover art if it's found. As an ID3 tag may contain multiple APIC frames that each have different picture types this doesn't seem like quite the right thing to do. Ideally only APIC frames of type 3, Cover (front), should be used. Of course, as I understand it, iTunes uses 0, Other as the picture type when it adds cover art. I'm guessing there is other software out there that copies this behavior as well. So, attached is a patch that will loop through the APIC frames in a tag and only set the cover art for picture type 3 or 0. This will use whichever is found first, though it might be nicer to have a preference for type 3, as it's the more correct type to use. -- Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp ====================================================================== The Constitution continues to remain no threat to our current form of government. -- Joseph Sobran |
From: Jorg S. <Jor...@gm...> - 2006-09-26 15:21:48
|
Todd Zullinger wrote: > It seems that id3_read_apic() just looks for the first APIC frame and > uses that for cover art if it's found. As an ID3 tag may contain > multiple APIC frames that each have different picture types this > doesn't seem like quite the right thing to do. Ideally only APIC > frames of type 3, Cover (front), should be used. Of course, as I > understand it, iTunes uses 0, Other as the picture type when it adds > cover art. I'm guessing there is other software out there that copies > this behavior as well. > > So, attached is a patch that will loop through the APIC frames in a > tag and only set the cover art for picture type 3 or 0. This will use > whichever is found first, though it might be nicer to have a > preference for type 3, as it's the more correct type to use. I had no idea... Thanks for the patch! I can confirm that iTunes uses type 0 and I also added code to prefer type 3 over type 0 as you suggested. Cheers, JCS. |
From: Todd Z. <tm...@po...> - 2006-09-26 16:56:40
Attachments:
gtkpod-coverart2.diff
|
Jorg Schuler wrote: > I had no idea... Thanks for the patch! I can confirm that iTunes > uses type 0 and I also added code to prefer type 3 over type 0 as > you suggested. Cool. Thanks! One thing, shouldn't the break in the test for type 0 be removed or moved to the test for type 3? If an APIC frame with type 0 happens to come before a type 3, then the type 0 will be used and the type 3 won't ever be seen. (Re)moving the break statement works for me testing with APIC frames of types 0 and 3 in various orders. -- Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp ====================================================================== I never forget a face, but in your case I'll be glad to make an exception. -- Groucho Marx |
From: Jorg S. <Jor...@gm...> - 2006-09-27 14:22:37
|
Todd Zullinger wrote: > Jorg Schuler wrote: >> I had no idea... Thanks for the patch! I can confirm that iTunes >> uses type 0 and I also added code to prefer type 3 over type 0 as >> you suggested. > > Cool. Thanks! > > One thing, shouldn't the break in the test for type 0 be removed or > moved to the test for type 3? If an APIC frame with type 0 happens to > come before a type 3, then the type 0 will be used and the type 3 > won't ever be seen. (Re)moving the break statement works for me > testing with APIC frames of types 0 and 3 in various orders. Thanks -- I haven't even seen the break statement :-( Cheers, JCS. |