pcmDriver stop methods (#270)

* pcmDriver stop methods * code rev * update version * sdl2 mixer callback, small improvement * code rev * sonarcloud code rev * code rev * PCMDriverTest init | TestIMusicDriver -> TestIAudioDriver * PCMDriverTest play/stop basic scenarios * code rev * sonarcloud code rev
Raffaello · Nov 5, 2023 · fb60bf8 · fb60bf8
1 parent c6db69d
commit fb60bf8
Show file tree

Hide file tree

Showing 10 changed files with 208 additions and 40 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -5,7 +5,7 @@ if(DEFINED ENV{VCPKG_ROOT} AND NOT DEFINED CMAKE_TOOLCHAIN_FILE)
 endif()
 
 
-project ("sdl2-hyper-sonic-drivers" VERSION 0.13.2 DESCRIPTION "SDL2 based Hyper-Sonic Drivers for emulating old soundcards")
+project ("sdl2-hyper-sonic-drivers" VERSION 0.13.3 DESCRIPTION "SDL2 based Hyper-Sonic Drivers for emulating old soundcards")
 include (TestBigEndian)
 TEST_BIG_ENDIAN(IS_BIG_ENDIAN)
 if(IS_BIG_ENDIAN)

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/sdl2/Mixer.cpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/sdl2/Mixer.cpp
@@ -257,11 +257,9 @@ namespace HyperSonicDrivers::audio::sdl2
         int16_t* buf = std::bit_cast<int16_t*>(samples);
         // we store stereo, 16-bit samples (div 2 for stereo and 2 from 8 to 16 bits)
         assert(len % 4 == 0);
-        len >>= 1;
         //  zero the buf (size of 2ch stereo: len*2 of 16 bits)
-        memset(buf, 0, len * sizeof(int16_t));
-        len >>= 1; // size of the stereo 16 bits buffer.
-
+        memset(buf, 0, len);
+        len >>= 2;
         // mix all channels
         size_t res = 0;
         for (const auto& ch : m_channels)

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp
@@ -38,7 +38,7 @@ namespace HyperSonicDrivers::audio::streams
         return m_curPos == m_sound->dataSize;
     }
 
-    std::weak_ptr<PCMSound> PCMStream::getSound() const noexcept
+    std::shared_ptr<PCMSound> PCMStream::getSound() const noexcept
     {
         return m_sound;
     }

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.hpp
@@ -18,7 +18,7 @@ namespace HyperSonicDrivers::audio::streams
         uint32_t getRate() const override;
         bool endOfData() const override;
 
