Empty WCOM fields in id3v2 tags

[Lukáš Lalinský]

I'd like to head more opinions if this is what we want to do. TagLib
has always been pretty strict regarding tag format correctness.

I do not personally mind trying to read as much as possible from even
broken tags.

Lukas

On Wed, Dec 19, 2012 at 10:18 PM, Mike Dawson <mikeesn at charter.net> wrote:
> Hi All,
>
>
>
> We’ve ran across quite a few tracks with empty WCOM frames in id3v2 tags.
>
>             - Contents of WCOM:
>
>                         * frameID=WCOM
>
>                         * frameSize = 0
>
>                         * d->dataLengthIndicator = false
>
>
>
> Taglib rejects the frame as invalid in id3v2framefactory.cpp and
> id3v2tag.cpp and invalidates the complete tag.
>
> According to //id3.org/id3v2.3.0 the frame is actually illegal, so taglibs
> reaction is technically correct.
>
>
>
> 3.3. ID3v2 frame overview
>
>     A tag must contain at least one frame. A frame must be at least 1 byte
> big, excluding the
>
>     header.
>
>
>
> However, many other parsers skip the bad frame and continue parsing the tag,
> for instance
>
> Winamp, Mp3tag, iTunes. Would a better method be to check for a valid field
> following the empty field,
>
> and then continue parsing?
>
>
>
> Although the same issue exists for the code in taglib.git at master, for
> illustration of the problem and not a suggested fix,
>
> the following two patches to version 1.7 allow the id3v2 tags with empty
> WCOM fields to be parsed:
>
>  ===========================
>
> diff --git a/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp
> b/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp
>
> index c531aaa..4fab414 100755
>
> --- a/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp
>
> +++ b/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp
>
> @@ -105,9 +105,12 @@ Frame *FrameFactory::createFrame(const ByteVector
> &origData, Header *tagHeader)
>
>
>
>    // A quick sanity check -- make sure that the frameID is 4 uppercase
> Latin1
>
>    // characters.  Also make sure that there is data in the frame.
>
> -
>
> -  if(!frameID.size() == (version < 3 ? 3 : 4) ||
>
> +//MED
>
> +/*  if(!frameID.size() == (version < 3 ? 3 : 4) ||
>
>       header->frameSize() <= uint(header->dataLengthIndicator() ? 4 : 0) ||
>
> + */
>
> +  if(frameID.size() != (version < 3 ? 3 : 4) ||                      /*1.8
> fix*/
>
> +     (header->dataLengthIndicator() &&  (header->frameSize() <= uint(4)) )
> || /*Empty URL fix*/
>
>       header->frameSize() > data.size())
>
>    {
>
>      delete header;
>
>
>
> =======================
>
> diff --git a/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp
> b/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp
> index 7e8012b..3fb5148 100755
> --- a/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp
> +++ b/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp
> @@ -446,12 +446,14 @@ void ID3v2::Tag::parse(const ByteVector &origData)
>        return;
>
>      // Checks to make sure that frame parsed correctly.
> -
> -    if(frame->size() <= 0) {
> +//MED - empty URL fix
> +//Add check somehow for frame.d.header.d.dataLengthIndicator()
> +// if(frame->size() <=0 && frame.d.header.d.dataLengthIndicator()
> +/*    if(frame->size() <= 0) {
>        delete frame;
>        return;
>      }
> -
> +*/
>      frameDataPosition += frame->size() +
> Frame::headerSize(d->header.majorVersion());
>      addFrame(frame);
>    }
>
>
> Thanks for any help.
>
>
>
> _______________________________________________
> taglib-devel mailing list
> taglib-devel at kde.org
> https://mail.kde.org/mailman/listinfo/taglib-devel
>