Unify File constructors with ID3v2::FrameFactory parameter (#1196)

Make constructors consistent so that the FrameFactory is at the
end and optional. Mark the alternative constructors as deprecated.

taglib/fileref.cpp

@@ -135,7 +135,7 @@ namespace
     File *file = nullptr;
 
     if(ext == "MP3" || ext == "MP2" || ext == "AAC")
-      file = new MPEG::File(stream, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle);
+      file = new MPEG::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(ext == "OGG")
       file = new Ogg::Vorbis::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(ext == "OGA") {
@@ -147,7 +147,7 @@ namespace
       }
     }
     else if(ext == "FLAC")
-      file = new FLAC::File(stream, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle);
+      file = new FLAC::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(ext == "MPC")
       file = new MPC::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(ext == "WV")
@@ -201,13 +201,13 @@ namespace
     File *file = nullptr;
 
     if(MPEG::File::isSupported(stream))
-      file = new MPEG::File(stream, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle);
+      file = new MPEG::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(Ogg::Vorbis::File::isSupported(stream))
       file = new Ogg::Vorbis::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(Ogg::FLAC::File::isSupported(stream))
       file = new Ogg::FLAC::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(FLAC::File::isSupported(stream))
-      file = new FLAC::File(stream, ID3v2::FrameFactory::instance(), readAudioProperties, audioPropertiesStyle);
+      file = new FLAC::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(MPC::File::isSupported(stream))
       file = new MPC::File(stream, readAudioProperties, audioPropertiesStyle);
     else if(WavPack::File::isSupported(stream))

taglib/flac/flacfile.cpp

@@ -93,9 +93,12 @@ bool FLAC::File::isSupported(IOStream *stream)
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-FLAC::File::File(FileName file, bool readProperties, Properties::ReadStyle) :
+FLAC::File::File(FileName file, bool readProperties,
+                 Properties::ReadStyle,
+                 ID3v2::FrameFactory *frameFactory) :
   TagLib::File(file),
-  d(std::make_unique<FilePrivate>())
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
 {
   if(isOpen())
     read(readProperties);
@@ -110,6 +113,17 @@ FLAC::File::File(FileName file, ID3v2::FrameFactory *frameFactory,
     read(readProperties);
 }
 
+FLAC::File::File(IOStream *stream, bool readProperties,
+                 Properties::ReadStyle,
+                 ID3v2::FrameFactory *frameFactory) :
+  TagLib::File(stream),
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
+{
+  if(isOpen())
+    read(readProperties);
+}
+
 FLAC::File::File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
                  bool readProperties, Properties::ReadStyle) :
   TagLib::File(stream),

taglib/flac/flacfile.h

