improvements (#255)

* code rev * code rev * code rev * code rev * code rev * onCallback interface definition * OplDriver constructor passing a device instead of hardware * MidiDriver_ADLIB construct with device instead of hardware arg * Sound -> PCMSound * code rev * renderer test * code rev
Raffaello · Sep 28, 2023 · 9a17c14 · 9a17c14
1 parent ca147cb
commit 9a17c14
Show file tree

Hide file tree

Showing 36 changed files with 269 additions and 190 deletions.
diff --git a/sdl2-hyper-sonic-drivers/CMakeLists.txt b/sdl2-hyper-sonic-drivers/CMakeLists.txt
@@ -81,7 +81,7 @@ target_sources(${LIB_NAME} PRIVATE
     # --- #
     ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/MIDI.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/midi/MIDITrack.cpp
-    ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/Sound.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/PCMSound.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/streams/PCMStream.cpp
     ${CMAKE_CURRENT_SOURCE_DIR}/src/HyperSonicDrivers/audio/streams/EmulatedStream.cpp
     # --- #

diff --git a/sdl2-hyper-sonic-drivers/examples/pcm-example.cpp b/sdl2-hyper-sonic-drivers/examples/pcm-example.cpp
@@ -3,7 +3,7 @@
 #include <HyperSonicDrivers/drivers/PCMDriver.hpp>
 #include <HyperSonicDrivers/files/WAVFile.hpp>
 #include <HyperSonicDrivers/files/VOCFile.hpp>
-#include <HyperSonicDrivers/audio/Sound.hpp>
+#include <HyperSonicDrivers/audio/PCMSound.hpp>
 #include <HyperSonicDrivers/utils/algorithms.hpp>
 #include <HyperSonicDrivers/utils/ILogger.hpp>
 #include <HyperSonicDrivers/files/loaders.hpp>

diff --git a/sdl2-hyper-sonic-drivers/sdl2-hyper-sonic-drivers.cpp b/sdl2-hyper-sonic-drivers/sdl2-hyper-sonic-drivers.cpp
@@ -322,57 +322,6 @@ int song()
 //    return 0;
 //}
 
-
-
-
-
-
-
-
-
-
-
-
-
-void rendererADL()
-{
-    // TODO: need to be re-tested
-    using hardware::opl::OplEmulator;
-    using hardware::opl::OplType;
-    using audio::mixer::eChannelGroup;
-    using utils::ILogger;
-    using hardware::opl::OPL;
-
-    audio::sdl2::Renderer r(44100, 1024);
-
-    r.setOutputFile("renderer.wav");
-
-    auto mixer = r.getMixer();
-
-    auto adlib = devices::make_device<devices::Adlib, devices::Opl>(mixer, OplEmulator::AUTO);
-    auto drv1 = drivers::westwood::ADLDriver(adlib, eChannelGroup::Music);
-    auto af = std::make_shared<files::westwood::ADLFile>("test/fixtures/DUNE0.ADL");
-    drv1.setADLFile(af);
-
-    auto eo = adlib->getOpl();
-    drv1.play(4);
-    while(drv1.isPlaying())
-        r.renderBuffer(eo);
-
-    r.releaseOutputFile();
-
-    files::WAVFile w("renderer.wav");
-    auto sound = w.getSound();
-
-    //r.setOutputFile("render2.dat");
-    //drv1.play(2);
-    //while(drv1.isPlaying())
-    //    r.renderBuffer(eo);
-    //while (drv1.isPlaying())
-    //    utils::delayMillis(1);
-    //utils::delayMillis(1000);
-}
-
 void rendererMIDI()
 {
     // TODO: need to review the MIDDrv as it is time dependant
@@ -439,10 +388,9 @@ int main(int argc, char* argv[])
     //newMixerTest();
     //testMultiOpl();
     //testMOplMultiDrv();
-    //rendererADL();
     //rendererMIDI();
     //midi_adlib();
-    testMT32();
+    //testMT32();
     return 0;
     //sdlMixer();
     //SDL_Delay(100);

diff --git a/...ers/src/HyperSonicDrivers/audio/Sound.cpp → .../src/HyperSonicDrivers/audio/PCMSound.cpp b/...ers/src/HyperSonicDrivers/audio/Sound.cpp → .../src/HyperSonicDrivers/audio/PCMSound.cpp
@@ -1,9 +1,9 @@
-#include <HyperSonicDrivers/audio/Sound.hpp>
+#include <HyperSonicDrivers/audio/PCMSound.hpp>
 #include <HyperSonicDrivers/utils/ILogger.hpp>
 
 namespace HyperSonicDrivers::audio
 {
-    Sound::Sound(
+    PCMSound::PCMSound(
         const mixer::eChannelGroup group,
         const bool isStereo,
         const uint32_t freq,

diff --git a/...ers/src/HyperSonicDrivers/audio/Sound.hpp → .../src/HyperSonicDrivers/audio/PCMSound.hpp b/...ers/src/HyperSonicDrivers/audio/Sound.hpp → .../src/HyperSonicDrivers/audio/PCMSound.hpp
@@ -12,13 +12,13 @@ namespace HyperSonicDrivers::audio
     * 16 bits signed PCM sound
     * if the original data is not in this format must be converted first
     **/
-    class Sound final
+    class PCMSound final
     {
     public:
-        Sound(Sound&) = delete;
-        Sound(Sound&&) = delete;
-        Sound& operator=(Sound&) = delete;
-        Sound(
+        PCMSound(PCMSound&) = delete;
+        PCMSound(PCMSound&&) = delete;
+        PCMSound& operator=(PCMSound&) = delete;
+        PCMSound(
             const mixer::eChannelGroup group,
             const bool isStereo,
             const uint32_t freq,

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/audio/streams/PCMStream.cpp
@@ -6,7 +6,7 @@ namespace HyperSonicDrivers::audio::streams
 {
     using utils::readLE_uint16;
 
-    PCMStream::PCMStream(const std::shared_ptr<Sound>& sound)
+    PCMStream::PCMStream(const std::shared_ptr<PCMSound>& sound)
         : m_sound(sound)
     {
     }
@@ -38,7 +38,7 @@ namespace HyperSonicDrivers::audio::streams
         return m_curPos == m_sound->dataSize;
     }
 
-    std::weak_ptr<Sound> PCMStream::getSound() const noexcept
+    std::weak_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
@@ -2,25 +2,25 @@
 
 #include <cstdint>
 #include <memory>
-#include <HyperSonicDrivers/audio/Sound.hpp>
+#include <HyperSonicDrivers/audio/PCMSound.hpp>
 #include <HyperSonicDrivers/audio/IAudioStream.hpp>
 
 namespace HyperSonicDrivers::audio::streams
 {
     class PCMStream final : public IAudioStream
     {
     public:
-        explicit PCMStream(const std::shared_ptr<Sound>& sound);
+        explicit PCMStream(const std::shared_ptr<PCMSound>& sound);
         ~PCMStream() override = default;
 
         size_t readBuffer(int16_t* buffer, const size_t numSamples) override;
         bool isStereo() const override;
         uint32_t getRate() const override;
         bool endOfData() const override;
 
-        std::weak_ptr<Sound>  getSound() const noexcept;
+        std::weak_ptr<PCMSound>  getSound() const noexcept;
     private:
-        std::shared_ptr<Sound> m_sound;
+        std::shared_ptr<PCMSound> m_sound;
         uint32_t m_curPos = 0;
     };
 }
diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/devices/IDevice.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/devices/IDevice.hpp
@@ -12,6 +12,7 @@ namespace HyperSonicDrivers::devices
 {
     /**
     * general interface for sound cards used by the drivers
+    * that is bound to a specific hardware
     **/
     class IDevice
     {
@@ -20,8 +21,8 @@ namespace HyperSonicDrivers::devices
         virtual ~IDevice() = default;
 
         virtual bool init() noexcept = 0;
-        inline bool isInit() const noexcept { return m_init; };
         virtual bool shutdown() noexcept = 0;
+        inline bool isInit() const noexcept { return m_init; };
 
         std::optional<uint8_t> getChannelId() const noexcept { return m_hardware->getChannelId(); };
 

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/devices/MT32.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/devices/MT32.hpp
@@ -20,11 +20,8 @@ namespace HyperSonicDrivers::devices
         );
         ~MT32() override = default;
 
-        //void sendEvent(const audio::midi::MIDIEvent& e) const noexcept override;
-        //void sendMessage(const uint8_t msg[], const uint8_t size) const noexcept override;
-        //void sendSysEx(const audio::midi::MIDIEvent& e) const noexcept override;
-        //void pause() const noexcept override;
-        //void resume() const noexcept override;
+        virtual bool init() noexcept = 0;
+        virtual bool shutdown() noexcept = 0;
 
     private:
         std::shared_ptr<hardware::mt32::MT32> m_mt32;

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/IMusicDriver.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/IMusicDriver.hpp
@@ -4,6 +4,9 @@ namespace HyperSonicDrivers::drivers
 {
     /**
     * Common Interface for music drivers: MIDI, ADL, XMI, MUS...
+    * TODO/Rename: if not merged with IMidiDriver, this can be renamed as IMusicPlayer
+    *              related to playing those files rather then using a driver,
+    *              ADLDriver at the moment is both
     * 
     **/
     class IMusicDriver

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/MIDDriver.cpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/MIDDriver.cpp
@@ -40,6 +40,11 @@ namespace HyperSonicDrivers::drivers
         const uint8_t pan
     ) : m_device(device), m_group(group), m_volume(volume), m_pan(pan)
     {
+        // TODO: move the acquire logic where the callback is set
+        // NOTE/TODO: this brings up the acquire should set up the callback too?
+        // it will brings to store m_device into IMidiDriver and pass it in that constructor...
+        // so not sure at the moment, but i think the driver should be responsible to acquire the hardware/device
+        // when they are open, and release it when they are closed
         if (!m_device->acquire(this))
         {
             utils::throwLogE<std::runtime_error>("Device is already in used by another driver or can't be init");
@@ -84,8 +89,8 @@ namespace HyperSonicDrivers::drivers
 
         m_midiDriver.reset();
 
-        auto opl = std::dynamic_pointer_cast<devices::Opl>(m_device)->getOpl();
-        auto opl_drv = std::make_unique<drivers::midi::opl::OplDriver>(opl);
+        //auto opl = std::dynamic_pointer_cast<devices::Opl>(m_device)->getOpl();
+        auto opl_drv = std::make_unique<drivers::midi::opl::OplDriver>(std::dynamic_pointer_cast<devices::Opl>(m_device));
         opl_drv->setOP2Bank(op2Bank);
         m_midiDriver = std::move(opl_drv);
         return open_();
@@ -95,11 +100,7 @@ namespace HyperSonicDrivers::drivers
     {
         if (m_device->isOpl())
         {
-            using hardware::opl::OplType;
-
-            auto opl = std::dynamic_pointer_cast<devices::Opl>(m_device)->getOpl();
-            m_midiDriver = std::make_unique<drivers::midi::scummvm::MidiDriver_ADLIB>(
-                opl, opl->type != OplType::OPL2);
+            m_midiDriver = std::make_unique<drivers::midi::scummvm::MidiDriver_ADLIB>(std::dynamic_pointer_cast<devices::Opl>(m_device));
         }
         else
         {
@@ -109,7 +110,7 @@ namespace HyperSonicDrivers::drivers
 
         return open_();
     }
-    
+
     //void MIDDriver::play(const std::shared_ptr<audio::MIDI>& midi) noexcept
     //{
     //    using audio::midi::MIDI_FORMAT;
@@ -167,6 +168,34 @@ namespace HyperSonicDrivers::drivers
             return;
         }
 
+        // TODO: this could be to set the callback frequency?
+        //       this block is doing nothing now.....
+        if (m_midi->division & 0x8000)
+        {
+            // ticks per frame
+            int smpte = (m_midi->division & 0x7FFF) >> 8;
+            int ticksPerFrame = m_midi->division & 0xFF;
+            switch (smpte)
+            {
+            case -24:
+            case -25:
+            case -29:
+            case -30:
+                logW("SMPTE not implemented yet");
+                break;
+            default:
+                logW(std::format("Division SMPTE not known = {}", smpte));
+            }
+
+            logD(std::format("Division: Ticks per frame = {}, {}", ticksPerFrame, smpte));
+            logW("division ticks per frame not implemented yet");
+        }
+        else
+        {
+            // ticks per quarter note
+            logD(std::format("Division: Ticks per quarter note = {}", m_midi->division & 0x7FFF));
+        }
+
         stop();
         if (!m_device->acquire(this)) {
             return;
@@ -177,7 +206,7 @@ namespace HyperSonicDrivers::drivers
         m_isPlaying = true;
     }
 
-    void MIDDriver::stop(/*const bool wait*/) noexcept
+    void MIDDriver::stop() noexcept
     {
         m_force_stop = true;
         m_paused = false;

diff --git a/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/MIDDriver.hpp b/sdl2-hyper-sonic-drivers/src/HyperSonicDrivers/drivers/MIDDriver.hpp
@@ -15,13 +15,14 @@
 
 namespace HyperSonicDrivers::drivers
 {
-    // TODO/NOTE: this is more the OnTimer logic and setup of OplDriver, but generalized
-    //            for whatever midi::driver ... need to be reivewed
-    // TODO: maybe rename it to midi_player
-    // TODO: deprecated class
+    /**
+    * This class is a wrapper around different midi drivers and has a embeeded timer processing track
+    * to send events to the device
+    **/
     class MIDDriver : public IMusicDriver
     {
     public:
+        [[deprecated("it will be replaced by another class without thread")]]
         explicit MIDDriver(
             const std::shared_ptr<devices::IDevice>& device,
             const audio::mixer::eChannelGroup group,
@@ -36,9 +37,6 @@ namespace HyperSonicDrivers::drivers
         // this restore the default MidiDriver (scummvm::MidiAdlib, MT32)
         bool resetBankOP2() noexcept;
 
-        //[[deprecated("use the other play method")]]
-        //void play(const std::shared_ptr<audio::MIDI>& midi) noexcept;
-
         void play(const uint8_t track = 0) noexcept override;
         void stop() noexcept override;
 
@@ -58,14 +56,15 @@ namespace HyperSonicDrivers::drivers
         inline void setTempo(const uint32_t tempo) noexcept { m_midiTempoChanged = true; m_tempo = tempo; }
         bool open_() noexcept;
     private:
+        // this is used to "lock" the device to a specific driver output and passed to IMidiDriver
         std::shared_ptr<devices::IDevice> m_device;
+        // this is to abstract the specific midi driver implementation
         std::unique_ptr<drivers::midi::IMidiDriver> m_midiDriver;
         std::shared_ptr<audio::MIDI> m_midi;
         const audio::mixer::eChannelGroup m_group;
         const uint8_t m_volume;
         const uint8_t m_pan;
-        // TODO: consider to create a utils/Thread class
-        //       to handle for each OS specific realtime and initialization step.
+
         std::jthread m_player;
 
         std::atomic<bool> m_isPlaying = false;