Revise code

- revise media_packet_pool_acquire_packet()
 : Add defensive code for spurious wakeup issue.
- remove unnecessary member variable in media_packet_pool_s
- change some member variable in media_packet_pool_s

[Version] 0.1.21
[Issue Type] Update

Change-Id: I1352340bccc08d63364c5ec1af55c0cb5a5456f4
Signed-off-by: Jeongmo Yang <jm80.yang@samsung.com>

diff --git a/include/media_packet_pool_private.h b/include/media_packet_pool_private.h
index 77edce17bd4963fac1fb77edb7175593ddaf20c2..4df57e9846b62035aad03d675c2a16b4fcadd7da 100644 (file)
--- a/include/media_packet_pool_private.h
+++ b/include/media_packet_pool_private.h
@@ -48,10 +48,9 @@ typedef struct _media_packet_pool_s {
        GMutex mutex;
        GQueue *queue;
        GCond queue_cond;
-       unsigned int curr_pool_size;
-       unsigned int min_pool_size;
-       unsigned int max_pool_size;
-       gboolean pool_created;
+       unsigned int pool_size;
+       unsigned int pool_size_min;
+       unsigned int pool_size_max;
        gboolean pool_allocated;
        media_format_h fmt_h;
        media_packet_h packet[MAX_PACKET];

diff --git a/packaging/capi-media-tool.spec b/packaging/capi-media-tool.spec
index c3412a8d93ff5abd200ea7a7247ba0ae4269f00d..bba00554f51cc93d439515d88f213ac75e2696da 100755 (executable)
--- a/packaging/capi-media-tool.spec
+++ b/packaging/capi-media-tool.spec
@@ -1,6 +1,6 @@
 Name:       capi-media-tool
 Summary:    A Core API media tool library in Tizen Native API
-Version:    0.1.20
+Version:    0.1.21
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0

diff --git a/src/media_packet_pool.c b/src/media_packet_pool.c
index c2b25c7c7e514ec1605f0c7db39ac26397ca66dc..049d6150e5d9c697c41b5aaa2ba3acdf06f7b50c 100644 (file)
--- a/src/media_packet_pool.c
+++ b/src/media_packet_pool.c
@@ -41,13 +41,12 @@ int media_packet_pool_create(media_packet_pool_h *pool)
                ret = MEDIA_PACKET_ERROR_OUT_OF_MEMORY;
                return ret;
        }
-       pool_handle->pool_created = TRUE;
 
        /* create Gqueue */
        pool_handle->queue = g_queue_new();
-       pool_handle->curr_pool_size = 0;
-       pool_handle->min_pool_size = 0;
-       pool_handle->max_pool_size = 0;
+       pool_handle->pool_size = 0;
+       pool_handle->pool_size_min = 0;
+       pool_handle->pool_size_max = 0;
        pool_handle->pool_allocated = false;
 
        /* take memory packet pool handle */
@@ -88,11 +87,11 @@ int media_packet_pool_set_size(media_packet_pool_h pool, int min_buffers, int ma
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (max_buffers < 0 || min_buffers < 0 || min_buffers > max_buffers)
+       if (max_buffers <= 0 || min_buffers <= 0 || min_buffers > max_buffers)
                return MEDIA_PACKET_ERROR_INVALID_PARAMETER;
 
-       pool_handle->min_pool_size = min_buffers;
-       pool_handle->max_pool_size = max_buffers;
+       pool_handle->pool_size_min = min_buffers;
+       pool_handle->pool_size_max = max_buffers;
 
        return ret;
 }
@@ -106,11 +105,11 @@ int media_packet_pool_get_size(media_packet_pool_h pool, int *min_size, int *max
        pool_handle = (media_packet_pool_s *)pool;
 
        if (curr_size)
-               *curr_size = pool_handle->curr_pool_size;
+               *curr_size = pool_handle->pool_size;
        if (min_size)
-               *min_size = pool_handle->min_pool_size;
+               *min_size = pool_handle->pool_size_min;
        if (max_size)
-               *max_size = pool_handle->max_pool_size;
+               *max_size = pool_handle->pool_size_max;
 
        return MEDIA_PACKET_ERROR_NONE;
 }
