Locking cleanups

3 files changed, 133 insertions, 138 deletions

diff --git a/include/kitchensink/internal/kitdecoder.h b/include/kitchensink/internal/kitdecoder.h
index 1bddda5..ef3edb7 100644
--- a/include/kitchensink/internal/kitdecoder.h
+++ b/include/kitchensink/internal/kitdecoder.h
@@ -13,8 +13,11 @@
 #include "kitchensink/kitsource.h"

 #include "kitchensink/internal/utils/kitbuffer.h"

 

-#define KIT_DEC_IN 0

-#define KIT_DEC_OUT 1

+enum {

+    KIT_DEC_BUF_IN = 0,

+    KIT_DEC_BUF_OUT,

+    KIT_DEC_BUF_COUNT

+};

 

 typedef struct Kit_Decoder Kit_Decoder;

 

@@ -31,7 +34,7 @@ KIT_LOCAL struct Kit_Decoder {
     AVCodecContext *codec_ctx;   ///< FFMpeg internal: Codec context

     AVFormatContext *format_ctx; ///< FFMpeg internal: Format context (owner: Kit_Source)

 

-    SDL_mutex *lock[2];          ///< Threading locks for input and output buffers

+    SDL_mutex *output_lock;      ///< Threading lock for output buffer

     Kit_Buffer *buffer[2];       ///< Buffers for incoming and decoded packets

 

     void *userdata;              ///< Decoder specific information (Audio, video, subtitle context)

diff --git a/src/internal/kitdecoder.c b/src/internal/kitdecoder.c
index b57cbc4..e2d0a25 100644
--- a/src/internal/kitdecoder.c
+++ b/src/internal/kitdecoder.c
@@ -71,23 +71,25 @@ Kit_Decoder* Kit_CreateDecoder(const Kit_Source *src, int stream_index,
     for(int i = 0; i < 2; i++) {

         dec->buffer[i] = Kit_CreateBuffer(bsizes[i], free_hooks[i]);

         if(dec->buffer[i] == NULL) {

-            Kit_SetError("Unable to allocate ringbuffer type %d for stream %d", i, stream_index);

+            Kit_SetError("Unable to allocate buffer for stream %d: %s", stream_index, SDL_GetError());

             goto exit_3;

         }

+    }

 

-        dec->lock[i] = SDL_CreateMutex();

-        if(dec->lock[i] == NULL) {

-            Kit_SetError("Unable to allocate mutex type %d for stream %d", i, stream_index);

-            goto exit_3;

-        }

+    // Create a lock for output buffer synchronization

+    dec->output_lock = SDL_CreateMutex();

+    if(dec->output_lock == NULL) {

+        Kit_SetError("Unable to allocate mutex for stream %d: %s", stream_index, SDL_GetError());

+        goto exit_4;

     }

 

     // That's that

     return dec;

 

+exit_4:

+    SDL_DestroyMutex(dec->output_lock);

 exit_3:

-    for(int i = 0; i < 2; i++) {

-        SDL_DestroyMutex(dec->lock[i]);

+    for(int i = 0; i < KIT_DEC_BUF_COUNT; i++) {

         Kit_DestroyBuffer(dec->buffer[i]);

     }

     avcodec_close(codec_ctx);

@@ -99,6 +101,51 @@ exit_0:
     return NULL;

 }

 

+void Kit_CloseDecoder(Kit_Decoder *dec) {

+    if(dec == NULL) return;

+    if(dec->dec_close) {

+        dec->dec_close(dec);

+    }

+    for(int i = 0; i < KIT_DEC_BUF_COUNT; i++) {

+        Kit_DestroyBuffer(dec->buffer[i]);

+    }

+    SDL_DestroyMutex(dec->output_lock);

+    avcodec_close(dec->codec_ctx);

+    avcodec_free_context(&dec->codec_ctx);

+    free(dec);

+}

+

+int Kit_RunDecoder(Kit_Decoder *dec) {

+    if(dec == NULL) return 0;

+

+    AVPacket *in_packet;

+    int is_output_full = 1;

+

+    // First, check if there is room in output buffer

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        is_output_full = Kit_IsBufferFull(dec->buffer[KIT_DEC_BUF_OUT]);

+        SDL_UnlockMutex(dec->output_lock);

+    }

+    if(is_output_full) {

+        return 0;

+    }

+

+    // Then, see if we have incoming data

+    in_packet = Kit_ReadDecoderInput(dec);

+    if(in_packet == NULL) {

+        return 0;

+    }

+

+    // Run decoder with incoming packet

+    dec->dec_decode(dec, in_packet);

+

+    // Free raw packet before returning

+    av_packet_free(&in_packet);

+    return 1;

+}

+

+// ---- Information API ----

+

 int Kit_GetDecoderCodecInfo(const Kit_Decoder *dec, Kit_Codec *codec) {

     if(dec == NULL) {

         memset(codec, 0, sizeof(Kit_Codec));

@@ -125,6 +172,8 @@ int Kit_GetDecoderStreamIndex(const Kit_Decoder *dec) {
     return dec->stream_index;

 }

 

+// ---- Clock handling ----

+

 void Kit_SetDecoderClockSync(Kit_Decoder *dec, double sync) {

     if(dec == NULL)

         return;

@@ -137,66 +186,52 @@ void Kit_ChangeDecoderClockSync(Kit_Decoder *dec, double sync) {
     dec->clock_sync += sync;

 }

 

+// ---- Input buffer handling ----

+

 int Kit_WriteDecoderInput(Kit_Decoder *dec, AVPacket *packet) {

     assert(dec != NULL);

-    int ret = 1;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_IN]) == 0) {

-        ret = Kit_WriteBuffer(dec->buffer[KIT_DEC_IN], packet);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_IN]);

-    }

