From 601b61fdd480bf7b5b479491d2a691ee33eb17ea Mon Sep 17 00:00:00 2001 From: Jeongmo Yang Date: Tue, 4 Feb 2025 08:58:19 +0900 Subject: [PATCH] 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 --- include/media_packet_pool_private.h | 2 +- packaging/capi-media-tool.spec | 2 +- src/media_packet_pool.c | 159 +++++++++++++++++----------- 3 files changed, 102 insertions(+), 61 deletions(-) diff --git a/include/media_packet_pool_private.h b/include/media_packet_pool_private.h index b7d0dd3..4b4f481 100644 --- 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 371c945..1632ab2 100644 --- 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 6612a90..01500ed 100644 --- 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); -- 2.34.1