From fb60bf8a1cc89e3ac1817ec7ab7e38fb8c970afc Mon Sep 17 00:00:00 2001 From: Raffaello Bertini Date: Sun, 5 Nov 2023 14:43:43 +0000 Subject: [PATCH] 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 --- CMakeLists.txt | 2 +- .../HyperSonicDrivers/audio/sdl2/Mixer.cpp | 6 +- .../audio/streams/PCMStream.cpp | 2 +- .../audio/streams/PCMStream.hpp | 2 +- .../HyperSonicDrivers/drivers/PCMDriver.cpp | 96 ++++++++++++---- .../HyperSonicDrivers/drivers/PCMDriver.hpp | 13 ++- .../test/HyperSonicDrivers/CMakeLists.txt | 11 +- .../HyperSonicDrivers/audio/IMixerMock.hpp | 3 +- ...tIMusicDriver.cpp => TestIAudioDriver.cpp} | 6 +- .../drivers/TestPCMDriver.cpp | 107 ++++++++++++++++++ 10 files changed, 208 insertions(+), 40 deletions(-) rename sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/{TestIMusicDriver.cpp => TestIAudioDriver.cpp} (75%) create mode 100644 sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestPCMDriver.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 42a64f22..5cdf3ff5 100644 --- 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 index 6b730ed1..c7c1515f 100644 --- 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(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 index 5d8b0ca2..343b59c0 100644 --- 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 PCMStream::getSound() const noexcept + std::shared_ptr 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 index 09b7d6c8..9c8901f4 100644 --- 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 getSound() const noexcept; + std::shared_ptr getSound() const noexcept; private: std::shared_ptr 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 index 13e261a7..37110ddc 100644 --- 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& 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,21 +23,10 @@ namespace HyperSonicDrivers::drivers bool PCMDriver::isPlaying(const std::shared_ptr& 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; @@ -46,26 +34,86 @@ namespace HyperSonicDrivers::drivers std::optional PCMDriver::play(const std::shared_ptr& 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(sound); + auto s = std::make_shared(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 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& sound, const bool releaseEndedStreams) + { + auto it = std::ranges::find_if( + m_PCMStreams_channels, + [&sound](std::pair&, 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& 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 index fb415e11..a3f27f01 100644 --- 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 #include -#include +#include +#include #include #include #include @@ -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& sound, const bool releaseEndedStreams = true); + void stop() noexcept; + const uint8_t max_streams; private: std::shared_ptr m_mixer; - std::vector> m_PCMStreams; + std::map, uint8_t> m_PCMStreams_channels; + + void releaseEndedStreams_() noexcept; + void releaseStreams_() noexcept; static bool isPCMStreamPlaying_(const std::shared_ptr& stream) noexcept; }; diff --git a/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/CMakeLists.txt b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/CMakeLists.txt index 5873f81a..24362b88 100644 --- 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 index 3e8f1f62..d115b853 100644 --- 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/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIMusicDriver.cpp b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIAudioDriver.cpp similarity index 75% rename from sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIMusicDriver.cpp rename to sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIAudioDriver.cpp index af6a7f9b..0248f435 100644 --- a/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIMusicDriver.cpp +++ b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestIAudioDriver.cpp @@ -4,10 +4,10 @@ namespace HyperSonicDrivers::drivers { - class IMusicDriverMock : public IAudioDriver + class IAudioDriverMock : public IAudioDriver { public: - IMusicDriverMock(const std::shared_ptr& device) : IAudioDriver(device) {} + IAudioDriverMock(const std::shared_ptr& device) : IAudioDriver(device) {} void play(const uint8_t track) noexcept override {}; void stop() noexcept override {}; bool isPlaying() const noexcept override { return false; }; @@ -15,7 +15,7 @@ namespace HyperSonicDrivers::drivers 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 new file mode 100644 index 00000000..6b714c19 --- /dev/null +++ b/sdl2-hyper-sonic-drivers/test/HyperSonicDrivers/drivers/TestPCMDriver.cpp @@ -0,0 +1,107 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +namespace HyperSonicDrivers::drivers +{ + TEST(PCMDriver, play_stop0) + { + auto mixer = audio::make_mixer(); + auto drv = PCMDriver(mixer); + + EXPECT_EQ(drv.max_streams, mixer->max_channels); + + auto sound_data = std::make_shared(1); + auto sound = std::make_shared(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(); + auto drv = PCMDriver(mixer); + + EXPECT_EQ(drv.max_streams, mixer->max_channels); + + auto sound_data = std::make_shared(1); + auto sound = std::make_shared(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(); + auto drv = PCMDriver(mixer); + + EXPECT_EQ(drv.max_streams, mixer->max_channels); + + auto sound_data = std::make_shared(1); + auto sound = std::make_shared(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(); + auto drv = PCMDriver(mixer); + + EXPECT_EQ(drv.max_streams, mixer->max_channels); + + auto sound_data = std::make_shared(1); + auto sound = std::make_shared(audio::mixer::eChannelGroup::Plain, true, 44100, 1, sound_data); + auto sound_data2 = std::make_shared(1); + auto sound2 = std::make_shared(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(); +}