-    return ret;

+    return Kit_WriteBuffer(dec->buffer[KIT_DEC_BUF_IN], packet);

 }

 

 bool Kit_CanWriteDecoderInput(Kit_Decoder *dec) {

     assert(dec != NULL);

-    bool ret = false;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_IN]) == 0) {

-        ret = !Kit_IsBufferFull(dec->buffer[KIT_DEC_IN]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_IN]);

-    }

-    return ret;

+    return !Kit_IsBufferFull(dec->buffer[KIT_DEC_BUF_IN]);

 }

 

 AVPacket* Kit_ReadDecoderInput(Kit_Decoder *dec) {

     assert(dec != NULL);

-    AVPacket *ret = NULL;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_IN]) == 0) {

-        ret = Kit_ReadBuffer(dec->buffer[KIT_DEC_IN]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_IN]);

-    }

-    return ret;

+    return Kit_ReadBuffer(dec->buffer[KIT_DEC_BUF_IN]);

 }

 

+void Kit_ClearDecoderInput(Kit_Decoder *dec) {

+    Kit_ClearBuffer(dec->buffer[KIT_DEC_BUF_IN]);

+}

+

+// ---- Output buffer handling ----

+

 int Kit_WriteDecoderOutput(Kit_Decoder *dec, void *packet) {

     assert(dec != NULL);

     int ret = 1;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        ret = Kit_WriteBuffer(dec->buffer[KIT_DEC_OUT], packet);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        ret = Kit_WriteBuffer(dec->buffer[KIT_DEC_BUF_OUT], packet);

+        SDL_UnlockMutex(dec->output_lock);

     }

     return ret;

 }

 

 void Kit_ClearDecoderOutput(Kit_Decoder *dec) {

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        Kit_ClearBuffer(dec->buffer[KIT_DEC_OUT]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

-    }

-}

-

-void Kit_ClearDecoderInput(Kit_Decoder *dec) {

-    if(SDL_LockMutex(dec->lock[KIT_DEC_IN]) == 0) {

-        Kit_ClearBuffer(dec->buffer[KIT_DEC_IN]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_IN]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        Kit_ClearBuffer(dec->buffer[KIT_DEC_BUF_OUT]);

+        SDL_UnlockMutex(dec->output_lock);

     }

 }

 

 void* Kit_PeekDecoderOutput(Kit_Decoder *dec) {

     assert(dec != NULL);

     void *ret = NULL;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        ret = Kit_PeekBuffer(dec->buffer[KIT_DEC_OUT]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        ret = Kit_PeekBuffer(dec->buffer[KIT_DEC_BUF_OUT]);

+        SDL_UnlockMutex(dec->output_lock);

     }

     return ret;

 }