@@ -143,17 +142,12 @@ int media_packet_pool_allocate(media_packet_pool_h pool)
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (!(pool_handle->pool_created)) {
-               LOGE("The media packet pool is not created.."); //LCOV_EXCL_LINE
-               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
-       }
-
-       if (!(pool_handle->min_pool_size)) {
+       if (pool_handle->pool_size_min == 0) {
                LOGE("The media packet pool size is not set. set pool size...");        //LCOV_EXCL_LINE
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
-       if (pool_handle->curr_pool_size >= pool_handle->max_pool_size) {
+       if (pool_handle->pool_size >= pool_handle->pool_size_max) {
                LOGE("The media packet pool is full..");        //LCOV_EXCL_LINE
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
 
@@ -164,14 +158,14 @@ int media_packet_pool_allocate(media_packet_pool_h pool)
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
-       for (i = 0; i < pool_handle->min_pool_size; i++) {
+       for (i = 0; i < pool_handle->pool_size_min; i++) {
                if (media_packet_create_alloc(pool_handle->fmt_h, _packet_finalize_cb, pool_handle, &pool_handle->packet[i]) != MEDIA_PACKET_ERROR_NONE) {
                        LOGE("The media packet pool is full or out of memory...");      //LCOV_EXCL_LINE
                        return MEDIA_PACKET_ERROR_INVALID_OPERATION;
                }
                g_queue_push_tail(pool_handle->queue, pool_handle->packet[i]);
                LOGD("[%d]%p queued", i, pool_handle->packet[i]);
-               pool_handle->curr_pool_size++;
+               pool_handle->pool_size++;
        }
 
        /* increase format reference count */
@@ -194,72 +188,65 @@ int media_packet_pool_acquire_packet(media_packet_pool_h pool, media_packet_h *p
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (timeout != -1) {
-               gint64 add = timeout;
-               wait_until = g_get_monotonic_time() + add;
-       }
-
        if (!g_atomic_int_get(&pool_handle->pool_allocated)) {
                LOGE("The media packet pool is not allocated...");      //LCOV_EXCL_LINE
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
+       if (timeout != -1)
+               wait_until = g_get_monotonic_time() + (gint64)timeout;
+
        g_mutex_lock(&pool_handle->mutex);
-       if (!(pool_handle->curr_pool_size)) {
-               g_mutex_unlock(&pool_handle->mutex);
+
+       if (pool_handle->pool_size == 0) {
                LOGE("The media packet pool size is not set. set pool size...");        //LCOV_EXCL_LINE
+               g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
-       while (TRUE) {
-               packet = g_queue_pop_head(pool_handle->queue);
-
-               if (packet)
-                       break;
+       while (!(packet = g_queue_pop_head(pool_handle->queue))) {
+               if (pool_handle->pool_size < pool_handle->pool_size_max) {
+                       LOGD("no packet in pool, but able to allocate new one [%d/%d]",
+                               pool_handle->pool_size, pool_handle->pool_size_max);
 
-               if (pool_handle->curr_pool_size < pool_handle->max_pool_size) {
-                       LOGD("no buffer, trying to allocate");
-
-                       if (media_packet_create_alloc(pool_handle->fmt_h, _packet_finalize_cb, pool_handle, &pool_handle->packet[pool_handle->curr_pool_size]) != MEDIA_PACKET_ERROR_NONE) {
-                               LOGE("The media packet pool is full or out of memory...");
-                               g_mutex_unlock(&pool_handle->mutex);
-                               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
+                       if (media_packet_create_alloc(pool_handle->fmt_h, _packet_finalize_cb,
+                               pool_handle, &pool_handle->packet[pool_handle->pool_size]) != MEDIA_PACKET_ERROR_NONE) {
+                               LOGE("failed to allocate new packet");
+                               ret = MEDIA_PACKET_ERROR_INVALID_OPERATION;
+                               break;
                        }
-                       packet = pool_handle->packet[pool_handle->curr_pool_size];
-                       pool_handle->curr_pool_size++;
 
+                       packet = pool_handle->packet[pool_handle->pool_size++];
                        break;
-               } else if (timeout == -1) {
-                       LOGD("Queue is empty, waiting for till release is done");
-                       g_cond_wait(&pool_handle->queue_cond, &pool_handle->mutex);
-                       packet = g_queue_pop_head(pool_handle->queue);
-                       break;
+               }
 
-               } else {
-                       if (!g_cond_wait_until(&pool_handle->queue_cond, &pool_handle->mutex, wait_until)) {
-                               LOGW("signal not received");
-                               break;
-                       }
-                       LOGD("Queue is Empty, waiting for timeout %" G_GSIZE_FORMAT "", wait_until);
-                       packet = g_queue_pop_head(pool_handle->queue);
-                       if (!packet) {
-                               LOGE("pkt is null");
-                               ret = MEDIA_PACKET_ERROR_NO_AVAILABLE_PACKET;
-                       }
+               /* wait for packet release signal */
+               if (timeout == -1) {
+                       LOGD("no available packet, wait for packet release");
+                       g_cond_wait(&pool_handle->queue_cond, &pool_handle->mutex);
+               } else if (!g_cond_wait_until(&pool_handle->queue_cond, &pool_handle->mutex, wait_until)) {
+                       LOGE("packet release signal timed out");
+                       ret = MEDIA_PACKET_ERROR_NO_AVAILABLE_PACKET;
                        break;
                }
+
+               LOGD("signal[timeout:%ld] received", timeout);
+       }
+
+       if (packet) {
+               LOGI("acquired packet %p", packet);
+               *pkt = (media_packet_h)packet;
        }
 
-       *pkt = (media_packet_h)packet;
        g_mutex_unlock(&pool_handle->mutex);
-       LOGD("The packet handle aquired from pool is  %p..", packet);
+
        return ret;
 }
 
 gboolean _is_packet_in_queue(media_packet_pool_s *pool_handle, media_packet_h packet)
 {
        int i;
-       for (i = 0; i < pool_handle->curr_pool_size; i++) {
+       for (i = 0; i < pool_handle->pool_size; i++) {
                if (pool_handle->packet[i] == packet)
                        return true;
        }
@@ -286,7 +273,7 @@ int media_packet_pool_release_packet(media_packet_pool_h pool, media_packet_h pk
 
        num_pkts = g_queue_get_length(pool_handle->queue);
 
-       if (num_pkts == pool_handle->curr_pool_size) {
+       if (num_pkts == pool_handle->pool_size) {
                LOGE("Queue is already full");  //LCOV_EXCL_LINE
                g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
@@ -322,13 +309,10 @@ int media_packet_pool_deallocate(media_packet_pool_h pool)
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (!(pool_handle->pool_created))
-               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
-
        g_mutex_lock(&pool_handle->mutex);
        num_pkts = g_queue_get_length(pool_handle->queue);
 
-       if (num_pkts < pool_handle->curr_pool_size) {
+       if (num_pkts < pool_handle->pool_size) {
                LOGE("packet is being used, release the packet before deallocate");             //LCOV_EXCL_LINE
                g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
@@ -374,9 +358,6 @@ int media_packet_pool_destroy(media_packet_pool_h pool)
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (!(pool_handle->pool_created))
-               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
-
        g_mutex_lock(&pool_handle->mutex);
        num_pkts = g_queue_get_length(pool_handle->queue);
        g_mutex_unlock(&pool_handle->mutex);