Re: Changes to write ID3v2.3 tags and unsynchronization

[Scott Wheeler]

Hi Jonathan --

Your mail got flagged by the KDE mail server as spam, so perhaps that  
was what was causing problems.  Occasionally it's a bit trigger happy.

I recently handed off maintainership to Lukas, but I'm sure if you  
send the patches along they'll at least be looked at.  Out of  
curiosity, which programs specifically still have problems reading  
ID3v2.4 tags?  I thought that was mostly a solved problem at this point.

-Scott

On Oct 23, 2009, at 8:12 AM, Jonathan Potter wrote:

> Hi Scott,
>
> (I tried to post this message to the list twice but it never showed  
> up, even though I am subscribed and getting emails. Figured I'd send  
> it to you directly in case you were interested)
>
> I've been working with taglib for a little while, integrating it  
> into our product (Directory Opus, a Windows file manager). I'd like  
> to say I think you've all done a great job – it's a really nicely  
> designed library and seems to work very well from what little we've  
> seen of it so far.
>
> After working with it for a while we came to the realisation that we  
> really needed to be able to write ID3v2.3 tags, as it seems like  
> most Windows-based software (including Windows Media Player) still  
> doesn't support ID3v2.4. Therefore I've made a number of changes to  
> taglib to support this (it didn't actually end up being very  
> difficult).
>
> I've also added support for unsynchronisation (both on a per-frame  
> level for 2.4 as well as a whole tag level for 2.3) – although it  
> seems like so few programs properly support unsynchronisation (at  
> least with ID3v2.3) that I'm not sure how useful this is.
>
> I'm just wondering now if anyone is interested in seeing these  
> changes – so far in in-house testing it seems to work ok but I'm  
> obviously not 100% sure it hasn't caused any problems as we're not  
> THAT familiar with the taglib code. So if anyone who is more  
> familiar with taglib would be interested in having a look, please  
> let me know as we'd love any feedback you may have.
>
> Cheers,
> Jonathan Potter
>
>

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

On Fri, Oct 23, 2009 at 8:35 AM, Scott Wheeler <wheeler@...> wrote:
> Out of curiosity, which programs specifically still have problems reading
> ID3v2.4 tags?  I thought that was mostly a solved problem at this point.

Windows Media Player (and indirectly every Windows app from
Microsoft), for example.
--
Lukas Lalinsky
lalinsky@...
_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Michael Smith-59]

>
> On Oct 23, 2009, at 8:12 AM, Jonathan Potter wrote:
>
>> Hi Scott,
>>
>> (I tried to post this message to the list twice but it never showed
>> up, even though I am subscribed and getting emails. Figured I'd send
>> it to you directly in case you were interested)
>>
>> I've been working with taglib for a little while, integrating it
>> into our product (Directory Opus, a Windows file manager). I'd like
>> to say I think you've all done a great job – it's a really nicely
>> designed library and seems to work very well from what little we've
>> seen of it so far.
>>
>> After working with it for a while we came to the realisation that we
>> really needed to be able to write ID3v2.3 tags, as it seems like
>> most Windows-based software (including Windows Media Player) still
>> doesn't support ID3v2.4. Therefore I've made a number of changes to
>> taglib to support this (it didn't actually end up being very
>> difficult).

I'd certainly be interested in that - I looked into what would be
needed a while back, and it didn't look too hard - so please send your
patches to the list! Some of us would certainly be interested in using
it, even if Lukas doesn't think it's appropriate for upstream.


>>
>> I've also added support for unsynchronisation (both on a per-frame
>> level for 2.4 as well as a whole tag level for 2.3) – although it
>> seems like so few programs properly support unsynchronisation (at
>> least with ID3v2.3) that I'm not sure how useful this is.

Yeah, from the point of view of completeness it'd be nice to have this
in the library - but in practice it's simply not very important.

Mike
_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

[Jonathan Potter]

Hi,

Patches are attached. To summarise the changes I've made:

1. id3v2.3 support

The default id3v2 version is set via the ID3DEFVERSION define. The basic
idea is:

- If ID3DEFVERSION is defined as 4, a version 4 tag will always be written -
this is the existing behaviour
- If ID3DEFVERSION is defined as 3, a version 3 tag will be written UNLESS
the file already had a version 4 tag, in which case the version 4 tag will
be preserved

When saving a version 3 tag, any version 4-specific frames will be converted
back to version 3 frames if possible - currently this only happens with the
TDRC frame which is converted back to TYER. If ID3VER3DUMPVER4TAGS is
defined then any version 4 frames which couldn't be converted will be
dropped from the tag.

Note that ID3DEFVERSION is currently defined twice, in id3v2frame.h and
id3v2header.h (this is due to the seeming lack of a common header file for
the whole project).