@@ -204,26 +239,26 @@ void* Kit_PeekDecoderOutput(Kit_Decoder *dec) {
 void* Kit_ReadDecoderOutput(Kit_Decoder *dec) {

     assert(dec != NULL);

     void *ret = NULL;

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        ret = Kit_ReadBuffer(dec->buffer[KIT_DEC_OUT]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        ret = Kit_ReadBuffer(dec->buffer[KIT_DEC_BUF_OUT]);

+        SDL_UnlockMutex(dec->output_lock);

     }

     return ret;

 }

 

 void Kit_ForEachDecoderOutput(Kit_Decoder *dec, Kit_ForEachItemCallback cb, void *userdata) {

     assert(dec != NULL);

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        Kit_ForEachItemInBuffer(dec->buffer[KIT_DEC_OUT], cb, userdata);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        Kit_ForEachItemInBuffer(dec->buffer[KIT_DEC_BUF_OUT], cb, userdata);

+        SDL_UnlockMutex(dec->output_lock);

     }

 }

 

 void Kit_AdvanceDecoderOutput(Kit_Decoder *dec) {

     assert(dec != NULL);

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        Kit_AdvanceBuffer(dec->buffer[KIT_DEC_OUT]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

+    if(SDL_LockMutex(dec->output_lock) == 0) {

+        Kit_AdvanceBuffer(dec->buffer[KIT_DEC_BUF_OUT]);

+        SDL_UnlockMutex(dec->output_lock);

     }

 }

 

@@ -235,52 +270,9 @@ void Kit_ClearDecoderBuffers(Kit_Decoder *dec) {
 }

 

 int Kit_LockDecoderOutput(Kit_Decoder *dec) {

-    return SDL_LockMutex(dec->lock[KIT_DEC_OUT]);

+    return SDL_LockMutex(dec->output_lock);

 }

 

 void Kit_UnlockDecoderOutput(Kit_Decoder *dec) {

-    SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

-}

-

-int Kit_RunDecoder(Kit_Decoder *dec) {

-    if(dec == NULL) return 0;

-

-    AVPacket *in_packet;

-    int is_output_full = 1;

-

-    // First, check if there is room in output buffer

-    if(SDL_LockMutex(dec->lock[KIT_DEC_OUT]) == 0) {

-        is_output_full = Kit_IsBufferFull(dec->buffer[KIT_DEC_OUT]);

-        SDL_UnlockMutex(dec->lock[KIT_DEC_OUT]);

-    }

-    if(is_output_full) {

-        return 0;

-    }

-

-    // Then, see if we have incoming data

-    in_packet = Kit_ReadDecoderInput(dec);

-    if(in_packet == NULL) {

-        return 0;

-    }

-

-    // Run decoder with incoming packet

-    dec->dec_decode(dec, in_packet);

-

-    // Free raw packet before returning

-    av_packet_free(&in_packet);

-    return 1;

-}

-

-void Kit_CloseDecoder(Kit_Decoder *dec) {

-    if(dec == NULL) return;

-    if(dec->dec_close) {

-        dec->dec_close(dec);

-    }

-    for(int i = 0; i < 2; i++) {

-        SDL_DestroyMutex(dec->lock[i]);

-        Kit_DestroyBuffer(dec->buffer[i]);

-    }

-    avcodec_close(dec->codec_ctx);

-    avcodec_free_context(&dec->codec_ctx);

-    free(dec);

+    SDL_UnlockMutex(dec->output_lock);

 }

diff --git a/src/kitplayer.c b/src/kitplayer.c
index ace5b1d..99786bc 100644
--- a/src/kitplayer.c
+++ b/src/kitplayer.c
@@ -106,10 +106,7 @@ static int _DecoderThread(void *ptr) {
 
                 // Run decoders for a bit
                 for(int i = 0; i < KIT_DEC_COUNT; i++) {
-                    Kit_Decoder *dec = player->decoders[i];
-                    if(dec == NULL)
-                        continue;
-                    while(Kit_RunDecoder(dec) == 1);
+                    while(Kit_RunDecoder(player->decoders[i]) == 1);
                 }
 
                 // Free decoder thread lock.
@@ -117,11 +114,11 @@ static int _DecoderThread(void *ptr) {
             }
 
             // We decoded as much as we could, sleep a bit.
-            SDL_Delay(1);
+            SDL_Delay(2);
         }
 
         // Just idle while waiting for work.
-        SDL_Delay(10);
+        SDL_Delay(25);
     }
 
     return 0;
