<div>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.</div><div><br></div><div>Stephen</div>
<p style="color: #A0A0A8;">On Friday, February 22, 2013 at 6:19 AM, Lukáš Lalinský wrote:</p>
<blockquote type="cite" style="border-left-style:solid;border-width:1px;margin-left:0px;padding-left:10px;">
<span><div><div><div>I'd like to head more opinions if this is what we want to do. TagLib</div><div>has always been pretty strict regarding tag format correctness.</div><div><br></div><div>I do not personally mind trying to read as much as possible from even</div><div>broken tags.</div><div><br></div><div>Lukas</div><div><br></div><div>On Wed, Dec 19, 2012 at 10:18 PM, Mike Dawson <<a href="mailto:mikeesn@charter.net">mikeesn@charter.net</a>> wrote:</div><blockquote type="cite"><div><div>Hi All,</div><div><br></div><div><br></div><div><br></div><div>We’ve ran across quite a few tracks with empty WCOM frames in id3v2 tags.</div><div><br></div><div> - Contents of WCOM:</div><div><br></div><div> * frameID=WCOM</div><div><br></div><div> * frameSize = 0</div><div><br></div><div> * d->dataLengthIndicator = false</div><div><br></div><div><br></div><div><br></div><div>Taglib rejects the frame as invalid in id3v2framefactory.cpp and</div><div>id3v2tag.cpp and invalidates the complete tag.</div><div><br></div><div>According to //<a href="http://id3.org/id3v2.3.0">id3.org/id3v2.3.0</a> the frame is actually illegal, so taglibs</div><div>reaction is technically correct.</div><div><br></div><div><br></div><div><br></div><div>3.3. ID3v2 frame overview</div><div><br></div><div> A tag must contain at least one frame. A frame must be at least 1 byte</div><div>big, excluding the</div><div><br></div><div> header.</div><div><br></div><div><br></div><div><br></div><div>However, many other parsers skip the bad frame and continue parsing the tag,</div><div>for instance</div><div><br></div><div>Winamp, Mp3tag, iTunes. Would a better method be to check for a valid field</div><div>following the empty field,</div><div><br></div><div>and then continue parsing?</div><div><br></div><div><br></div><div><br></div><div>Although the same issue exists for the code in taglib.git at master, for</div><div>illustration of the problem and not a suggested fix,</div><div><br></div><div>the following two patches to version 1.7 allow the id3v2 tags with empty</div><div>WCOM fields to be parsed:</div><div><br></div><div> ===========================</div><div><br></div><div>diff --git a/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp</div><div>b/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp</div><div><br></div><div>index c531aaa..4fab414 100755</div><div><br></div><div>--- a/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp</div><div><br></div><div>+++ b/taglib-1.7/taglib/mpeg/id3v2/id3v2framefactory.cpp</div><div><br></div><div>@@ -105,9 +105,12 @@ Frame *FrameFactory::createFrame(const ByteVector</div><div>&origData, Header *tagHeader)</div><div><br></div><div><br></div><div><br></div><div> // A quick sanity check -- make sure that the frameID is 4 uppercase</div><div>Latin1</div><div><br></div><div> // characters. Also make sure that there is data in the frame.</div><div><br></div><div>-</div><div><br></div><div>- if(!frameID.size() == (version < 3 ? 3 : 4) ||</div><div><br></div><div>+//MED</div><div><br></div><div>+/* if(!frameID.size() == (version < 3 ? 3 : 4) ||</div><div><br></div><div> header->frameSize() <= uint(header->dataLengthIndicator() ? 4 : 0) ||</div><div><br></div><div>+ */</div><div><br></div><div>+ if(frameID.size() != (version < 3 ? 3 : 4) || /*1.8</div><div>fix*/</div><div><br></div><div>+ (header->dataLengthIndicator() && (header->frameSize() <= uint(4)) )</div><div>|| /*Empty URL fix*/</div><div><br></div><div> header->frameSize() > data.size())</div><div><br></div><div> {</div><div><br></div><div> delete header;</div><div><br></div><div><br></div><div><br></div><div>=======================</div><div><br></div><div>diff --git a/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp</div><div>b/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp</div><div>index 7e8012b..3fb5148 100755</div><div>--- a/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp</div><div>+++ b/taglib-1.7/taglib/mpeg/id3v2/id3v2tag.cpp</div><div>@@ -446,12 +446,14 @@ void ID3v2::Tag::parse(const ByteVector &origData)</div><div> return;</div><div><br></div><div> // Checks to make sure that frame parsed correctly.</div><div>-</div><div>- if(frame->size() <= 0) {</div><div>+//MED - empty URL fix</div><div>+//Add check somehow for frame.d.header.d.dataLengthIndicator()</div><div>+// if(frame->size() <=0 && frame.d.header.d.dataLengthIndicator()</div><div>+/* if(frame->size() <= 0) {</div><div> delete frame;</div><div> return;</div><div> }</div><div>-</div><div>+*/</div><div> frameDataPosition += frame->size() +</div><div>Frame::headerSize(d->header.majorVersion());</div><div> addFrame(frame);</div><div> }</div><div><br></div><div><br></div><div>Thanks for any help.</div><div><br></div><div><br></div><div><br></div><div>_______________________________________________</div><div>taglib-devel mailing list</div><div><a href="mailto:taglib-devel@kde.org">taglib-devel@kde.org</a></div><div><a href="https://mail.kde.org/mailman/listinfo/taglib-devel">https://mail.kde.org/mailman/listinfo/taglib-devel</a></div></div></blockquote><div>_______________________________________________</div><div>taglib-devel mailing list</div><div><a href="mailto:taglib-devel@kde.org">taglib-devel@kde.org</a></div><div><a href="https://mail.kde.org/mailman/listinfo/taglib-devel">https://mail.kde.org/mailman/listinfo/taglib-devel</a></div></div></div></span>
</blockquote>
<div>
<br>
</div>