@@ -88,11 +88,12 @@ namespace TagLib {
        *
        * \note In the current implementation, \a propertiesStyle is ignored.
        *
-       * \deprecated This constructor will be dropped in favor of the one below
-       * in a future version.
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory (default if null).
        */
       File(FileName file, bool readProperties = true,
-           Properties::ReadStyle propertiesStyle = Properties::Average);
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
 
       /*!
        * Constructs an FLAC file from \a file.  If \a readProperties is true the
@@ -103,11 +104,27 @@ namespace TagLib {
        *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
-      // BIC: merge with the above constructor, kept for source compatibility
+      TAGLIB_DEPRECATED
       File(FileName file, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);
 
+      /*!
+       * Constructs a FLAC file from \a stream.  If \a readProperties is true the
+       * file's audio properties will also be read.
+       *
+       * \note TagLib will *not* take ownership of the stream, the caller is
+       * responsible for deleting it after the File object.
+       *
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory (default if null).
+       *
+       * \note In the current implementation, \a propertiesStyle is ignored.
+       */
+      File(IOStream *stream, bool readProperties = true,
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
+
       /*!
        * Constructs a FLAC file from \a stream.  If \a readProperties is true the
        * file's audio properties will also be read.
@@ -120,6 +137,7 @@ namespace TagLib {
        *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
+      TAGLIB_DEPRECATED
       File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);

taglib/mpeg/mpegfile.cpp

@@ -25,6 +25,7 @@
 
 #include "mpegfile.h"
 
+#include "id3v2framefactory.h"
 #include "tdebug.h"
 #include "tpropertymap.h"
 #include "apefooter.h"
@@ -122,9 +123,12 @@ bool MPEG::File::isSupported(IOStream *stream)
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-MPEG::File::File(FileName file, bool readProperties, Properties::ReadStyle readStyle) :
+MPEG::File::File(FileName file, bool readProperties,
+                 Properties::ReadStyle readStyle,
+                 ID3v2::FrameFactory *frameFactory) :
   TagLib::File(file),
-  d(std::make_unique<FilePrivate>())
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
 {
   if(isOpen())
     read(readProperties, readStyle);
@@ -139,6 +143,17 @@ MPEG::File::File(FileName file, ID3v2::FrameFactory *frameFactory,
     read(readProperties, readStyle);
 }
 
+MPEG::File::File(IOStream *stream, bool readProperties,
+                 Properties::ReadStyle readStyle,
+                 ID3v2::FrameFactory *frameFactory) :
+  TagLib::File(stream),
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
+{
+  if(isOpen())
+    read(readProperties, readStyle);
+}
+
 MPEG::File::File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
                  bool readProperties, Properties::ReadStyle readStyle) :
   TagLib::File(stream),

taglib/mpeg/mpegfile.h

@@ -77,11 +77,12 @@ namespace TagLib {
        * 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.
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory (default if null).
        */
       File(FileName file, bool readProperties = true,
-           Properties::ReadStyle propertiesStyle = Properties::Average);
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
 
       /*!
        * Constructs an MPEG file from \a file.  If \a readProperties is true the
@@ -93,7 +94,7 @@ namespace TagLib {
        * 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
+      TAGLIB_DEPRECATED
       File(FileName file, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);
@@ -110,7 +111,28 @@ namespace TagLib {
        *
        * 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.
+       *
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory (default if null).
        */
+      File(IOStream *stream, bool readProperties = true,
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
+
+      /*!
+       * Constructs an MPEG file from \a stream.  If \a readProperties is true the
+       * file's audio properties will also be read.
+       *
+       * \note TagLib will *not* take ownership of the stream, the caller is
+       * responsible for deleting it after the File object.
+       *
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory.
+       *
+       * 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.
+       */
+      TAGLIB_DEPRECATED
       File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);

taglib/trueaudio/trueaudiofile.cpp

@@ -78,9 +78,12 @@ bool TrueAudio::File::isSupported(IOStream *stream)
 // public members
 ////////////////////////////////////////////////////////////////////////////////
 
-TrueAudio::File::File(FileName file, bool readProperties, Properties::ReadStyle) :
+TrueAudio::File::File(FileName file, bool readProperties,
+                      Properties::ReadStyle,
+                      ID3v2::FrameFactory *frameFactory) :
   TagLib::File(file),
-  d(std::make_unique<FilePrivate>())
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
 {
   if(isOpen())
     read(readProperties);
@@ -95,9 +98,12 @@ TrueAudio::File::File(FileName file, ID3v2::FrameFactory *frameFactory,
     read(readProperties);
 }
 
-TrueAudio::File::File(IOStream *stream, bool readProperties, Properties::ReadStyle) :
+TrueAudio::File::File(IOStream *stream, bool readProperties,
+                      Properties::ReadStyle,
+                      ID3v2::FrameFactory *frameFactory) :
   TagLib::File(stream),
-  d(std::make_unique<FilePrivate>())
+  d(std::make_unique<FilePrivate>(
+    frameFactory ? frameFactory : ID3v2::FrameFactory::instance()))
 {
   if(isOpen())
     read(readProperties);

taglib/trueaudio/trueaudiofile.h

@@ -30,6 +30,7 @@
 #ifndef TAGLIB_TRUEAUDIOFILE_H
 #define TAGLIB_TRUEAUDIOFILE_H
 
+#include "taglib.h"
 #include "tfile.h"
 #include "trueaudioproperties.h"
 
@@ -82,10 +83,14 @@ namespace TagLib {
        * Constructs a TrueAudio file from \a file.  If \a readProperties is true
        * the file's audio properties will also be read.
        *
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory (default if null).
+       *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
       File(FileName file, bool readProperties = true,
-           Properties::ReadStyle propertiesStyle = Properties::Average);
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
 
       /*!
        * Constructs a TrueAudio file from \a file.  If \a readProperties is true
@@ -96,6 +101,7 @@ namespace TagLib {
        *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
+      TAGLIB_DEPRECATED
       File(FileName file, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);
@@ -107,10 +113,14 @@ namespace TagLib {
        * \note TagLib will *not* take ownership of the stream, the caller is
        * responsible for deleting it after the File object.
        *
+       * If this file contains an ID3v2 tag, the frames will be created using
+       * \a frameFactory.
+       *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
       File(IOStream *stream, bool readProperties = true,
-           Properties::ReadStyle propertiesStyle = Properties::Average);
+           Properties::ReadStyle propertiesStyle = Properties::Average,
+           ID3v2::FrameFactory *frameFactory = nullptr);
 
       /*!
        * Constructs a TrueAudio file from \a stream.  If \a readProperties is true
@@ -124,6 +134,7 @@ namespace TagLib {
        *
        * \note In the current implementation, \a propertiesStyle is ignored.
        */
+      TAGLIB_DEPRECATED
       File(IOStream *stream, ID3v2::FrameFactory *frameFactory,
            bool readProperties = true,
            Properties::ReadStyle propertiesStyle = Properties::Average);

tests/test_id3v2framefactory.cpp

@@ -26,11 +26,14 @@
 #include <functional>
 #include <memory>
 
+#include "flacproperties.h"
+#include "mpegproperties.h"
 #include "tbytevector.h"
 #include "tpropertymap.h"
 #include "mpegfile.h"
 #include "flacfile.h"
 #include "trueaudiofile.h"
+#include "trueaudioproperties.h"
 #include "wavfile.h"
 #include "aifffile.h"
 #include "dsffile.h"
@@ -211,7 +214,8 @@ public:
         return new MPEG::File(fileName);
       },
       [](const char *fileName, ID3v2::FrameFactory *factory) {
-        return new MPEG::File(fileName, factory);
+        return new MPEG::File(fileName, true, MPEG::Properties::Average,
+          factory);
       },
       [](const File &f) {
         return static_cast<const MPEG::File &>(f).hasID3v2Tag();
@@ -234,7 +238,8 @@ public:
         return new FLAC::File(fileName);
       },
       [](const char *fileName, ID3v2::FrameFactory *factory) {
-        return new FLAC::File(fileName, factory);
+        return new FLAC::File(fileName, true, FLAC::Properties::Average,
+          factory);
       },
       [](const File &f) {
         return static_cast<const FLAC::File &>(f).hasID3v2Tag();
@@ -258,7 +263,8 @@ public:
         return new TrueAudio::File(fileName);
       },
       [](const char *fileName, ID3v2::FrameFactory *factory) {
-        return new TrueAudio::File(fileName, factory);
+        return new TrueAudio::File(fileName, true,
+          TrueAudio::Properties::Average, factory);
       },
       [](const File &f) {
         return static_cast<const TrueAudio::File &>(f).hasID3v2Tag();