Do not scan full MPEG file for ID3v2 tag in Fast read style (taglib#968)

This is based on the patch used by VLC, but the full scan is only
skipped when the read style is explicitly set to Fast.
https://code.videolan.org/videolan/vlc/-/blob/master/contrib/src/taglib/0001-Implement-ID3v2-readStyle-avoid-worst-case.patch

taglib/mpeg/mpegfile.cpp

@@ -122,30 +122,30 @@ bool MPEG::File::isSupported(IOStream *stream)
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-MPEG::File::File(FileName file, bool readProperties, Properties::ReadStyle) :
+MPEG::File::File(FileName file, bool readProperties, Properties::ReadStyle readStyle) :
  TagLib::File(file),
  d(std::make_unique<FilePrivate>())
 {
  if(isOpen())
- read(readProperties);
+ read(readProperties, readStyle);
 }
 
 MPEG::File::File(FileName file, ID3v2::FrameFactory *frameFactory,
- bool readProperties, Properties::ReadStyle) :
+ bool readProperties, Properties::ReadStyle readStyle) :
  TagLib::File(file),
  d(std::make_unique<FilePrivate>(frameFactory))
 {
  if(isOpen())
- read(readProperties);
+ read(readProperties, readStyle);
 }
 
 MPEG::File::File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
- bool readProperties, Properties::ReadStyle) :
+ bool readProperties, Properties::ReadStyle readStyle) :
  TagLib::File(stream),
  d(std::make_unique<FilePrivate>(frameFactory))
 {
  if(isOpen())
- read(readProperties);
+ read(readProperties, readStyle);
 }
 
 MPEG::File::~File() = default;
@@ -450,11 +450,11 @@ bool MPEG::File::hasAPETag() const
 // private members
 ////////////////////////////////////////////////////////////////////////////////
 