-        std::weak_ptr<PCMSound>  getSound() const noexcept;
+        std::shared_ptr<PCMSound>  getSound() const noexcept;
     private:
         std::shared_ptr<PCMSound> m_sound;
         uint32_t m_curPos = 0;

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.cpp
@@ -8,14 +8,13 @@ namespace HyperSonicDrivers::drivers
     PCMDriver::PCMDriver(const std::shared_ptr<audio::IMixer>& mixer, const uint8_t max_channels) :
         max_streams(std::min(mixer->max_channels, max_channels)), m_mixer(mixer)
     {
-        m_PCMStreams.resize(max_streams);
     }
 
     bool PCMDriver::isPlaying() const noexcept
     {
-        for(const auto& ss: m_PCMStreams)
+        for (const auto& [stream, _] : m_PCMStreams_channels)
         {
-            if (isPCMStreamPlaying_(ss))
+            if (isPCMStreamPlaying_(stream))
                 return true;
         }
 
@@ -24,48 +23,97 @@ namespace HyperSonicDrivers::drivers
 
     bool PCMDriver::isPlaying(const std::shared_ptr<audio::PCMSound>& sound) const noexcept
     {
-        // TODO:
-        // should map channelId to check directly in the mixer?
-        // how to find a free slot then? 
-        // does we need to really track it?
-        // probably using a map instead of a vector is ok,
-        // no need to define nether max-channels.
-        // but that is because if wanting to reserve some channels for something
-        // else that is not PCM related...
-        // anyway... it could be achieved having the mixer a "lock or reserved channel"
-        // feature or something that that one won't be used unless
-        // it is for the resources that has been reserved for.....
-        for(const auto& ss : m_PCMStreams)
+        for (const auto& [stream, _] : m_PCMStreams_channels)
         {
-            if (ss->getSound().lock() == sound)
-                return isPCMStreamPlaying_(ss);
+            if (stream->getSound() == sound)
+                return isPCMStreamPlaying_(stream);
         }
 
         return false;
     }
 
     std::optional<uint8_t> PCMDriver::play(const std::shared_ptr<audio::PCMSound>& sound, const uint8_t volume, const int8_t pan)
     {
-        // find first free slot
-        auto it = std::ranges::find_if_not(m_PCMStreams, isPCMStreamPlaying_);
-        if (it == m_PCMStreams.end())
+        releaseEndedStreams_();
+        if (m_PCMStreams_channels.size() == max_streams)
             return std::nullopt;
 
-        *it = std::make_shared<PCMStream>(sound);
+        auto s = std::make_shared<PCMStream>(sound);
 
         auto channelId =  m_mixer->play(
             sound->group,
-            *it,
+            s,
             volume,
             pan
         );
 
-        if (!channelId.has_value())
-            *it = nullptr;
+        if (channelId.has_value())
+            m_PCMStreams_channels[s] = channelId.value();
 
         return channelId;
     }
 
+    void PCMDriver::stop(const uint8_t channel_id, const bool releaseEndedStreams) noexcept
+    {
+        auto it = std::ranges::find_if(
+            m_PCMStreams_channels,
+            [channel_id](std::pair<const std::shared_ptr<audio::streams::PCMStream>&, const int> p) {
+                return channel_id == p.second;
+            }
+        );
+
+        if (it == m_PCMStreams_channels.end())
+            return;
+
+        if (!(it->first)->isEnded())
+            m_mixer->reset(channel_id);
+
+        if (releaseEndedStreams)
+        {
+            m_PCMStreams_channels.erase(it);
+            releaseEndedStreams_();
+        }
+    }
+
+    void PCMDriver::stop(const std::shared_ptr<audio::PCMSound>& sound, const bool releaseEndedStreams)
+    {
+        auto it = std::ranges::find_if(
+            m_PCMStreams_channels,
+            [&sound](std::pair<const std::shared_ptr<audio::streams::PCMStream>&, const int> p) {
+                return p.first != nullptr && sound == p.first->getSound();
+            }
+        );
+
+        if (it == m_PCMStreams_channels.end())
+            return;
+
+        stop(it->second, releaseEndedStreams);
+    }
+
+    void PCMDriver::stop() noexcept
+    {
+        for (const auto& [_, ch_id] : m_PCMStreams_channels)
+            stop(ch_id, false);
+
+        releaseStreams_();
+    }
+
+    void PCMDriver::releaseEndedStreams_() noexcept
+    {
+        for (auto it = m_PCMStreams_channels.begin(); it != m_PCMStreams_channels.end();)
+        {
+            if (!isPCMStreamPlaying_(it->first))
+                it = m_PCMStreams_channels.erase(it);
+            else
+                ++it;
+        }
+    }
+
+    void PCMDriver::releaseStreams_() noexcept
+    {
+        m_PCMStreams_channels.clear();
+    }
+
     inline bool PCMDriver::isPCMStreamPlaying_(const std::shared_ptr<audio::streams::PCMStream>& stream) noexcept
     {
         return stream != nullptr && !stream->isEnded();

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/PCMDriver.hpp
@@ -1,8 +1,8 @@
 #pragma once
 
-#include <memory>
 #include <cstdint>
-#include <vector>
+#include <memory>
+#include <map>
 #include <optional>
 #include <HyperSonicDrivers/audio/IMixer.hpp>
 #include <HyperSonicDrivers/audio/streams/PCMStream.hpp>
@@ -30,11 +30,18 @@ namespace HyperSonicDrivers::drivers
             const uint8_t volume = audio::mixer::Channel_max_volume,
             const int8_t pan = 0
         );
+        void stop(const uint8_t channel_id, const bool releaseEndedStreams = true) noexcept;
+        void stop(const std::shared_ptr<audio::PCMSound>& sound, const bool releaseEndedStreams = true);
+        void stop() noexcept;
+
 
         const uint8_t max_streams;
     private:
         std::shared_ptr<audio::IMixer> m_mixer;
-        std::vector<std::shared_ptr<audio::streams::PCMStream>> m_PCMStreams;
+        std::map<std::shared_ptr<audio::streams::PCMStream>, uint8_t> m_PCMStreams_channels;
+
+        void releaseEndedStreams_() noexcept;
+        void releaseStreams_() noexcept;
 
         static bool isPCMStreamPlaying_(const std::shared_ptr<audio::streams::PCMStream>& stream) noexcept;
     };

