This is an OpenPGP/MIME signed message (RFC 2440 and 3156)
--------------enigE56D09E425D9A80C9651D6FA
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit

Allan Sandfeld Jensen wrote:
> CVS commit by carewolf: 
> 
> Add support for reading musepack(mpc)-files and parsing APE-tags(v1 and v2).
> Bumb version to have something to check for in JuK and kfile_mpc

Cool, APEv2 support, just what I was looking for.

Some questions about this (I am not too familiar with the MusePack 
format, so bear with me if some of my questions are MusePack FAQs):

The FLAC and MPEG implementations each use a proxy Tag implementation so 
all metadata formats can be checked for a given property (title, etc), 
and, most importantly, can be written out to all present tag formats 
when changed. The MPC::File class on the other hand will only give 
access to either the APE or the ID3v1 information if no APE information 
was present. Is this by design?

I notice that the MPC::File class also checks for ID3v2 metadata, but 
provides no access to that information. Also by design?

Also, I thought that APE tags, like Xiph (Ogg) tags, could occur 
multiple times in a file. The Xiph tag implementation gives access to 
the information as a list, while your code only gives access to the last 
occurence of a tag. For example, if there are multiple 'title' tags, 
your code, only the last occurence is exposed, while the Xiph tag gives 
access to all values (but uses the first).

The APE spec recommends readers to treat tags as case-insensitive; bt 
your implementation doesn't normalize the tag key. Of course, this is 
not against the spec, but the XiphComment class does ensure all keys are 
uppercase (although that spec states that tag keys are 
case-insensitive). The APE spec further forbids there to be any 2 
key-value pairs where the key only differs in case.

Some MP3 players, particularly FooBar2K on Windows, also support APEv2 
tags on MP3 files. Not having deep-dived into that part of your commit 
yet, how easy would it be for the APE code to be reused for MPEG::File 
as well? (I note that the ID3* implementations are then shared by 3, and 
the APE class by 2 different file formats; possibly a refactoring of 
their namespaces and file locations is in order). MPEG APEv2 support is 
currently a wishlist item: http://bugs.kde.org/show_bug.cgi?id=84666

I am currently implementing ReplayGain support 
(http://bugs.kde.org/show_bug.cgi?id=85134), and the MP3Gain tool that 
creates ReplayGain information for MP3 files writes the information to 
APEv2 tags, so I'll need APE support if taglib is to support that very 
popular tool. I may look into retooling the APE support into MPEG::File 
myself because of that.

I'll certainly also add ReplayGain support to your code, if Scott 
Wheeler decides to accept my patches, the MPC format certainly supports 
it. However, a quick scan of the MusePack pages tells me that ReplayGain 
values may well be stored in a proprietary format (not APE or ID3v1) 
which means that without a MPC::Tag proxy class I cannot support it; at 
least not without ugly hacks or a redesign of the architecture.

Sorry if I sound overly critical, I have just had my head rather deep 
into the library. :) Even though I have had no hand in the current lib 
yet, thanks for your contributions, they certainly help me!

Martijn Pieters

--------------enigE56D09E425D9A80C9651D6FA
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: OpenPGP digital signature
Content-Disposition: attachment; filename="signature.asc"

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.4 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFA+tta3xaj2GOvgP0RArbWAJ4504eNS2E15yBrF7DlEHW+XoEpfgCgigXX
V0i3lbVOK8MwbOR5guVnMo4=
=+GWs
-----END PGP SIGNATURE-----

--------------enigE56D09E425D9A80C9651D6FA--