Empty WCOM fields in id3v2 tags

[Stephen F. Booth]

I agree with Lukas.  In general, I think being lenient on read and strict on write is a good philosophy to follow. In this case I don't think any data loss could occur by handling empty WCOM frames in lieu of rejecting the file.

Stephen  

On Friday, February 22, 2013 at 6:19 AM, Lukáš Lalinský wrote:

> 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 (mailto: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 (http://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 (mailto:taglib-devel at kde.org)
> > https://mail.kde.org/mailman/listinfo/taglib-devel
> >  
>  
> _______________________________________________
> taglib-devel mailing list
> taglib-devel at kde.org (mailto:taglib-devel at kde.org)
> https://mail.kde.org/mailman/listinfo/taglib-devel
>  
>  


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/taglib-devel/attachments/20130222/e99a1c34/attachment.html>