Improvement code

- media_format_h handling
- Protect some values with mutex

[Version] 1.0.3
[Issue Type] Improvement

Change-Id: I482f7e18035d87aac1b17e3466fbb2c31388c03a
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 b7d0dd3a64b36a3faf862ca0444dba7fd4c9f74e..4b4f48116e5affbfd36d7f39d05cc34b21b3dc25 100644 (file)
--- a/include/media_packet_pool_private.h
+++ b/include/media_packet_pool_private.h
@@ -55,7 +55,7 @@ typedef struct _media_packet_pool_s {
        unsigned int pool_size;
        unsigned int pool_size_min;
        unsigned int pool_size_max;
-       gint pool_allocated;
+       gboolean pool_allocated;
        media_format_h fmt_h;
        media_packet_h packet[MAX_PACKET];
 } media_packet_pool_s;

diff --git a/packaging/capi-media-tool.spec b/packaging/capi-media-tool.spec
index 371c9458f35a644cf9c9db0d7975d00240fae041..1632ab2edd71d46cf3744a7473fa0673efdfce01 100644 (file)
--- 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:    1.0.2
+Version:    1.0.3
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0

diff --git a/src/media_packet_pool.c b/src/media_packet_pool.c
index 6612a90e75dba9d0f6a78c54c09f863d56948ad3..01500ed98bc6834d2c6280188a74e14ade875e36 100644 (file)
--- a/src/media_packet_pool.c
+++ b/src/media_packet_pool.c
@@ -45,7 +45,7 @@ int media_packet_pool_create(media_packet_pool_h *pool)
        pool_handle->pool_size = 0;
        pool_handle->pool_size_min = 0;
        pool_handle->pool_size_max = 0;
-       pool_handle->pool_allocated = 0;
+       pool_handle->pool_allocated = FALSE;
 
        /* take memory packet pool handle */
        *pool = (media_packet_pool_h) pool_handle;
