fix mem leak

1. add packet list to check mem leak
2. add media format unref

Change-Id: I73f403825d836bbe9738ce550887f0a28ab0f3e3
Signed-off-by: Eunhae Choi <eunhae1.choi@samsung.com>

diff --git a/include/common/player.h b/include/common/player.h
index f88f0b9..3b9b7bb 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- a/src/player.c
+++ b/src/player.c
@@ -42,6 +42,8 @@
 #include "player_private.h"
 #include <mm_evas_renderer.h>
 
+#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 (file)
--- 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) {