From 4c4be0a263c12c7948736a97cbc2a96171dc735a Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 22 Mar 2015 20:24:09 +0900 Subject: [PATCH 1/4] Add a dummy byte to an empty ID3v2 frame to stick to the ID3v2 spec. --- taglib/mpeg/id3v2/id3v2frame.cpp | 3 +++ tests/test_id3v2.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index bee5375a..85e9f66d 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -193,6 +193,9 @@ void Frame::setText(const String &) ByteVector Frame::render() const { ByteVector fieldData = renderFields(); + if(fieldData.isEmpty()) + fieldData = ByteVector("\x00", 1); + d->header->setFrameSize(fieldData.size()); ByteVector headerData = d->header->render(); diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index e0d2d176..c62be280 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -92,6 +92,7 @@ class TestID3v2 : public CppUnit::TestFixture CPPUNIT_TEST(testParseTableOfContentsFrame); CPPUNIT_TEST(testRenderTableOfContentsFrame); CPPUNIT_TEST(testShrinkPadding); + CPPUNIT_TEST(testEmptyFrame); CPPUNIT_TEST_SUITE_END(); public: @@ -1039,6 +1040,35 @@ public: } } + void testEmptyFrame() + { + ScopedFileCopy copy("xing", ".mp3"); + string newname = copy.fileName(); + + { + MPEG::File f(newname.c_str()); + ID3v2::Tag *tag = f.ID3v2Tag(true); + + ID3v2::UrlLinkFrame *frame1 = new ID3v2::UrlLinkFrame( + ByteVector("WOAF\x00\x00\x00\x01\x00\x00\x00", 11)); + tag->addFrame(frame1); + + ID3v2::TextIdentificationFrame *frame2 = new ID3v2::TextIdentificationFrame("TIT2"); + frame2->setText("Title"); + tag->addFrame(frame2); + + f.save(); + } + + { + MPEG::File f(newname.c_str()); + CPPUNIT_ASSERT_EQUAL(true, f.hasID3v2Tag()); + + ID3v2::Tag *tag = f.ID3v2Tag(); + CPPUNIT_ASSERT_EQUAL(String("Title"), tag->title()); + } + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestID3v2); From d33d684fab86c0e4e438fcc31ee1451c1f137811 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 24 Mar 2015 10:31:52 +0900 Subject: [PATCH 2/4] Discard empty ID3v2 frames instead of adding a dummy null byte. --- taglib/mpeg/id3v2/id3v2frame.cpp | 3 --- taglib/mpeg/id3v2/id3v2tag.cpp | 11 +++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 85e9f66d..bee5375a 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -193,9 +193,6 @@ void Frame::setText(const String &) ByteVector Frame::render() const { ByteVector fieldData = renderFields(); - if(fieldData.isEmpty()) - fieldData = ByteVector("\x00", 1); - d->header->setFrameSize(fieldData.size()); ByteVector headerData = d->header->render(); diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 267a45d0..33345407 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -591,12 +591,19 @@ ByteVector ID3v2::Tag::render(int version) const for(FrameList::ConstIterator it = frameList.begin(); it != frameList.end(); it++) { (*it)->header()->setVersion(version); if((*it)->header()->frameID().size() != 4) { - debug("A frame of unsupported or unknown type \'" + debug("An ID3v2 frame of unsupported or unknown type \'" + String((*it)->header()->frameID()) + "\' has been discarded"); continue; } - if(!(*it)->header()->tagAlterPreservation()) + if(!(*it)->header()->tagAlterPreservation()) { + const ByteVector frameData = (*it)->render(); + if(frameData.size() == Frame::headerSize()) { + debug("An empty ID3v2 frame \'" + + String((*it)->header()->frameID()) + "\' has been discarded"); + continue; + } tagData.append((*it)->render()); + } } // Compute the amount of padding, and append that to tagData. From 5f0a7da4810f75e7d0cf49661b1f393172eb2b31 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 24 Mar 2015 10:41:39 +0900 Subject: [PATCH 3/4] Take into account the frame header version when skipping an empty frame. --- taglib/mpeg/id3v2/id3v2tag.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index 33345407..f18dcebc 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -597,7 +597,7 @@ ByteVector ID3v2::Tag::render(int version) const } if(!(*it)->header()->tagAlterPreservation()) { const ByteVector frameData = (*it)->render(); - if(frameData.size() == Frame::headerSize()) { + if(frameData.size() == Frame::headerSize((*it)->header()->version())) { debug("An empty ID3v2 frame \'" + String((*it)->header()->frameID()) + "\' has been discarded"); continue; From 8da00134829f61890c0f847716295d77ac013197 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 18 May 2015 19:03:20 +0900 Subject: [PATCH 4/4] Add a test to check if an empty ID3v2 frame is really skipped. --- tests/test_id3v2.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index c62be280..f4c8517b 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -1066,6 +1066,7 @@ public: ID3v2::Tag *tag = f.ID3v2Tag(); CPPUNIT_ASSERT_EQUAL(String("Title"), tag->title()); + CPPUNIT_ASSERT_EQUAL(true, tag->frameListMap()["WOAF"].isEmpty()); } }