From: Eunhae Choi Date: Mon, 25 Apr 2016 09:54:17 +0000 (+0900) Subject: fix mem leak X-Git-Tag: accepted/tizen/common/20160429.170707^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=08ec639d3dc404ca52b2543ad5a4f861ef703939;p=platform%2Fcore%2Fapi%2Fplayer.git fix mem leak 1. add packet list to check mem leak 2. add media format unref Change-Id: I73f403825d836bbe9738ce550887f0a28ab0f3e3 Signed-off-by: Eunhae Choi --- diff --git a/include/common/player.h b/include/common/player.h index f88f0b9..3b9b7bb 100644 --- a/include/common/player.h +++ b/include/common/player.h @@ -882,11 +882,13 @@ int player_set_display(player_h player, player_display_type_e type, player_displ * @brief Registers a media packet video callback function to be called once per frame. * @since_tizen 2.3 * @remarks This function should be called before preparing. \n - * A registered callback is called on the internal thread of the player. \n + * A registered callback is called on the internal thread of the player. \n * A video frame can be retrieved using a registered callback as a media packet.\n * The callback function holds the same buffer that will be drawn on the display device.\n * So if you change the media packet in a registerd callback, it will be displayed on the device\n - * and the media packet is available until it's destroyed by media_packet_destroy(). + * and the media packet is available until it's destroyed by media_packet_destroy().\n + * The packet have to be destroyed as quickly as possible after rendering the packet\n + * and all the packets have to be destoryed before player_unprepare() is called.\n * @param[in] player The handle to the media player * @param[in] callback The callback function to be registered * @param[in] user_data The user data to be passed to the callback function diff --git a/include/player_private.h b/include/player_private.h index 57f41da..3fd1159 100644 --- a/include/player_private.h +++ b/include/player_private.h @@ -84,10 +84,12 @@ typedef struct _callback_cb_info { gpointer user_data[MUSE_PLAYER_EVENT_TYPE_NUM]; GMutex player_mutex; GCond player_cond[MUSE_PLAYER_API_MAX]; + GList *packet_list; + GMutex packet_list_mutex; msg_buff_s buff; player_event_queue event_queue; media_format_h pkt_fmt; - MMHandleType local_handle; + void *evas_info; tbm_bufmgr bufmgr; } callback_cb_info_s; @@ -105,10 +107,7 @@ typedef struct _player_cli_s { gboolean have_evas_callback; } player_cli_s; -/* Internal handle */ -#define INT_HANDLE(h) ((h)->cb_info->local_handle) - -/* player callback infomatnio */ +/* player callback information */ #define CALLBACK_INFO(h) ((h)->cb_info) /* MSG Channel socket fd */ #define MSG_FD(h) (CALLBACK_INFO(h)->fd) @@ -116,6 +115,8 @@ typedef struct _player_cli_s { #define DATA_FD(h) (CALLBACK_INFO(h)->data_fd) /* TBM buffer manager */ #define TBM_BUFMGR(h) (CALLBACK_INFO(h)->bufmgr) +/* evas display handle */ +#define EVAS_HANDLE(h) ((h)->cb_info->evas_info) /* server tbm bo */ #define SERVER_TBM_BO(h) ((h)->server.bo) diff --git a/include/wearable/player.h b/include/wearable/player.h index b90a1b5..dc24104 100644 --- a/include/wearable/player.h +++ b/include/wearable/player.h @@ -880,11 +880,13 @@ int player_set_display(player_h player, player_display_type_e type, player_displ * @brief Registers a media packet video callback function to be called once per frame. * @since_tizen 2.3.1 * @remarks This function should be called before preparing. \n - * A registered callback is called on the internal thread of the player. \n + * A registered callback is called on the internal thread of the player. \n * A video frame can be retrieved using a registered callback as a media packet.\n * The callback function holds the same buffer that will be drawn on the display device.\n * So if you change the media packet in a registerd callback, it will be displayed on the device\n - * and the media packet is available until it's destroyed by media_packet_destroy(). + * and the media packet is available until it's destroyed by media_packet_destroy().\n + * The packet have to be destroyed as quickly as possible after rendering the packet\n + * and all the packets have to be destoryed before player_unprepare() is called.\n * @param[in] player The handle to the media player * @param[in] callback The callback function to be registered * @param[in] user_data The user data to be passed to the callback function diff --git a/packaging/capi-media-player.spec b/packaging/capi-media-player.spec index e10ba67..4cbb426 100644 --- a/packaging/capi-media-player.spec +++ b/packaging/capi-media-player.spec @@ -3,7 +3,7 @@ Name: capi-media-player Summary: A Media Player API -Version: 0.3.5 +Version: 0.3.6 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/player.c b/src/player.c index aba0b27..bcadd81 100644 --- a/src/player.c +++ b/src/player.c @@ -42,6 +42,8 @@ #include "player_private.h" #include +#define INVALID_SOCKET -1 + typedef struct { int int_data; char *buf; @@ -50,7 +52,8 @@ typedef struct { typedef struct { intptr_t remote_pkt; - callback_cb_info_s *cb_info; + callback_cb_info_s *cb_info; /* to monitor the packet_list */ + gint fd; } _media_pkt_fin_data; /* @@ -64,22 +67,23 @@ static int _player_deinit_memory_buffer(player_cli_s * pc); int _player_media_packet_finalize(media_packet_h pkt, int error_code, void *user_data) { - int ret = 0; + int ret = MEDIA_PACKET_FINALIZE; tbm_surface_h tsurf = NULL; muse_player_api_e api = MUSE_PLAYER_API_MEDIA_PACKET_FINALIZE_CB; _media_pkt_fin_data *fin_data = (_media_pkt_fin_data *) user_data; intptr_t packet; - char *sndMsg; + char *snd_msg = NULL; + int snd_len = 0; - if (pkt == NULL || user_data == NULL) { + if (!pkt) { LOGE("invalid parameter buffer %p, user_data %p", pkt, user_data); - return MEDIA_PACKET_FINALIZE; + return ret; } ret = media_packet_get_tbm_surface(pkt, &tsurf); if (ret != MEDIA_PACKET_ERROR_NONE) { LOGE("media_packet_get_tbm_surface failed 0x%x", ret); - return MEDIA_PACKET_FINALIZE; + goto EXIT; } if (tsurf) { @@ -87,14 +91,44 @@ int _player_media_packet_finalize(media_packet_h pkt, int error_code, void *user tsurf = NULL; } + if (!fin_data || fin_data->fd <= INVALID_SOCKET) { + LOGE("invalid parameter, fd: %d", (fin_data) ? (fin_data->fd) : (-1)); + goto EXIT; + } + +#if 1 /* FIXME : after applying media_packet_pool, no need to check packet_list manully. */ packet = fin_data->remote_pkt; - sndMsg = muse_core_msg_json_factory_new(api, MUSE_TYPE_POINTER, "packet", packet, 0); - muse_core_ipc_send_msg(fin_data->cb_info->fd, sndMsg); - muse_core_msg_json_factory_free(sndMsg); + snd_msg = muse_core_msg_json_factory_new(api, MUSE_TYPE_POINTER, "packet", packet, 0); + snd_len = muse_core_ipc_send_msg(fin_data->fd, snd_msg); + muse_core_msg_json_factory_free(snd_msg); + + if (snd_len > 0) { + /* player handle is available. remove packet from the list */ + if (fin_data->cb_info->packet_list) { + g_mutex_lock(&fin_data->cb_info->packet_list_mutex); + if (g_list_find(fin_data->cb_info->packet_list, pkt)) { + fin_data->cb_info->packet_list = g_list_remove(fin_data->cb_info->packet_list, pkt); + } else { + LOGW("there is no packet(%p) in exported list.", pkt); + } + g_mutex_unlock(&fin_data->cb_info->packet_list_mutex); + } else { + LOGW("there is no packet list.", pkt); + } + } else { + LOGE("fail to send msg."); + } +#else + player_msg_send1_no_return(api, fin_data->cb_info->fd, POINTER, packet); +#endif - g_free(fin_data); +EXIT: + if (fin_data) { + g_free(fin_data); + fin_data = NULL; + } - return MEDIA_PACKET_FINALIZE; + return ret; } static int __player_convert_error_code(int code, char *func_name) @@ -478,6 +512,18 @@ static void __media_packet_video_frame_cb_handler(callback_cb_info_s * cb_info, LOGD("width %d, height %d", sinfo.width, sinfo.height); + if (!cb_info) { + LOGE("cb_info is null"); + return; + } + + if (!cb_info->user_cb[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]) { + /* send msg to release packet. */ + LOGE("_video_decoded_cb is not set"); + player_msg_send1_no_return(MUSE_PLAYER_API_MEDIA_PACKET_FINALIZE_CB, cb_info->fd, POINTER, packet); + return; + } + for (i = 0; i < 4; i++) { if (key[i]) { bo_num++; @@ -520,22 +566,35 @@ static void __media_packet_video_frame_cb_handler(callback_cb_info_s * cb_info, fin_data = g_new(_media_pkt_fin_data, 1); fin_data->remote_pkt = packet; fin_data->cb_info = cb_info; + fin_data->fd = cb_info->fd; ret = media_packet_create_from_tbm_surface(cb_info->pkt_fmt, tsurf, (media_packet_finalize_cb) _player_media_packet_finalize, (void *)fin_data, &pkt); if (ret != MEDIA_PACKET_ERROR_NONE) { LOGE("media_packet_create_from_tbm_surface failed"); tbm_surface_destroy(tsurf); tsurf = NULL; } + + /* keep the media packet to avoid mem leak. */ + g_mutex_lock(&cb_info->packet_list_mutex); + cb_info->packet_list = g_list_append(cb_info->packet_list, pkt); + g_mutex_unlock(&cb_info->packet_list_mutex); } + if (pkt) { if (pts != 0) { ret = media_packet_set_pts(pkt, (uint64_t) pts); if (ret != MEDIA_PACKET_ERROR_NONE) LOGE("media_packet_set_pts failed"); } - /* call media packet callback */ - ((player_media_packet_video_decoded_cb) cb_info->user_cb[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]) (pkt, cb_info->user_data[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]); + if (cb_info->user_cb[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]) { + /* call media packet callback */ + ((player_media_packet_video_decoded_cb) cb_info->user_cb[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]) (pkt, cb_info->user_data[MUSE_PLAYER_EVENT_TYPE_MEDIA_PACKET_VIDEO_FRAME]); + } else { + LOGE("_video_decoded_cb is not set"); + media_packet_destroy(pkt); + } } + for (i = 0; i < bo_num; i++) { if (bo[i]) tbm_bo_unref(bo[i]); @@ -984,6 +1043,8 @@ static callback_cb_info_s *callback_new(gint sockfd) for (i = 0; i < MUSE_PLAYER_API_MAX; i++) g_cond_init(&cb_info->player_cond[i]); + g_mutex_init(&cb_info->packet_list_mutex); + buff = &cb_info->buff; buff->recvMsg = g_new(char, MUSE_MSG_MAX_LENGTH + 1); buff->bufLen = MUSE_MSG_MAX_LENGTH + 1; @@ -1002,11 +1063,15 @@ static void callback_destroy(callback_cb_info_s * cb_info) int i; g_return_if_fail(cb_info); - muse_core_connection_close(cb_info->fd); - muse_core_connection_close(cb_info->data_fd); + if (cb_info->fd > INVALID_SOCKET) + muse_core_connection_close(cb_info->fd); + if (cb_info->data_fd > INVALID_SOCKET) + muse_core_connection_close(cb_info->data_fd); + cb_info->fd = cb_info->data_fd = INVALID_SOCKET; g_thread_join(cb_info->thread); g_thread_unref(cb_info->thread); + cb_info->thread = NULL; LOGI("%p Callback destroyed", cb_info->thread); @@ -1014,6 +1079,8 @@ static void callback_destroy(callback_cb_info_s * cb_info) for (i = 0; i < MUSE_PLAYER_API_MAX; i++) g_cond_clear(&cb_info->player_cond[i]); + g_mutex_clear(&cb_info->packet_list_mutex); + g_free(cb_info->buff.recvMsg); g_free(cb_info); } @@ -1085,7 +1152,7 @@ int player_create(player_h * player) PLAYER_INSTANCE_CHECK(player); int ret = PLAYER_ERROR_NONE; - int sock_fd = -1; + int sock_fd = INVALID_SOCKET; int pid = getpid(); muse_player_api_e api = MUSE_PLAYER_API_CREATE; @@ -1096,7 +1163,7 @@ int player_create(player_h * player) LOGD("ENTER"); sock_fd = muse_core_client_new(); - if (sock_fd < 0) { + if (sock_fd <= INVALID_SOCKET) { LOGE("connection failure %d", errno); ret = PLAYER_ERROR_INVALID_OPERATION; goto ERROR; @@ -1165,6 +1232,15 @@ int player_destroy(player_h player) player_msg_send(api, pc, ret_buf, ret); + if (pc->cb_info && pc->cb_info->packet_list) { + g_mutex_lock(&pc->cb_info->packet_list_mutex); + LOGW("num of remained packets : %d !!", g_list_length(pc->cb_info->packet_list)); + /* each media packet have to destroyed in application. */ + g_list_free(pc->cb_info->packet_list); + pc->cb_info->packet_list = NULL; + g_mutex_unlock(&pc->cb_info->packet_list_mutex); + } + if (player_unset_evas_object_cb(player) != MM_ERROR_NONE) LOGW("fail to unset evas object callback"); @@ -1248,8 +1324,9 @@ int player_unprepare(player_h player) if (!CALLBACK_INFO(pc)) return PLAYER_ERROR_INVALID_STATE; - if (mm_evas_renderer_destroy(&INT_HANDLE(pc)) != MM_ERROR_NONE) + if (mm_evas_renderer_destroy(&EVAS_HANDLE(pc)) != MM_ERROR_NONE) { LOGW("fail to unset evas client"); + } player_msg_send(api, pc, ret_buf, ret); if (ret == PLAYER_ERROR_NONE) { @@ -1518,8 +1595,8 @@ int player_start(player_h player) LOGD("ENTER"); - if (INT_HANDLE(pc)) { - ret = mm_evas_renderer_update_param(INT_HANDLE(pc)); + if (EVAS_HANDLE(pc)) { + ret = mm_evas_renderer_update_param(EVAS_HANDLE(pc)); if (ret != PLAYER_ERROR_NONE) return ret; } @@ -1795,15 +1872,15 @@ int player_set_display(player_h player, player_display_type_e type, player_displ evas_object_geometry_get(obj, &wl_win.wl_window_x, &wl_win.wl_window_y, &wl_win.wl_window_width, &wl_win.wl_window_height); - if (INT_HANDLE(pc)) { + if (EVAS_HANDLE(pc)) { LOGW("evas client already exists"); - if (mm_evas_renderer_destroy(&INT_HANDLE(pc)) != MM_ERROR_NONE) + if (mm_evas_renderer_destroy(&EVAS_HANDLE(pc)) != MM_ERROR_NONE) LOGW("fail to unset evas client"); } - if (mm_evas_renderer_create(&INT_HANDLE(pc), obj) != MM_ERROR_NONE) { + if (mm_evas_renderer_create(&EVAS_HANDLE(pc), obj) != MM_ERROR_NONE) { LOGW("fail to set evas client"); } - if (player_set_media_packet_video_frame_decoded_cb(player, mm_evas_renderer_write, (void *)INT_HANDLE(pc)) != PLAYER_ERROR_NONE) { + if (player_set_media_packet_video_frame_decoded_cb(player, mm_evas_renderer_write, (void *)EVAS_HANDLE(pc)) != PLAYER_ERROR_NONE) { LOGW("fail to set decoded callback"); } } @@ -1841,7 +1918,7 @@ int player_set_display_mode(player_h player, player_display_mode_e mode) LOGD("ENTER"); - ret = mm_evas_renderer_set_geometry(INT_HANDLE(pc), mode); + ret = mm_evas_renderer_set_geometry(EVAS_HANDLE(pc), mode); /* FIXME : devide server and client and consider which error code will be returned */ if (ret == PLAYER_ERROR_NONE) { return ret; @@ -1864,7 +1941,7 @@ int player_get_display_mode(player_h player, player_display_mode_e * pmode) LOGD("ENTER"); - ret = mm_evas_renderer_get_geometry(INT_HANDLE(pc), &mode); + ret = mm_evas_renderer_get_geometry(EVAS_HANDLE(pc), &mode); if (ret == PLAYER_ERROR_NONE && mode != -1) { *pmode = (player_display_mode_e) mode; return ret; @@ -1906,7 +1983,7 @@ int player_set_display_rotation(player_h player, player_display_rotation_e rotat LOGD("ENTER"); - ret = mm_evas_renderer_set_rotation(INT_HANDLE(pc), rotation); + ret = mm_evas_renderer_set_rotation(EVAS_HANDLE(pc), rotation); if (ret == PLAYER_ERROR_NONE) { return ret; } @@ -1928,7 +2005,7 @@ int player_get_display_rotation(player_h player, player_display_rotation_e * pro LOGD("ENTER"); - ret = mm_evas_renderer_get_rotation(INT_HANDLE(pc), &rotation); + ret = mm_evas_renderer_get_rotation(EVAS_HANDLE(pc), &rotation); if (ret == PLAYER_ERROR_NONE && rotation != -1) { *protation = (player_display_rotation_e) rotation; return ret; @@ -1954,7 +2031,7 @@ int player_set_display_visible(player_h player, bool visible) LOGD("ENTER"); - ret = mm_evas_renderer_set_visible(INT_HANDLE(pc), visible); + ret = mm_evas_renderer_set_visible(EVAS_HANDLE(pc), visible); if (ret == PLAYER_ERROR_NONE) { return ret; } @@ -1977,7 +2054,7 @@ int player_is_display_visible(player_h player, bool * pvisible) LOGD("ENTER"); - ret = mm_evas_renderer_get_visible(INT_HANDLE(pc), &visible); + ret = mm_evas_renderer_get_visible(EVAS_HANDLE(pc), &visible); if (ret == PLAYER_ERROR_NONE) { if (visible) *pvisible = TRUE; @@ -2884,6 +2961,8 @@ int player_push_media_stream(player_h player, media_packet_h packet) else if (is_audio) media_format_get_audio_info(format, &push_media.mimetype, NULL, NULL, NULL, NULL); + media_format_unref(format); + #ifdef __UN_USED if (push_media.buf_type == PUSH_MEDIA_BUF_TYPE_TBM) { bo = tbm_bo_alloc(pc->cb_info->bufmgr, push_media.size, TBM_BO_DEFAULT); diff --git a/test/player_media_packet_test.c b/test/player_media_packet_test.c index 765f78a..3fa5553 100644 --- a/test/player_media_packet_test.c +++ b/test/player_media_packet_test.c @@ -226,6 +226,7 @@ static void pipe_cb(void *data, void *buf, unsigned int len) media_packet_get_format(ad->packet, &format); media_format_get_video_info(format, &mimetype, NULL, NULL, NULL, NULL); + media_format_unref(format); if (mimetype == MEDIA_FORMAT_I420 || mimetype == MEDIA_FORMAT_NV12 || mimetype == MEDIA_FORMAT_NV12T) {