[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: kdesupport/taglib
From:       Allan Sandfeld Jensen <kde () carewolf ! com>
Date:       2004-07-18 21:57:21
Message-ID: 200407182357.21923.kde () carewolf ! com
[Download RAW message or body]

On Sunday 18 July 2004 22:19, Martijn Pieters wrote:
> 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?
>
Yes. Both APE and ID3v1 are designed to be added to the end of the file, so 
there is only room for one or the other. I consider something like detecting 
multiple tags (stupid tagger that adds id3v1 for an already APE taged file), 
but only in order to strip the extraerroneous tag.

It will change once SV8 formalizes because it allows APEv2 tags after the 
format header, and recommends it for streaming.

> I notice that the MPC::File class also checks for ID3v2 metadata, but
> provides no access to that information. Also by design?
>
Yes, MPC does not allow ID3v2, but the player skips any it encounters, so I 
decided to do the same in the tagger.

> 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).
>
Actually APE only allows one tag of the same name, but it _can_ be a list. I 
will look at how to implement the lists soon.

> 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.
>
Okay, will normalize then. Thanks for the hint.

> 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
>
It would be somewhat easy, the hardest part would be making the new pseudo-tag 
that combines ID3v2 and APE, and which one should win in case of an 
information conflict?

> 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.
>
Okay. I thought there also was a replaygain ID3v2 frame.

> 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.
>
The replaygain values to not belong in the tag anyway. IMHO you ought to put 
them in the audioproperties.

> 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!
>
No, it's really nice with feedback. I am much more woried about Wheeler, I 
ended up commiting the patch without his approval, and even bumbing the 
version number, because it was needed for the KDE feature freeze. I've been 
such a bad boy ;)

`Allan
[prev in list] [next in list] [prev in thread] [next in thread]