2. Unsynchronisation

- If ID3VER4UNSYNC is defined, unsynchronisation will be performed on a
per-frame basis if needed (when writing a version 4 tag)
- If ID3VER3UNSYNC is defined, unsynchronisation will be performed on a
whole-tag basis if needed

Added a number of functions in id3v2synchdata.cpp to support this


3. Fixed (mostly by casting) some minor compile issues affecting Windows

- A number of minor compile warnings from Visual Studio were fixed
- A number of x64 (64 bit) compile problems were also fixed


4. Added utility function in mpegfile.cpp (replaceFrame) to automatically
replace any existing frame(s) with a new one.


Please let me know if you have any questions about the changes I've made!

Cheers,
Jon

> -----Original Message-----
> From: mlrsmith@... [mailto:mlrsmith@...] On Behalf Of
> Michael Smith
> Sent: Friday, 23 October 2009 5:57 PM
> To: taglib-devel@...
> Cc: jon@...
> Subject: Re: Changes to write ID3v2.3 tags and unsynchronization
>
> I'd certainly be interested in that - I looked into what would be
> needed a while back, and it didn't look too hard - so please send your
> patches to the list! Some of us would certainly be interested in using
> it, even if Lukas doesn't think it's appropriate for upstream.
>

_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel

On Sat, Oct 24, 2009 at 1:12 AM, Jonathan Potter <listsub@...> wrote:

> 1. id3v2.3 support
>
> The default id3v2 version is set via the ID3DEFVERSION define. The basic
> idea is:
>
> - If ID3DEFVERSION is defined as 4, a version 4 tag will always be written -
> this is the existing behaviour
> - If ID3DEFVERSION is defined as 3, a version 3 tag will be written UNLESS
> the file already had a version 4 tag, in which case the version 4 tag will
> be preserved
>
> When saving a version 3 tag, any version 4-specific frames will be converted
> back to version 3 frames if possible - currently this only happens with the
> TDRC frame which is converted back to TYER. If ID3VER3DUMPVER4TAGS is
> defined then any version 4 frames which couldn't be converted will be
> dropped from the tag.
>
> Note that ID3DEFVERSION is currently defined twice, in id3v2frame.h and
> id3v2header.h (this is due to the seeming lack of a common header file for
> the whole project).

I'm sorry, but I must say that I don't like this. I don't think having
a compile-time option is a good idea. The library should support both
versions and the decision whether to save ID2v2.3 or ID3v2.4 should be
a run-time option.

Also, the way you downgrade ID3v2.4 frames to ID3v2.3 is incomplete.
For TDRC, you should use TYER + TDAT + TIME, not just TYER. See
http://bzr.oxygene.sk/picard/trunk/picard/formats/mutagenext/compatid3.py#l-152
for a more complete implementation.

Regarding text encoding, I think instead of checkEncoding, there
should be a function on Tag that downgrades tags to ID3v2.3 and this
would include changing UTF-8 to UTF-16. You forgot to check the
version-aware encoding in unsynchronizedlyricsframe,
synchronizedlyricsframe and generalencapsulatedobjectframe (which
currently doesn't call checkEncoding at all).

And the downgrade code should also make sure that all text frames have
only a single value. Saving text frames with multiple values is not
supported by ID3v2.3.

Some other comments about the code:
 - to render ID3v2.3 header size, use ByteVector::fromUInt instead of
a custom code.
 - the code doesn't follow the style in the rest of TagLib
 - no unit tests

> 2. Unsynchronisation
>
> - If ID3VER4UNSYNC is defined, unsynchronisation will be performed on a
> per-frame basis if needed (when writing a version 4 tag)
> - If ID3VER3UNSYNC is defined, unsynchronisation will be performed on a
> whole-tag basis if needed
>
> Added a number of functions in id3v2synchdata.cpp to support this

TagLib never encoded the frame data. Is there a strong reason to add
it? TagLib used to not even support reading synch-encoded tags until
1.5. I think I'd prefer to keep the data without encoding for
compatibility reasons.

> 3. Fixed (mostly by casting) some minor compile issues affecting Windows
>
> - A number of minor compile warnings from Visual Studio were fixed
> - A number of x64 (64 bit) compile problems were also fixed

It would be nice to submit these separately, as they can go to 1.6.1.
The v2.3 support can't.

> 4. Added utility function in mpegfile.cpp (replaceFrame) to automatically
> replace any existing frame(s) with a new one.

--
Lukas Lalinsky
lalinsky@...
_______________________________________________
taglib-devel mailing list
taglib-devel@...
https://mail.kde.org/mailman/listinfo/taglib-devel