@@ -57,36 +57,55 @@ int media_packet_pool_create(media_packet_pool_h *pool)
 
 int media_packet_pool_set_media_format(media_packet_pool_h pool, media_format_h fmt)
 {
-       int ret = MEDIA_PACKET_ERROR_NONE;
+       int ret = 0;
        media_packet_pool_s *pool_handle = NULL;
        media_format_type_e type;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        MEDIA_PACKET_POOL_NULL_ARG_CHECK(fmt);
 
+       pool_handle = (media_packet_pool_s *)pool;
+
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
        ret = media_format_get_type(fmt, &type);
        if (ret != MEDIA_FORMAT_ERROR_NONE) {
-               LOGE("failed to media_format_get_type()");
+               LOGE("media_format_get_type() failed[0x%x]", ret);
                return MEDIA_PACKET_ERROR_INVALID_PARAMETER;
        }
+
        if (type != MEDIA_FORMAT_AUDIO && type != MEDIA_FORMAT_VIDEO) {
                LOGE("invalid format type, type[0x%x]", type);
                return MEDIA_PACKET_ERROR_INVALID_PARAMETER;
        }
 
-       pool_handle = (media_packet_pool_s *)pool;
+       ret = media_format_ref(fmt);
+       if (ret != MEDIA_FORMAT_ERROR_NONE) {
+               LOGE("media_format_ref() failed[0x%x]", ret);   //LCOV_EXCL_LINE
+               return MEDIA_PACKET_ERROR_INVALID_PARAMETER;
+       }
+
+       if (pool_handle->fmt_h) {
+               LOGI("unref media format[%p]", pool_handle->fmt_h);
+               media_format_unref(pool_handle->fmt_h);
+       }
+
        pool_handle->fmt_h = fmt;
 
-       return ret;
+       return MEDIA_PACKET_ERROR_NONE;
 }
 
 int media_packet_pool_set_size(media_packet_pool_h pool, int min_buffers, int max_buffers)
 {
        media_packet_pool_s *pool_handle = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
        if (max_buffers <= 0 || min_buffers <= 0 ||
                min_buffers > max_buffers || max_buffers > MAX_PACKET) {
                LOGE("invalid size - set[min:%d,max:%d], pool max[%d]",
@@ -103,11 +122,14 @@ int media_packet_pool_set_size(media_packet_pool_h pool, int min_buffers, int ma
 int media_packet_pool_get_size(media_packet_pool_h pool, int *min_size, int *max_size, int *curr_size)
 {
        media_packet_pool_s *pool_handle = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
 
        pool_handle = (media_packet_pool_s *)pool;
 
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
        if (curr_size)
                *curr_size = pool_handle->pool_size;
        if (min_size)
@@ -122,28 +144,40 @@ static int __packet_finalize_cb(media_packet_h packet, int error_code, void *use
 {
        media_packet_pool_h pool = NULL;
        media_packet_pool_s *pool_handle = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        pool = (media_packet_pool_h)user_data;
        pool_handle = (media_packet_pool_s *)pool;
 
-       if (g_atomic_int_get(&pool_handle->pool_allocated)) {
-               if (media_packet_pool_release_packet(pool, packet) != MEDIA_PACKET_ERROR_NONE)
-                       LOGW("media packet couldn't be released. packet(%p) might be in queue", packet);
-               return MEDIA_PACKET_REUSE;
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
+       if (!pool_handle->pool_allocated) {
+               LOGW("pool is not allocated");
+               return MEDIA_PACKET_FINALIZE;
        }
 
-       return MEDIA_PACKET_FINALIZE;
+       /* early unlock for media_packet_pool_release_packet() */
+       g_clear_pointer(&locker, g_mutex_locker_free);
+
+       if (media_packet_pool_release_packet(pool, packet) != MEDIA_PACKET_ERROR_NONE)
+               LOGW("media packet couldn't be released. packet(%p) might be in queue", packet);
+
+       return MEDIA_PACKET_REUSE;
 }
 
 int media_packet_pool_allocate(media_packet_pool_h pool)
 {
-       int i, num_pkts = 0;
-
+       int i = 0;
+       int num_pkts = 0;
+       int ret = 0;
        media_packet_pool_s *pool_handle = NULL;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
        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;
@@ -155,28 +189,28 @@ int media_packet_pool_allocate(media_packet_pool_h pool)
 
        }
 
-       if (g_atomic_int_get(&pool_handle->pool_allocated)) {
-               LOGE("The media packet is already allocated. set media format size...");        //LCOV_EXCL_LINE
+       if (pool_handle->pool_allocated) {
+               LOGE("The media packet is already allocated");  //LCOV_EXCL_LINE
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
        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
+               ret = media_packet_create_alloc(pool_handle->fmt_h,
+                       __packet_finalize_cb, pool_handle, &pool_handle->packet[i]);
+
+               if (ret != MEDIA_PACKET_ERROR_NONE) {
+                       LOGE("media_packet_create_alloc() failed[0x%x]", ret);  //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->pool_size++;
-       }
 
-       /* increase format reference count */
-       if (media_format_ref(pool_handle->fmt_h) != MEDIA_FORMAT_ERROR_NONE) {
-               LOGE("failed to increase ref count");   //LCOV_EXCL_LINE
-               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
+               g_queue_push_tail(pool_handle->queue, pool_handle->packet[i]);
+
+               pool_handle->pool_size++;
        }
 
-       g_atomic_int_set(&pool_handle->pool_allocated, 1);
+       pool_handle->pool_allocated = TRUE;
 
        return MEDIA_PACKET_ERROR_NONE;
 }
@@ -189,20 +223,20 @@ int media_packet_pool_acquire_packet(media_packet_pool_h pool, media_packet_h *p
        gint64 wait_until = -1;
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
-
-       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;
-       }
+       g_autoptr(GMutexLocker) locker = NULL;
 
        if (timeout != -1)
                wait_until = g_get_monotonic_time() + (gint64)timeout;
 
-       g_mutex_lock(&pool_handle->mutex);
+       locker = g_mutex_locker_new(&pool_handle->mutex);
+
+       if (!pool_handle->pool_allocated) {
+               LOGE("pool is not allocated...");       //LCOV_EXCL_LINE
+               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
+       }
 
        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;
        }
 
@@ -240,8 +274,6 @@ int media_packet_pool_acquire_packet(media_packet_pool_h pool, media_packet_h *p
                *pkt = (media_packet_h)packet;
        }
 
-       g_mutex_unlock(&pool_handle->mutex);
-
        return ret;
 }
 
@@ -260,15 +292,15 @@ int media_packet_pool_release_packet(media_packet_pool_h pool, media_packet_h pk
        int num_pkts = 0;
        media_packet_pool_s *pool_handle = NULL;
        GList *find;
+       g_autoptr(GMutexLocker) locker = NULL;
 
        MEDIA_PACKET_POOL_INSTANCE_CHECK(pool);
        pool_handle = (media_packet_pool_s *)pool;
 
-       g_mutex_lock(&pool_handle->mutex);
+       locker = g_mutex_locker_new(&pool_handle->mutex);
 
        if ((find = g_queue_find(pool_handle->queue, pkt))) {
                LOGE("unable to release '%p' to the pool. Already released", pkt);      //LCOV_EXCL_LINE
-               g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
@@ -276,7 +308,6 @@ int media_packet_pool_release_packet(media_packet_pool_h pool, media_packet_h pk
 
        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;
        }
 
@@ -286,7 +317,6 @@ int media_packet_pool_release_packet(media_packet_pool_h pool, media_packet_h pk
                LOGD("The packet released to pool is  %p..\n", pkt);
        } else {
                LOGE("packet is not aquired from pool %p", pkt);        //LCOV_EXCL_LINE
-               g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
@@ -295,8 +325,6 @@ int media_packet_pool_release_packet(media_packet_pool_h pool, media_packet_h pk
        media_packet_unset_flags(pkt, MEDIA_PACKET_END_OF_STREAM);
        media_packet_unset_flags(pkt, MEDIA_PACKET_SYNC_FRAME);
 
-       g_mutex_unlock(&pool_handle->mutex);
-
        return MEDIA_PACKET_ERROR_NONE;
 }
 
@@ -311,6 +339,13 @@ int media_packet_pool_deallocate(media_packet_pool_h pool)
        pool_handle = (media_packet_pool_s *)pool;
 
        g_mutex_lock(&pool_handle->mutex);
+
+       if (!pool_handle->pool_allocated) {
+               LOGE("pool is not allocated");
+               g_mutex_unlock(&pool_handle->mutex);
+               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
+       }
+
        num_pkts = g_queue_get_length(pool_handle->queue);
 
        if (num_pkts < pool_handle->pool_size) {
@@ -319,38 +354,34 @@ int media_packet_pool_deallocate(media_packet_pool_h pool)
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
-       g_atomic_int_set(&pool_handle->pool_allocated, 0);
+       pool_handle->pool_allocated = FALSE;
 
        while (!g_queue_is_empty(pool_handle->queue)) {
-
                packet = g_queue_pop_head(pool_handle->queue);
                if (packet == NULL) {
-                       g_mutex_unlock(&pool_handle->mutex);
-                       LOGE("Failed to get packet handle from Queue ");        //LCOV_EXCL_LINE
-                       return MEDIA_PACKET_ERROR_INVALID_OPERATION;
-               }
-               ret = media_packet_destroy(packet);
-               LOGI("The packet handle(%p) will be deallocated..", packet);
-               if (ret != MEDIA_PACKET_ERROR_NONE) {
-                       g_mutex_unlock(&pool_handle->mutex);
-                       return ret;
+                       LOGW("NULL packet from queue");         //LCOV_EXCL_LINE
+                       continue;
                }
-       }
 
-       /* unreference media_format */
-       if (media_format_unref(pool_handle->fmt_h) != MEDIA_FORMAT_ERROR_NONE) {
-               LOGE("failed to decrease ref count");           //LCOV_EXCL_LINE
                g_mutex_unlock(&pool_handle->mutex);
-               return MEDIA_PACKET_ERROR_INVALID_OPERATION;
+
+               LOGI("destroy packet[%p]", packet);
+
+               ret = media_packet_destroy(packet);
+               if (ret != MEDIA_PACKET_ERROR_NONE)
+                       LOGW("destroy packet[%p] failed[0x%x]", packet, ret);
+
+               g_mutex_lock(&pool_handle->mutex);
        }
 
        g_mutex_unlock(&pool_handle->mutex);
 
-       return ret;
+       return MEDIA_PACKET_ERROR_NONE;
 }
 
 int media_packet_pool_destroy(media_packet_pool_h pool)
 {
+       int ret = 0;
        int num_pkts = 0;
 
        media_packet_pool_s *pool_handle = NULL;
@@ -359,14 +390,24 @@ int media_packet_pool_destroy(media_packet_pool_h pool)
        pool_handle = (media_packet_pool_s *)pool;
 
        g_mutex_lock(&pool_handle->mutex);
-       num_pkts = g_queue_get_length(pool_handle->queue);
-       g_mutex_unlock(&pool_handle->mutex);
 
+       num_pkts = g_queue_get_length(pool_handle->queue);
        if (num_pkts > 0) {
-               LOGE("The packet pool needs to deallocate first ");             //LCOV_EXCL_LINE
+               LOGE("The packet pool needs to deallocate first[num_pkts:%d]", num_pkts);       //LCOV_EXCL_LINE
+               g_mutex_unlock(&pool_handle->mutex);
                return MEDIA_PACKET_ERROR_INVALID_OPERATION;
        }
 
+       if (pool_handle->fmt_h) {
+               ret = media_format_unref(pool_handle->fmt_h);
+               if (ret != MEDIA_FORMAT_ERROR_NONE)
+                       LOGW("media_format_unref() failed[0x%x]", ret);         //LCOV_EXCL_LINE
+
+               pool_handle->fmt_h = NULL;
+       }
+
+       g_mutex_unlock(&pool_handle->mutex);
+
        g_queue_free(pool_handle->queue);
        LOGI("The packet pool handle(%p) will be destroyed..", pool_handle);
        free(pool_handle);