Moved Player/Transport interaction to using a simple callback/listener

interface instead of events. Events were causing complications with
multi-threading, and introducing unnecessary runtime complexity.

src/contrib/alsaout/AlsaOut.cpp

@@ -34,7 +34,7 @@
 
 #include "AlsaOut.h"
 
-#define BUFFER_COUNT 8
+#define BUFFER_COUNT 32
 #define PCM_ACCESS_TYPE SND_PCM_ACCESS_RW_INTERLEAVED
 #define PCM_FORMAT SND_PCM_FORMAT_FLOAT_LE
 
@@ -56,9 +56,14 @@
 static inline bool playable(snd_pcm_t* pcm) {
     snd_pcm_state_t state = snd_pcm_state(pcm);
 
-    return
+    if (state == SND_PCM_STATE_RUNNING ||
-        state == SND_PCM_STATE_RUNNING ||
+        state == SND_PCM_STATE_PREPARED)
-        state == SND_PCM_STATE_PREPARED;
+    {
+        return true;
+    }
+
+    std::cerr << "AlsaOut: invalid device state: " << (int) state << "\n";
+    return false;
 }
 
 using namespace musik::core::sdk;
@@ -238,7 +243,7 @@ void AlsaOut::WriteLoop() {
                 float volume = (float) this->volume;
 
                 /* software volume; alsa doesn't support this internally. this is about
-                as terrible as an algorithm can be -- it's just a linear ramp. */\
+                as terrible as an algorithm can be -- it's just a linear ramp. */
                 if (volume != 1.0f) {
                     float *buffer = next->buffer->BufferPointer();
                     for (size_t i = 0; i < samples; i++) {
@@ -272,12 +277,14 @@ bool AlsaOut::Play(IBuffer *buffer, IBufferProvider* provider) {
     {
         LOCK("play");
 
-        if (!playable(this->pcmHandle) ||
+        if (this->CountBuffersWithProvider(provider) >= BUFFER_COUNT) {
-            this->CountBuffersWithProvider(provider) >= BUFFER_COUNT)
-        {
             return false;
         }
 
+        if (!playable(this->pcmHandle)) {
+            std::cerr << "AlsaOut: sanity check -- stream not playable. adding buffer to queue anyway\n";
+        }
+
         std::shared_ptr<BufferContext> context(new BufferContext());
         context->buffer = buffer;
         context->provider = provider;

src/contrib/coreaudioout/CoreAudioOut.cpp

@@ -36,7 +36,7 @@
 
 #include <iostream>
 
-#define BUFFER_COUNT 16
+#define BUFFER_COUNT 32
 
 using namespace musik::core::sdk;
 

src/contrib/waveout/WaveOut.cpp

@@ -35,7 +35,7 @@
 #include "WaveOut.h"
 
 #define MAX_VOLUME 0xFFFF
-#define MAX_BUFFERS_PER_OUTPUT 16
+#define MAX_BUFFERS_PER_OUTPUT 32
 
 static void notifyBufferProcessed(WaveOutBuffer *buffer) {
     /* let the provider know the output device is done with the buffer; the

src/core/audio/GaplessTransport.cpp

@@ -45,18 +45,11 @@ using namespace musik::core::sdk;
 
 static std::string TAG = "Transport";
 
-#define DISCONNECT_AND_DESTROY_PLAYER(instance, player) \
-    if (player) { \
-        player->PlaybackAlmostEnded.disconnect(instance); \
-        player->PlaybackError.disconnect(instance); \
-        player->PlaybackFinished.disconnect(instance); \
-        player->PlaybackStarted.disconnect(instance); \
-        player->Destroy(); \
-    }
-
 #define RESET_NEXT_PLAYER(instance) \
-    DISCONNECT_AND_DESTROY_PLAYER(instance, this->nextPlayer); \
+    if (instance->nextPlayer) { \
-    instance->nextPlayer = nullptr;
+        instance->nextPlayer->Destroy(); \
+        instance->nextPlayer = nullptr; \
+    }
 
 GaplessTransport::GaplessTransport()
 : volume(1.0)
@@ -78,7 +71,7 @@ GaplessTransport::~GaplessTransport() {
     }
 
     for (auto it = players.begin(); it != players.end(); it++) {
-        DISCONNECT_AND_DESTROY_PLAYER(this, (*it));
+        (*it)->Destroy();
     }
 }
 
@@ -92,7 +85,7 @@ void GaplessTransport::PrepareNextTrack(const std::string& trackUrl) {
     {
         LockT lock(this->stateMutex);
         RESET_NEXT_PLAYER(this);
-        this->nextPlayer = Player::Create(trackUrl, this->output);
+        this->nextPlayer = Player::Create(trackUrl, this->output, this);
         startNext = this->nextCanStart;
     }
 
@@ -104,7 +97,7 @@ void GaplessTransport::PrepareNextTrack(const std::string& trackUrl) {
 void GaplessTransport::Start(const std::string& url) {
     musik::debug::info(TAG, "we were asked to start the track at " + url);
 
-    Player* newPlayer = Player::Create(url, this->output);
+    Player* newPlayer = Player::Create(url, this->output, this);
     musik::debug::info(TAG, "Player created successfully");
 
     this->StartWithPlayer(newPlayer);
@@ -112,11 +105,6 @@ void GaplessTransport::Start(const std::string& url) {
 
 void GaplessTransport::StartWithPlayer(Player* newPlayer) {
     if (newPlayer) {
-        newPlayer->PlaybackStarted.connect(this, &GaplessTransport::OnPlaybackStarted);
-        newPlayer->PlaybackAlmostEnded.connect(this, &GaplessTransport::OnPlaybackAlmostEnded);
-        newPlayer->PlaybackFinished.connect(this, &GaplessTransport::OnPlaybackFinished);
-        newPlayer->PlaybackError.connect(this, &GaplessTransport::OnPlaybackError);
-
         bool playingNext = false;
 
         {
@@ -124,7 +112,7 @@ void GaplessTransport::StartWithPlayer(Player* newPlayer) {
 
             playingNext = (newPlayer == nextPlayer);
             if (nextPlayer != nullptr && newPlayer != nextPlayer) {
-                DISCONNECT_AND_DESTROY_PLAYER(this, this->nextPlayer);
+                this->nextPlayer->Destroy();
             }
 
             this->nextPlayer = nullptr;
@@ -172,7 +160,7 @@ void GaplessTransport::StopInternal(
             auto it = this->active.begin();
             while (it != this->active.end()) {
                 if (*it != exclude) {
-                    DISCONNECT_AND_DESTROY_PLAYER(this, (*it));
+                    (*it)->Destroy();
                     it = this->active.erase(it);
                 }
                 else {
@@ -311,7 +299,7 @@ void GaplessTransport::RemoveFromActive(Player* player) {
 
     /* outside of the critical section, otherwise potential deadlock */
     if (found) {
-        DISCONNECT_AND_DESTROY_PLAYER(this, player);
+        player->Destroy();
     }
 }
 

src/core/audio/GaplessTransport.h

@@ -47,7 +47,7 @@
 
 namespace musik { namespace core { namespace audio {
 
-    class GaplessTransport : public ITransport, public sigslot::has_slots<> {
+    class GaplessTransport : public ITransport, private Player::PlayerEventListener, public sigslot::has_slots<> {
         public:
             GaplessTransport();
             virtual ~GaplessTransport();
@@ -88,10 +88,10 @@ namespace musik { namespace core { namespace audio {
             void RaiseStreamEvent(int type, Player* player);
             void SetPlaybackState(int state);
 
-            void OnPlaybackStarted(Player* player);
+            virtual void OnPlaybackStarted(Player* player);
-            void OnPlaybackAlmostEnded(Player* player);
+            virtual void OnPlaybackAlmostEnded(Player* player);
-            void OnPlaybackFinished(Player* player);
+            virtual void OnPlaybackFinished(Player* player);
-            void OnPlaybackError(Player* player);
+            virtual void OnPlaybackError(Player* player);
 
             musik::core::sdk::PlaybackState state;
             std::recursive_mutex stateMutex;

src/core/audio/Player.cpp

@@ -102,8 +102,8 @@ namespace musik {
     }
 }
 
-Player* Player::Create(const std::string &url, OutputPtr output) {
+Player* Player::Create(const std::string &url, OutputPtr output, PlayerEventListener *listener) {
-    return new Player(url, output);
+    return new Player(url, output, listener);
 }
 
 OutputPtr Player::CreateDefaultOutput() {
@@ -121,14 +121,15 @@ OutputPtr Player::CreateDefaultOutput() {
     return OutputPtr();
 }
 
-Player::Player(const std::string &url, OutputPtr output)
+Player::Player(const std::string &url, OutputPtr output, PlayerEventListener *listener)
 : state(Player::Precache)
 , url(url)
 , currentPosition(0)
 , output(output)
 , notifiedStarted(false)
 , setPosition(-1)
-, fftContext(nullptr) {
+, fftContext(nullptr)
+, listener(listener) {
     musik::debug::info(TAG, "new instance created");
 
     this->spectrum = new float[FFT_N / 2];
@@ -288,10 +289,13 @@ void musik::core::audio::playerThreadLoop(Player* player) {
                     buffer.reset(); /* important! we're done with this one locally. */
                 }
                 else {
-                    /* the output device queue is full. we should block and wait until
+                    /* the output device queue is probably full. we should block and wait until
-                    the output lets us know that it needs more data */
+                    the output lets us know that it needs more data. if we are starved for
+                    more than a second, try to push the buffer into the output again. this
+                    may happen if the sound driver has some sort of transient problem and
+                    is temporarily unable to process the bufer (ALSA, i'm looking at you) */
                     std::unique_lock<std::mutex> lock(player->queueMutex);
-                    player->writeToOutputCondition.wait(lock);
+                    player->writeToOutputCondition.wait_for(lock, std::chrono::milliseconds(1000));
                 }
             }
             /* if we're unable to obtain a buffer, it means we're out of data and the
@@ -303,18 +307,18 @@ void musik::core::audio::playerThreadLoop(Player* player) {
 
         /* if the Quit flag isn't set, that means the stream has ended "naturally", i.e.
         it wasn't stopped by the user. raise the "almost ended" flag. */
-        if (!player->Exited()) {
+        if (!player->Exited() && player->listener) {
-            player->PlaybackAlmostEnded(player);
+            player->listener->OnPlaybackAlmostEnded(player);
         }
     }
 
     /* if the stream failed to open... */
     else {
-        player->PlaybackError(player);
+        if (!player->Exited() && player->listener) {
+            player->listener->OnPlaybackError(player);
+        }
     }
 
-    player->state = Player::Quit;
-
     /* unlock any remaining buffers... see comment above */
     if (buffer) {
         player->OnBufferProcessed(buffer.get());
@@ -330,7 +334,11 @@ void musik::core::audio::playerThreadLoop(Player* player) {
         }
     }
 
-    player->PlaybackFinished(player);
+    if (!player->Exited() && player->listener) {
+        player->listener->OnPlaybackFinished(player);
+    }
+
+    player->state = Player::Quit;
 
     delete player;
 }
@@ -484,6 +492,8 @@ void Player::OnBufferProcessed(IBuffer *buffer) {
     and send them to the output before they are actually processed by the
     output device */
     if (started) {
-        this->PlaybackStarted(this);
+        if (!this->Exited() && this->listener) {
+            this->listener->OnPlaybackStarted(this);
+        }
     }
 }

src/core/audio/Player.h

@@ -54,11 +54,20 @@ namespace musik { namespace core { namespace audio {
 
     class Player : public musik::core::sdk::IBufferProvider {
         public:
+            class PlayerEventListener {
+                public:
+                    virtual void OnPlaybackStarted(Player *player) = 0;
+                    virtual void OnPlaybackAlmostEnded(Player *player) = 0;
+                    virtual void OnPlaybackFinished(Player *player) = 0;
+                    virtual void OnPlaybackError(Player *player) = 0;
+            };
+
             static OutputPtr CreateDefaultOutput();
 
             static Player* Create(
                 const std::string &url,
-                OutputPtr output);
+                OutputPtr output,
+                PlayerEventListener *listener);
 
             virtual void OnBufferProcessed(musik::core::sdk::IBuffer *buffer);
 
@@ -72,12 +81,6 @@ namespace musik { namespace core { namespace audio {
 
             bool Exited();
 
-            typedef sigslot::signal1<Player*> PlayerEvent;
-            PlayerEvent PlaybackStarted;
-            PlayerEvent PlaybackAlmostEnded;
-            PlayerEvent PlaybackFinished;
-            PlayerEvent PlaybackError;
-
         private:
             bool PreBuffer();
             int State();
@@ -88,7 +91,8 @@ namespace musik { namespace core { namespace audio {
 
             Player(
                 const std::string &url,
-                OutputPtr output);
+                OutputPtr output,
+                PlayerEventListener *listener);
 
             virtual ~Player();
 
@@ -106,6 +110,7 @@ namespace musik { namespace core { namespace audio {
             StreamPtr stream;
             ThreadPtr thread;
             BufferList lockedBuffers;
+            PlayerEventListener* listener;
 
             std::string url;