'SoundFileFactory' implementation overhaul

examples/sound/Sound.cpp

@@ -4,7 +4,6 @@
 #include <SFML/Audio.hpp>
 
 #include <iostream>
-#include <string>
 
 
 ////////////////////////////////////////////////////////////
@@ -22,7 +21,7 @@ void playSound()
     std::cout << "killdeer.wav:" << '\n'
               << " " << buffer.getDuration().asSeconds() << " seconds" << '\n'
               << " " << buffer.getSampleRate() << " samples / sec" << '\n'
-              << " " << buffer.getChannelCount() << " channels" << std::endl;
+              << " " << buffer.getChannelCount() << " channels" << '\n';
 
     // Create a sound instance and play it
     sf::Sound sound(buffer);
@@ -35,10 +34,10 @@ void playSound()
         sf::sleep(sf::milliseconds(100));
 
         // Display the playing position
-        std::cout << "\rPlaying... " << sound.getPlayingOffset().asSeconds() << " sec        ";
+        std::cout << "\rPlaying... " << sound.getPlayingOffset().asSeconds() << " sec        " << std::flush;
-        std::cout << std::flush;
     }
-    std::cout << std::endl << std::endl;
+
+    std::cout << '\n' << std::endl;
 }
 
 
@@ -57,7 +56,7 @@ void playMusic(const std::filesystem::path& filename)
     std::cout << filename << ":" << '\n'
               << " " << music.getDuration().asSeconds() << " seconds" << '\n'
               << " " << music.getSampleRate() << " samples / sec" << '\n'
-              << " " << music.getChannelCount() << " channels" << std::endl;
+              << " " << music.getChannelCount() << " channels" << '\n';
 
     // Play it
     music.play();
@@ -69,9 +68,9 @@ void playMusic(const std::filesystem::path& filename)
         sf::sleep(sf::milliseconds(100));
 
         // Display the playing position
-        std::cout << "\rPlaying... " << music.getPlayingOffset().asSeconds() << " sec        ";
+        std::cout << "\rPlaying... " << music.getPlayingOffset().asSeconds() << " sec        " << std::flush;
-        std::cout << std::flush;
     }
+
     std::cout << '\n' << std::endl;
 }
 

include/SFML/Audio/SoundFileFactory.hpp

@@ -31,7 +31,7 @@
 
 #include <filesystem>
 #include <memory>
-#include <vector>
+#include <unordered_map>
 
 #include <cstddef>
 
@@ -67,6 +67,13 @@ public:
     template <typename T>
     static void unregisterReader();
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Check if a reader is registered
+    ///
+    ////////////////////////////////////////////////////////////
+    template <typename T>
+    static bool isReaderRegistered();
+
     ////////////////////////////////////////////////////////////
     /// \brief Register a new writer
     ///
@@ -85,6 +92,13 @@ public:
     template <typename T>
     static void unregisterWriter();
 
+    ////////////////////////////////////////////////////////////
+    /// \brief Check if a writer is registered
+    ///
+    ////////////////////////////////////////////////////////////
+    template <typename T>
+    static bool isWriterRegistered();
+
     ////////////////////////////////////////////////////////////
     /// \brief Instantiate the right reader for the given file on disk
     ///
@@ -136,27 +150,20 @@ private:
     ////////////////////////////////////////////////////////////
     // Types
     ////////////////////////////////////////////////////////////
-    struct ReaderFactory
+    template <typename T>
-    {
+    using CreateFnPtr = std::unique_ptr<T> (*)();
-        bool (*check)(InputStream&);
-        std::unique_ptr<SoundFileReader> (*create)();
-    };
-    using ReaderFactoryArray = std::vector<ReaderFactory>;
 
-    struct WriterFactory
+    using ReaderCheckFnPtr = bool (*)(InputStream&);
-    {
+    using WriterCheckFnPtr = bool (*)(const std::filesystem::path&);
-        bool (*check)(const std::filesystem::path&);
+
-        std::unique_ptr<SoundFileWriter> (*create)();
+    using ReaderFactoryMap = std::unordered_map<CreateFnPtr<SoundFileReader>, ReaderCheckFnPtr>;
-    };
+    using WriterFactoryMap = std::unordered_map<CreateFnPtr<SoundFileWriter>, WriterCheckFnPtr>;
-    using WriterFactoryArray = std::vector<WriterFactory>;
 
     ////////////////////////////////////////////////////////////