diff --git a/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/CMakeLists.txt b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/CMakeLists.txt
@@ -171,8 +171,15 @@ macro_test(
 )
 
 macro_test(
-    EXE TestIMusicDriver
-    FILES "drivers/TestIMusicDriver.cpp"
+    EXE TestIAudioDriver
+    FILES "drivers/TestIAudioDriver.cpp"
+    LINKS_PRIVATE hyper-sonic-drivers-static
+    FIXTURES 
+)
+
+macro_test(
+    EXE TestPCMDriver
+    FILES "drivers/TestPCMDriver.cpp"
     LINKS_PRIVATE hyper-sonic-drivers-static
     FIXTURES 
 )

diff --git a/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/audio/IMixerMock.hpp b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/audio/IMixerMock.hpp
@@ -12,6 +12,7 @@ namespace HyperSonicDrivers::audio
     {
     public:
         int rate = 44100;
+        uint8_t cur_ch = 255;
 
         IMixerMock() : IMixer(32, 44100, 1024) {};
         explicit IMixerMock(const int freq) : IMixer(32, freq, 1024) {};
@@ -24,7 +25,7 @@ namespace HyperSonicDrivers::audio
             const uint8_t vol,
             const int8_t pan
         ) override {
-            return std::make_optional(0);
+            return std::make_optional((++cur_ch) % max_channels);
         };
 
         void suspend() noexcept override {};

diff --git a/...SonicDrivers/drivers/TestIMusicDriver.cpp → ...SonicDrivers/drivers/TestIAudioDriver.cpp b/...SonicDrivers/drivers/TestIMusicDriver.cpp → ...SonicDrivers/drivers/TestIAudioDriver.cpp
@@ -4,18 +4,18 @@
 
 namespace HyperSonicDrivers::drivers
 {
-    class IMusicDriverMock : public IAudioDriver
+    class IAudioDriverMock : public IAudioDriver
     {
     public:
-        IMusicDriverMock(const std::shared_ptr<devices::IDevice>& device) : IAudioDriver(device) {}
+        IAudioDriverMock(const std::shared_ptr<devices::IDevice>& device) : IAudioDriver(device) {}
         void play(const uint8_t track) noexcept override {};
         void stop() noexcept override {};
         bool isPlaying() const noexcept override { return false; };
     };
 
     TEST(IAudioDriver, cstor_nullptr)
     {
-        EXPECT_THROW(IMusicDriverMock md(nullptr), std::runtime_error);
+        EXPECT_THROW(IAudioDriverMock md(nullptr), std::runtime_error);
     }
 }
 