-void MPEG::File::read(bool readProperties)
+void MPEG::File::read(bool readProperties, Properties::ReadStyle readStyle)
 {
  // Look for an ID3v2 tag
 
- d->ID3v2Location = findID3v2();
+ d->ID3v2Location = findID3v2(readStyle);
 
  if(d->ID3v2Location >= 0) {
  d->tag.set(ID3v2Index, new ID3v2::Tag(this, d->ID3v2Location, d->ID3v2FrameFactory));
@@ -487,7 +487,7 @@ void MPEG::File::read(bool readProperties)
  ID3v1Tag(true);
 }
 
-offset_t MPEG::File::findID3v2()
+offset_t MPEG::File::findID3v2(Properties::ReadStyle readStyle)
 {
  if(!isValid())
  return -1;
@@ -500,6 +500,9 @@ offset_t MPEG::File::findID3v2()
  if(readBlock(headerID.size()) == headerID)
  return 0;
 
+ if(readStyle == Properties::Fast)
+ return -1;
+
  const Header firstHeader(this, 0, true);
  if(firstHeader.isValid())
  return -1;

taglib/mpeg/mpegfile.h

@@ -74,7 +74,8 @@ namespace TagLib {
  * Constructs an MPEG file from \a file. If \a readProperties is true the
  * file's audio properties will also be read.
  *
- * \note In the current implementation, \a propertiesStyle is ignored.
+ * If \a propertiesStyle is not Fast, the file will be scanned
+ * completely if no ID3v2 tag or MPEG sync code is found at the start.
  *
  * \deprecated This constructor will be dropped in favor of the one below
  * in a future version.
@@ -89,7 +90,8 @@ namespace TagLib {
  * If this file contains and ID3v2 tag the frames will be created using
  * \a frameFactory.
  *
- * \note In the current implementation, \a propertiesStyle is ignored.
+ * If \a propertiesStyle is not Fast, the file will be scanned
+ * completely if no ID3v2 tag or MPEG sync code is found at the start.
  */
  // BIC: merge with the above constructor, kept for source compatibility
  File(FileName file, ID3v2::FrameFactory *frameFactory,
@@ -106,7 +108,8 @@ namespace TagLib {
  * If this file contains and ID3v2 tag the frames will be created using
  * \a frameFactory.
  *
- * \note In the current implementation, \a propertiesStyle is ignored.
+ * If \a propertiesStyle is not Fast, the file will be scanned
+ * completely if no ID3v2 tag or MPEG sync code is found at the start.
  */
  File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
  bool readProperties = true,
@@ -321,8 +324,8 @@ namespace TagLib {
  static bool isSupported(IOStream *stream);
 
  private:
- void read(bool readProperties);
- offset_t findID3v2();
+ void read(bool readProperties, Properties::ReadStyle readStyle);
+ offset_t findID3v2(Properties::ReadStyle readStyle);
 
  class FilePrivate;
  std::unique_ptr<FilePrivate> d;

tests/test_mpeg.cpp

@@ -68,6 +68,7 @@ class TestMPEG : public CppUnit::TestFixture
  CPPUNIT_TEST(testEmptyAPE);
  CPPUNIT_TEST(testIgnoreGarbage);
  CPPUNIT_TEST(testExtendedHeader);
+ CPPUNIT_TEST(testReadStyleFast);
  CPPUNIT_TEST_SUITE_END();
 
 public:
@@ -540,6 +541,45 @@ class TestMPEG : public CppUnit::TestFixture
  }
  }
 
+ void testReadStyleFast()
+ {
+ const ScopedFileCopy copy("lame_cbr", ".mp3");
+ {
+ MPEG::File f(copy.fileName().c_str(), true, MPEG::Properties::Fast);
+ CPPUNIT_ASSERT(f.audioProperties());
+ CPPUNIT_ASSERT_EQUAL(1887, f.audioProperties()->lengthInSeconds());
+ CPPUNIT_ASSERT_EQUAL(1887164, f.audioProperties()->lengthInMilliseconds());
+ CPPUNIT_ASSERT_EQUAL(64, f.audioProperties()->bitrate());
+ CPPUNIT_ASSERT_EQUAL(1, f.audioProperties()->channels());
+ CPPUNIT_ASSERT_EQUAL(44100, f.audioProperties()->sampleRate());
+ CPPUNIT_ASSERT(f.isValid());
+ CPPUNIT_ASSERT(f.hasID3v2Tag());
+ CPPUNIT_ASSERT_EQUAL(String(""), f.ID3v2Tag()->title());
+ PropertyMap properties = f.properties();
+ CPPUNIT_ASSERT_EQUAL(String("-1.020000 dB"), properties.value("REPLAYGAIN_TRACK_GAIN").front());
+ CPPUNIT_ASSERT_EQUAL(String("0.920032"), properties.value("REPLAYGAIN_TRACK_PEAK").front());
+ properties["TITLE"] = String("A Title");
+ properties["Artist"] = String("An Artist");
+ f.setProperties(properties);
+ f.save();
+ }
+ {
+ MPEG::File f(copy.fileName().c_str(), true, MPEG::Properties::Fast);
+ CPPUNIT_ASSERT(f.isValid());
+ CPPUNIT_ASSERT(f.hasID3v2Tag());
+ CPPUNIT_ASSERT_EQUAL(String("A Title"), f.ID3v2Tag()->title());
+ CPPUNIT_ASSERT_EQUAL(String("An Artist"), f.ID3v2Tag()->artist());
+ }
+ {
+ MPEG::File f(TEST_FILE_PATH_C("garbage.mp3"), true, MPEG::Properties::Fast);
+ CPPUNIT_ASSERT(f.isValid());
+ // Garbage prevents detection of ID3v2 with fast read style
+ CPPUNIT_ASSERT(!f.hasID3v2Tag());
+ CPPUNIT_ASSERT_EQUAL(static_cast<offset_t>(2255), f.firstFrameOffset());
+ CPPUNIT_ASSERT_EQUAL(static_cast<offset_t>(6015), f.lastFrameOffset());
+ }
+ }
+
 };
 
 CPPUNIT_TEST_SUITE_REGISTRATION(TestMPEG);