-    // Static member data
+    // Static member functions
     ////////////////////////////////////////////////////////////
-    // NOLINTBEGIN(readability-identifier-naming)
+    static ReaderFactoryMap& getReaderFactoryMap();
-    static inline ReaderFactoryArray s_readers; //!< List of all registered readers
+    static WriterFactoryMap& getWriterFactoryMap();
-    static inline WriterFactoryArray s_writers; //!< List of all registered writers
-    // NOLINTEND(readability-identifier-naming)
 };
 
 } // namespace sf
@@ -182,7 +189,10 @@ private:
 /// Usage example:
 /// \code
 /// sf::SoundFileFactory::registerReader<MySoundFileReader>();
+/// assert(sf::SoundFileFactory::isReaderRegistered<MySoundFileReader>());
+///
 /// sf::SoundFileFactory::registerWriter<MySoundFileWriter>();
+/// assert(sf::SoundFileFactory::isWriterRegistered<MySoundFileWriter>());
 /// \endcode
 ///
 /// \see sf::InputSoundFile, sf::OutputSoundFile, sf::SoundFileReader, sf::SoundFileWriter

include/SFML/Audio/SoundFileFactory.inl

@@ -46,18 +46,12 @@ std::unique_ptr<SoundFileWriter> createWriter()
 }
 } // namespace priv
 
+
 ////////////////////////////////////////////////////////////
 template <typename T>
 void SoundFileFactory::registerReader()
 {
-    // Make sure the same class won't be registered twice
+    getReaderFactoryMap()[&priv::createReader<T>] = &T::check;
-    unregisterReader<T>();
-
-    // Create a new factory with the functions provided by the class
-    const ReaderFactory factory{&T::check, &priv::createReader<T>};
-
-    // Add it
-    s_readers.push_back(factory);
 }
 
 
@@ -65,26 +59,23 @@ void SoundFileFactory::registerReader()
 template <typename T>
 void SoundFileFactory::unregisterReader()
 {
-    // Remove the instance(s) of the reader from the array of factories
+    getReaderFactoryMap().erase(&priv::createReader<T>);
-    s_readers.erase(std::remove_if(s_readers.begin(),
-                                   s_readers.end(),
-                                   [](const ReaderFactory& readerFactory)
-                                   { return readerFactory.create == &priv::createReader<T>; }),
-                    s_readers.end());
 }
 
+
+////////////////////////////////////////////////////////////
+template <typename T>
+bool SoundFileFactory::isReaderRegistered()
+{
+    return getReaderFactoryMap().count(&priv::createReader<T>) == 1;
+}
+
+
 ////////////////////////////////////////////////////////////
 template <typename T>
 void SoundFileFactory::registerWriter()
 {
-    // Make sure the same class won't be registered twice
+    getWriterFactoryMap()[&priv::createWriter<T>] = &T::check;
-    unregisterWriter<T>();
-
-    // Create a new factory with the functions provided by the class
-    const WriterFactory factory{&T::check, &priv::createWriter<T>};
-
-    // Add it
-    s_writers.push_back(factory);
 }
 
 
@@ -92,12 +83,15 @@ void SoundFileFactory::registerWriter()
 template <typename T>
 void SoundFileFactory::unregisterWriter()
 {
-    // Remove the instance(s) of the writer from the array of factories
+    getWriterFactoryMap().erase(&priv::createWriter<T>);
-    s_writers.erase(std::remove_if(s_writers.begin(),
+}
-                                   s_writers.end(),
+
-                                   [](const WriterFactory& writerFactory)
+
-                                   { return writerFactory.create == &priv::createWriter<T>; }),
+////////////////////////////////////////////////////////////
-                    s_writers.end());
+template <typename T>
+bool SoundFileFactory::isWriterRegistered()
+{
+    return getWriterFactoryMap().count(&priv::createWriter<T>) == 1;
 }
 
 } // namespace sf

src/SFML/Audio/SoundFileFactory.cpp

@@ -42,34 +42,11 @@
 #include <ostream>
 
 