diff --git a/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestPCMDriver.cpp b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestPCMDriver.cpp
@@ -0,0 +1,107 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#include <HyperSonicDrivers/drivers/PCMDriver.hpp>
+#include <HyperSonicDrivers/audio/IMixerMock.hpp>
+#include <HyperSonicDrivers/audio/PCMSound.hpp>
+#include <HyperSonicDrivers/audio/mixer/ChannelGroup.hpp>
+#include <memory>
+#include <cstdint>
+
+namespace HyperSonicDrivers::drivers
+{
+    TEST(PCMDriver, play_stop0)
+    {
+        auto mixer = audio::make_mixer<audio::IMixerMock>();
+        auto drv = PCMDriver(mixer);
+
+        EXPECT_EQ(drv.max_streams, mixer->max_channels);
+
+        auto sound_data = std::make_shared<int16_t[]>(1);
+        auto sound = std::make_shared<audio::PCMSound>(audio::mixer::eChannelGroup::Plain, true, 44100, 1, sound_data);
+        auto ch_id = drv.play(sound);
+
+        ASSERT_TRUE(drv.isPlaying(sound));
+        ASSERT_TRUE(drv.isPlaying());
+        ASSERT_TRUE(ch_id.has_value());
+        EXPECT_EQ(ch_id.value(), 0);
+
+        drv.stop(ch_id.value());
+        EXPECT_FALSE(drv.isPlaying(sound));
+        EXPECT_FALSE(drv.isPlaying());
+    }
+
+    TEST(PCMDriver, play_stop1)
+    {
+        auto mixer = audio::make_mixer<audio::IMixerMock>();
+        auto drv = PCMDriver(mixer);
+
+        EXPECT_EQ(drv.max_streams, mixer->max_channels);
+
+        auto sound_data = std::make_shared<int16_t[]>(1);
+        auto sound = std::make_shared<audio::PCMSound>(audio::mixer::eChannelGroup::Plain, true, 44100, 1, sound_data);
+        auto ch_id = drv.play(sound);
+
+        ASSERT_TRUE(drv.isPlaying(sound));
+        ASSERT_TRUE(drv.isPlaying());
+        ASSERT_TRUE(ch_id.has_value());
+        EXPECT_EQ(ch_id.value(), 0);
+
+        drv.stop(sound);
+        EXPECT_FALSE(drv.isPlaying(sound));
+        EXPECT_FALSE(drv.isPlaying());
+    }
+
+    TEST(PCMDriver, play_stop2)
+    {
+        auto mixer = audio::make_mixer<audio::IMixerMock>();
+        auto drv = PCMDriver(mixer);
+
+        EXPECT_EQ(drv.max_streams, mixer->max_channels);
+
+        auto sound_data = std::make_shared<int16_t[]>(1);
+        auto sound = std::make_shared<audio::PCMSound>(audio::mixer::eChannelGroup::Plain, true, 44100, 1, sound_data);
+        auto ch_id = drv.play(sound);
+
+        ASSERT_TRUE(drv.isPlaying(sound));
+        ASSERT_TRUE(drv.isPlaying());
+        ASSERT_TRUE(ch_id.has_value());
+        EXPECT_EQ(ch_id.value(), 0);
+
+        drv.stop();
+        EXPECT_FALSE(drv.isPlaying(sound));
+        EXPECT_FALSE(drv.isPlaying());
+    }
+
+    TEST(PCMDriver, play_stop_complex)
+    {
+        auto mixer = audio::make_mixer<audio::IMixerMock>();
+        auto drv = PCMDriver(mixer);
+
+        EXPECT_EQ(drv.max_streams, mixer->max_channels);
+
+        auto sound_data = std::make_shared<int16_t[]>(1);
+        auto sound = std::make_shared<audio::PCMSound>(audio::mixer::eChannelGroup::Plain, true, 44100, 1, sound_data);
+        auto sound_data2 = std::make_shared<int16_t[]>(1);
+        auto sound2 = std::make_shared<audio::PCMSound>(audio::mixer::eChannelGroup::Unknown, true, 22050, 1, sound_data2);
+
+        auto ch_id = drv.play(sound);
+        EXPECT_TRUE(drv.play(sound2).has_value());
+        EXPECT_TRUE(drv.play(sound2).has_value());
+        EXPECT_TRUE(drv.play(sound2).has_value());
+
+        ASSERT_TRUE(drv.isPlaying(sound));
+        ASSERT_TRUE(drv.isPlaying());
+        ASSERT_TRUE(ch_id.has_value());
+        EXPECT_EQ(ch_id.value(), 0);
+
+        drv.stop(ch_id.value());
+        EXPECT_FALSE(drv.isPlaying(sound));
+        EXPECT_TRUE(drv.isPlaying());
+    }
+}
+
+int main(int argc, char** argv)
+{
+    ::testing::InitGoogleTest(&argc, argv);
+    return RUN_ALL_TESTS();
+}