@@ -202,7 +199,10 @@ void Kit_ClosePlayer(Kit_Player *player) {
     if(player == NULL) return;
 
     // Kill the decoder thread and mutex
-    player->state = KIT_CLOSED;
+    if(SDL_LockMutex(player->dec_lock) == 0) {
+        player->state = KIT_CLOSED;
+        SDL_UnlockMutex(player->dec_lock);
+    }
     SDL_WaitThread(player->dec_thread, NULL);
     SDL_DestroyMutex(player->dec_lock);
 
@@ -319,21 +319,15 @@ void Kit_GetPlayerInfo(const Kit_Player *player, Kit_PlayerInfo *info) {
 }
 
 static void _SetClockSync(Kit_Player *player) {
-    if(SDL_LockMutex(player->dec_lock) == 0) {
-        double sync = _GetSystemTime();
-        for(int i = 0; i < KIT_DEC_COUNT; i++) {
-            Kit_SetDecoderClockSync(player->decoders[i], sync);
-        }
-        SDL_UnlockMutex(player->dec_lock);
+    double sync = _GetSystemTime();
+    for(int i = 0; i < KIT_DEC_COUNT; i++) {
+        Kit_SetDecoderClockSync(player->decoders[i], sync);
     }
 }
 
 static void _ChangeClockSync(Kit_Player *player, double delta) {
-    if(SDL_LockMutex(player->dec_lock) == 0) {
-        for(int i = 0; i < KIT_DEC_COUNT; i++) {
-            Kit_ChangeDecoderClockSync(player->decoders[i], delta);
-        }
-        SDL_UnlockMutex(player->dec_lock);
+    for(int i = 0; i < KIT_DEC_COUNT; i++) {
+        Kit_ChangeDecoderClockSync(player->decoders[i], delta);
     }
 }
 
@@ -345,32 +339,38 @@ Kit_PlayerState Kit_GetPlayerState(const Kit_Player *player) {
 void Kit_PlayerPlay(Kit_Player *player) {
     assert(player != NULL);
     double tmp;
-    switch(player->state) {
-        case KIT_PLAYING:
-        case KIT_CLOSED:
-            break;
-        case KIT_PAUSED:
-            tmp = _GetSystemTime() - player->pause_started;
-            _ChangeClockSync(player, tmp);
-            player->state = KIT_PLAYING;
-            break;
-        case KIT_STOPPED:
-            _SetClockSync(player);
-            player->state = KIT_PLAYING;
-            break;
+    if(SDL_LockMutex(player->dec_lock) == 0) {
+        switch(player->state) {
+            case KIT_PLAYING:
+            case KIT_CLOSED:
+                break;
+            case KIT_PAUSED:
+                tmp = _GetSystemTime() - player->pause_started;
+                _ChangeClockSync(player, tmp);
+                player->state = KIT_PLAYING;
+                break;
+            case KIT_STOPPED:
+                _SetClockSync(player);
+                player->state = KIT_PLAYING;
+                break;
+        }
+        SDL_UnlockMutex(player->dec_lock);
     }
 }
 
 void Kit_PlayerStop(Kit_Player *player) {
     assert(player != NULL);
-    switch(player->state) {
-        case KIT_STOPPED:
-        case KIT_CLOSED:
-            break;
-        case KIT_PLAYING:
-        case KIT_PAUSED:
-            player->state = KIT_STOPPED;
-            break;
+    if(SDL_LockMutex(player->dec_lock) == 0) {
+        switch(player->state) {
+            case KIT_STOPPED:
+            case KIT_CLOSED:
+                break;
+            case KIT_PLAYING:
+            case KIT_PAUSED:
+                player->state = KIT_STOPPED;
+                break;
+        }
+        SDL_UnlockMutex(player->dec_lock);
     }
 }