Fix parsing of ID3v2.2 frames (taglib#1228)

ufleisch · May 14, 2024 · 3b5b08e · 3b5b08e
1 parent ebf4c5b
commit 3b5b08e
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 3 deletions.
diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp
@@ -843,8 +843,10 @@ void ID3v2::Tag::parse(const ByteVector &origData)
  break;
  }
 
- Frame *frame = d->factory->createFrame(data.mid(frameDataPosition),
- &d->header);
+ const ByteVector origData = data.mid(frameDataPosition);
+ const Header *tagHeader = &d->header;
+ unsigned int headerVersion = tagHeader->majorVersion();
+ Frame *frame = d->factory->createFrame(origData, tagHeader);
 
  if(!frame)
  return;
@@ -856,7 +858,15 @@ void ID3v2::Tag::parse(const ByteVector &origData)
  return;
  }
 
- frameDataPosition += frame->size() + frame->headerSize();
+ if(frame->header()->version() == headerVersion) {
+ frameDataPosition += frame->size() + frame->headerSize();
+ } else {
+ // The frame was converted to another version, e.g. from 2.2 to 2.4.
+ // We must advance the frame data position according to the original
+ // frame, not the converted frame because its header size might differ.
+ Frame::Header origHeader(origData, headerVersion);
+ frameDataPosition += origHeader.frameSize() + origHeader.size();
+ }
  addFrame(frame);
  }
 

diff --git a/tests/data/itunes10.mp3 b/tests/data/itunes10.mp3
diff --git a/tests/test_mpeg.cpp b/tests/test_mpeg.cpp
@@ -71,6 +71,7 @@ class TestMPEG : public CppUnit::TestFixture
  CPPUNIT_TEST(testIgnoreGarbage);
  CPPUNIT_TEST(testExtendedHeader);
  CPPUNIT_TEST(testReadStyleFast);
+ CPPUNIT_TEST(testID3v22Properties);
  CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -618,6 +619,56 @@ class TestMPEG : public CppUnit::TestFixture
  }
  }
 
+ void testID3v22Properties()
+ {
+ ScopedFileCopy copy("itunes10", ".mp3");
+
+ MPEG::File f(copy.fileName().c_str());
+ PropertyMap expectedProperties(SimplePropertyMap{
+ {"ALBUM", {"Album"}},
+ {"ALBUMARTIST", {"Album Artist"}},
+ {"ALBUMARTISTSORT", {"Sort Album Artist"}},
+ {"ALBUMSORT", {"Sort Album"}},
+ {"ARTIST", {"Artist"}},
+ {"ARTISTSORT", {"Sort Artist"}},
+ {"BPM", {"180"}},
+ {"COMMENT", {"Comments"}},
+ {"COMMENT:ITUNPGAP", {"1"}},
+ {"COMPILATION", {"1"}},
+ {"COMPOSER", {"Composer"}},
+ {"COMPOSERSORT", {"Sort Composer"}},
+ {"DATE", {"2011"}},
+ {"DISCNUMBER", {"1/2"}},
+ {"GENRE", {"Heavy Metal"}},
+ {"LYRICS", {"Lyrics"}},
+ {"SUBTITLE", {"Description"}},
+ {"TITLE", {"iTunes10MP3"}},
+ {"TITLESORT", {"Sort Name"}},
+ {"TRACKNUMBER", {"1/10"}},
+ {"WORK", {"Grouping"}}
+ });
+ expectedProperties.addUnsupportedData("APIC");
+ expectedProperties.addUnsupportedData("UNKNOWN/RVA");
+
+ PropertyMap properties = f.properties();
+ if (expectedProperties != properties) {
+ CPPUNIT_ASSERT_EQUAL(expectedProperties.toString(), properties.toString());
+ }
+ CPPUNIT_ASSERT(expectedProperties == properties);
+
+ const String PICTURE_KEY("PICTURE");
+ CPPUNIT_ASSERT_EQUAL(StringList(PICTURE_KEY), f.complexPropertyKeys());
+ auto pictures = f.complexProperties(PICTURE_KEY);
+ CPPUNIT_ASSERT_EQUAL(1U, pictures.size());
+ auto picture = pictures.front();
+ CPPUNIT_ASSERT_EQUAL(String("image/png"), picture.value("mimeType").toString());
+ CPPUNIT_ASSERT(picture.value("description").toString().isEmpty());
+ CPPUNIT_ASSERT_EQUAL(String("Other"), picture.value("pictureType").toString());
+ auto data = picture.value("data").toByteVector();
+ CPPUNIT_ASSERT(data.startsWith("\x89PNG\x0d\x0a\x1a\x0a"));
+ CPPUNIT_ASSERT_EQUAL(2315U, data.size());
+ }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);