-namespace
-{
-// Register all the built-in readers and writers if not already done
-void ensureDefaultReadersWritersRegistered()
-{
-    static bool registered = false;
-    if (!registered)
-    {
-        sf::SoundFileFactory::registerReader<sf::priv::SoundFileReaderFlac>();
-        sf::SoundFileFactory::registerWriter<sf::priv::SoundFileWriterFlac>();
-        sf::SoundFileFactory::registerReader<sf::priv::SoundFileReaderMp3>();
-        sf::SoundFileFactory::registerReader<sf::priv::SoundFileReaderOgg>();
-        sf::SoundFileFactory::registerWriter<sf::priv::SoundFileWriterOgg>();
-        sf::SoundFileFactory::registerReader<sf::priv::SoundFileReaderWav>();
-        sf::SoundFileFactory::registerWriter<sf::priv::SoundFileWriterWav>();
-        registered = true;
-    }
-}
-} // namespace
-
 namespace sf
 {
 ////////////////////////////////////////////////////////////
 std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromFilename(const std::filesystem::path& filename)
 {
-    // Register the built-in readers/writers on first call
-    ensureDefaultReadersWritersRegistered();
-
     // Wrap the input file into a file stream
     FileInputStream stream;
     if (!stream.open(filename))
@@ -79,7 +56,7 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromFilename(cons
     }
 
     // Test the filename in all the registered factories
-    for (const ReaderFactory& readerFactory : s_readers)
+    for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap())
     {
         if (stream.seek(0) == -1)
         {
@@ -87,8 +64,8 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromFilename(cons
             return nullptr;
         }
 
-        if (readerFactory.check(stream))
+        if (fpCheck(stream))
-            return readerFactory.create();
+            return fpCreate();
     }
 
     // No suitable reader found
@@ -100,15 +77,12 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromFilename(cons
 ////////////////////////////////////////////////////////////
 std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromMemory(const void* data, std::size_t sizeInBytes)
 {
-    // Register the built-in readers/writers on first call
-    ensureDefaultReadersWritersRegistered();
-
     // Wrap the memory file into a file stream
     MemoryInputStream stream;
     stream.open(data, sizeInBytes);
 
     // Test the stream for all the registered factories
-    for (const ReaderFactory& readerFactory : s_readers)
+    for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap())
     {
         if (stream.seek(0) == -1)
         {
@@ -116,8 +90,8 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromMemory(const
             return nullptr;
         }
 
-        if (readerFactory.check(stream))
+        if (fpCheck(stream))
-            return readerFactory.create();
+            return fpCreate();
     }
 
     // No suitable reader found
@@ -129,11 +103,8 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromMemory(const
 ////////////////////////////////////////////////////////////
 std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromStream(InputStream& stream)
 {
-    // Register the built-in readers/writers on first call
-    ensureDefaultReadersWritersRegistered();
-
     // Test the stream for all the registered factories
-    for (const ReaderFactory& readerFactory : s_readers)
+    for (const auto& [fpCreate, fpCheck] : getReaderFactoryMap())
     {
         if (stream.seek(0) == -1)
         {
@@ -141,8 +112,8 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromStream(InputS
             return nullptr;
         }
 
-        if (readerFactory.check(stream))
+        if (fpCheck(stream))
-            return readerFactory.create();
+            return fpCreate();
     }
 
     // No suitable reader found
@@ -154,14 +125,11 @@ std::unique_ptr<SoundFileReader> SoundFileFactory::createReaderFromStream(InputS
 ////////////////////////////////////////////////////////////
 std::unique_ptr<SoundFileWriter> SoundFileFactory::createWriterFromFilename(const std::filesystem::path& filename)
 {
-    // Register the built-in readers/writers on first call
-    ensureDefaultReadersWritersRegistered();
-
     // Test the filename in all the registered factories
-    for (const WriterFactory& writerFactory : s_writers)
+    for (const auto& [fpCreate, fpCheck] : getWriterFactoryMap())
     {
-        if (writerFactory.check(filename))
+        if (fpCheck(filename))
-            return writerFactory.create();
+            return fpCreate();
     }
 
     // No suitable writer found
@@ -169,4 +137,29 @@ std::unique_ptr<SoundFileWriter> SoundFileFactory::createWriterFromFilename(cons
     return nullptr;
 }
 
+
+////////////////////////////////////////////////////////////
+SoundFileFactory::ReaderFactoryMap& SoundFileFactory::getReaderFactoryMap()
+{
+    // The map is pre-populated with default readers on construction
+    static ReaderFactoryMap result{{&priv::createReader<priv::SoundFileReaderFlac>, &priv::SoundFileReaderFlac::check},
+                                   {&priv::createReader<priv::SoundFileReaderMp3>, &priv::SoundFileReaderMp3::check},
+                                   {&priv::createReader<priv::SoundFileReaderOgg>, &priv::SoundFileReaderOgg::check},
+                                   {&priv::createReader<priv::SoundFileReaderWav>, &priv::SoundFileReaderWav::check}};
+
+    return result;
+}
+
+
+////////////////////////////////////////////////////////////
+SoundFileFactory::WriterFactoryMap& SoundFileFactory::getWriterFactoryMap()
+{
+    // The map is pre-populated with default writers on construction
+    static WriterFactoryMap result{{&priv::createWriter<priv::SoundFileWriterFlac>, &priv::SoundFileWriterFlac::check},
+                                   {&priv::createWriter<priv::SoundFileWriterOgg>, &priv::SoundFileWriterOgg::check},
+                                   {&priv::createWriter<priv::SoundFileWriterWav>, &priv::SoundFileWriterWav::check}};
+
+    return result;
+}
+
 } // namespace sf

test/Audio/SoundFileFactory.test.cpp

@@ -5,11 +5,60 @@
 #include <SFML/Audio/SoundFileWriter.hpp>
 
 #include <SFML/System/FileInputStream.hpp>
+#include <SFML/System/InputStream.hpp>
 
 #include <catch2/catch_test_macros.hpp>
 
+#include <filesystem>
+#include <optional>
 #include <type_traits>
 
+#include <cstdint>
+
+namespace
+{
+
+struct NoopSoundFileReader : sf::SoundFileReader
+{
+    static bool check(sf::InputStream&)
+    {
+        return false;
+    }
+
+    std::optional<Info> open(sf::InputStream&) override
+    {
+        return {};
+    }
+
+    void seek(std::uint64_t) override
+    {
+    }
+
+    std::uint64_t read(std::int16_t*, std::uint64_t) override
+    {
+        return 0;
+    }
+};
+
+struct NoopSoundFileWriter : sf::SoundFileWriter
+{
+    static bool check(const std::filesystem::path&)
+    {
+        return false;
+    }
+
+    bool open(const std::filesystem::path&, unsigned int, unsigned int) override
+    {
+        return false;
+    }
+
+    void write(const std::int16_t*, std::uint64_t) override
+    {
+    }
+};
+
+} // namespace
+
 TEST_CASE("[Audio] sf::SoundFileFactory")
 {
     SECTION("Type traits")
@@ -20,6 +69,28 @@ TEST_CASE("[Audio] sf::SoundFileFactory")
         STATIC_CHECK(std::is_nothrow_move_assignable_v<sf::SoundFileFactory>);
     }
 
+    SECTION("isReaderRegistered()")
+    {
+        CHECK(!sf::SoundFileFactory::isReaderRegistered<NoopSoundFileReader>());
+
+        sf::SoundFileFactory::registerReader<NoopSoundFileReader>();
+        CHECK(sf::SoundFileFactory::isReaderRegistered<NoopSoundFileReader>());
+
+        sf::SoundFileFactory::unregisterReader<NoopSoundFileReader>();
+        CHECK(!sf::SoundFileFactory::isReaderRegistered<NoopSoundFileReader>());
+    }
+
+    SECTION("isWriterRegistered()")
+    {
+        CHECK(!sf::SoundFileFactory::isWriterRegistered<NoopSoundFileWriter>());
+
+        sf::SoundFileFactory::registerWriter<NoopSoundFileWriter>();
+        CHECK(sf::SoundFileFactory::isWriterRegistered<NoopSoundFileWriter>());
+
+        sf::SoundFileFactory::unregisterWriter<NoopSoundFileWriter>();
+        CHECK(!sf::SoundFileFactory::isWriterRegistered<NoopSoundFileWriter>());
+    }
+
     SECTION("createReaderFromFilename()")
     {
         SECTION("Missing file")