A minor refactoring for duplicated code 36/297636/10 accepted/tizen_8.0_unified tizen_8.0 accepted/tizen/8.0/unified/20231005.092926 accepted/tizen/unified/20230828.102408 tizen_8.0_m2_release
authorSeungbae Shin <seungbae.shin@samsung.com>
Tue, 22 Aug 2023 06:45:18 +0000 (15:45 +0900)
committerSeungbae Shin <seungbae.shin@samsung.com>
Fri, 25 Aug 2023 04:12:45 +0000 (13:12 +0900)
this also fixes _wav_play_sound_simple operation which wasn't working
since applying fd-passing feature.

[Version] 0.3.13
[Issue Type] Refactoring

Change-Id: Ic90d2571cf2f9fea99d0aed06a45a2602996574a

packaging/capi-media-wav-player.spec
src/wav_player_private.c
test/wav_player_test.c

index 961133d..09e9c05 100755 (executable)
@@ -1,6 +1,6 @@
 Name:       capi-media-wav-player
 Summary:    A wav player library in Tizen C API
-Version:    0.3.12
+Version:    0.3.13
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0
index ca131e6..98b291a 100644 (file)
@@ -41,7 +41,7 @@
 #define PA_SOUND_PLAYER_SIGNAL_EOS                     "EOS"
 
 typedef struct dbus_cb_data {
-       int handle;
+       int handle_id;
        guint subs_id;
        wav_player_playback_completed_cb cb;
        void *user_data;
@@ -59,6 +59,34 @@ static int __convert_dbus_error(const char *error_msg)
        return WAV_PLAYER_ERROR_INVALID_OPERATION;
 }
 
+static int __convert_to_absolute_path(const char *path, gchar **full_path)
+{
+       g_autofree gchar *_full_path = NULL;
+       char *cwd = NULL;
+
+       g_assert(path);
+       g_assert(full_path);
+
+       if (path[0] != '/') {
+               cwd = getcwd(NULL, 0);
+               _full_path = g_strdup_printf("%s/%s", cwd, path);
+               free(cwd);
+       } else {
+               _full_path = g_strdup(path);
+       }
+
+       if (access(_full_path, R_OK) != 0) {
+               char str_error[256];
+               strerror_r(errno, str_error, sizeof(str_error));
+               LOGE("accessing read for file[%s] error: [%s][%d]", _full_path, str_error, errno);
+               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+       }
+
+       *full_path = g_steal_pointer(&_full_path);
+
+       return WAV_PLAYER_ERROR_NONE;
+}
+
 static void __weak_notify_cb(gpointer data, GObject *where_the_object_was)
 {
        LOGD("object(%p) destroyed, data(%p)", where_the_object_was, data);
@@ -71,6 +99,7 @@ static GDBusConnection *__get_dbus_connection(void)
 
        conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &err);
        if (!conn) {
+               g_assert(err);
                LOGE("g_bus_get_sync() error (%s)", err->message);
                return NULL;
        }
@@ -89,7 +118,7 @@ static void __internal_complete_cb(GDBusConnection *connection,
                                        GVariant *params,
                                        gpointer userdata)
 {
-       gint handle = -1;
+       gint handle_id = -1;
        gboolean stopped_by_user = FALSE;
        dbus_cb_data_s *dbus_data = (dbus_cb_data_s *)userdata;
 
@@ -98,196 +127,231 @@ static void __internal_complete_cb(GDBusConnection *connection,
                return;
        }
 
-       g_variant_get(params, "(ib)", &handle, &stopped_by_user);
-       LOGI("Incoming/expected handle(%d/%d), id(%u), callback(%p), stopped_by_user(%d)",
-               handle, dbus_data->handle, dbus_data->subs_id, dbus_data->cb, stopped_by_user);
+       g_variant_get(params, "(ib)", &handle_id, &stopped_by_user);
+       LOGI("Incoming/expected handle_id(%d/%d), id(%u), callback(%p), stopped_by_user(%d)",
+               handle_id, dbus_data->handle_id, dbus_data->subs_id, dbus_data->cb, stopped_by_user);
 
-       if (handle != dbus_data->handle)
+       if (handle_id != dbus_data->handle_id)
                return;
 
        if (!stopped_by_user) {
-               LOGI("Invoking user callback(%p) for handle(%d)", dbus_data->cb, handle);
-               dbus_data->cb(handle, dbus_data->user_data);
+               LOGI("Invoking user callback(%p) for handle_id(%d)", dbus_data->cb, handle_id);
+               dbus_data->cb(handle_id, dbus_data->user_data);
+               LOGI("user callback returned");
        }
 
-       LOGI("user callback returned");
-
        g_dbus_connection_signal_unsubscribe(connection, dbus_data->subs_id);
        g_object_unref(connection);
 
        g_free(dbus_data);
 }
 
-int _wav_play_sound(const char *path, sound_stream_info_h stream_info, unsigned loop_count,
-                       wav_player_playback_completed_cb callback, void *user_data, bool stop_others, int *id)
+static int __get_sound_manager_stream_properties(sound_stream_info_h stream_info, char **stream_type, int *stream_id)
 {
-       int ret = WAV_PLAYER_ERROR_NONE;
-       char m_path[PATH_MAX];
-       char *stream_type = NULL;
-       int stream_id;
+       int ret = SOUND_MANAGER_ERROR_NONE;
        bool result = false;
 
-       int handle;
-       g_autoptr(GDBusConnection) conn = NULL;
-       g_autoptr(GError) err = NULL;
-       g_autoptr(GVariant) reply = NULL;
-       dbus_cb_data_s *dbus_cb_data = NULL;
-
-       g_autoptr(GUnixFDList) fd_list = NULL;
-       int fd = -1;
-       int appended_index = -1;
-
-       if (path == NULL || stream_info == NULL) {
-               LOGE("invalid params");
-               return WAV_PLAYER_ERROR_INVALID_PARAMETER;
-       }
-
-       LOGI("path(%s), loop(%u), cb(%p) user_data(%p)", path, loop_count, callback, user_data);
-
-       m_path[0] = '\0';
-       if (path[0] != '/' && getcwd(m_path, PATH_MAX) != NULL)
-               strncat(m_path, "/", PATH_MAX - strlen(m_path) - 1);
+       g_assert(stream_info);
+       g_assert(stream_type);
+       g_assert(stream_id);
 
-       strncat(m_path, path, PATH_MAX - strlen(m_path) - 1);
-       if (access(m_path, R_OK) != 0) {
-               char str_error[256];
-               strerror_r(errno, str_error, sizeof(str_error));
-               LOGE("file [%s] doesn't exists : [%s][%d]", m_path, str_error, errno);
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+       if ((ret = sound_manager_is_available_stream_information(stream_info, NATIVE_API_WAV_PLAYER, &result))) {
+               LOGE("can't check available. ret(0x%x)", ret); //LCOV_EXCL_LINE
+               return WAV_PLAYER_ERROR_INVALID_OPERATION; //LCOV_EXCL_LINE
        }
 
-       ret = sound_manager_is_available_stream_information(stream_info, NATIVE_API_WAV_PLAYER, &result);
-       if (!result || ret) {
-               LOGE("stream info is not available. ret(0x%x), result(%d)", ret, result);
+       if (!result) {
+               LOGE("stream info is not available. result(%d)", result);
                return WAV_PLAYER_ERROR_NOT_SUPPORTED_TYPE;
        }
 
-       ret = sound_manager_get_type_from_stream_information(stream_info, &stream_type);
-       if (ret) {
-               LOGE("can't get stream type. ret(0x%x)", ret);
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+       if ((ret = sound_manager_get_type_from_stream_information(stream_info, stream_type))) {
+               LOGE("can't get stream type. ret(0x%x)", ret); //LCOV_EXCL_LINE
+               return WAV_PLAYER_ERROR_INVALID_OPERATION; //LCOV_EXCL_LINE
        }
 
-       ret = sound_manager_get_index_from_stream_information(stream_info, &stream_id);
-       if (ret) {
-               LOGE("can't get stream index. ret(0x%x)", ret);
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+       if ((ret = sound_manager_get_index_from_stream_information(stream_info, stream_id))) {
+               LOGE("can't get stream index. ret(0x%x)", ret); //LCOV_EXCL_LINE
+               return WAV_PLAYER_ERROR_INVALID_OPERATION; //LCOV_EXCL_LINE
        }
 
-       if (!(conn = __get_dbus_connection()))
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+       return WAV_PLAYER_ERROR_NONE;
+}
+
+static int __get_fd_passing_list(const char *full_path, int *fd, GUnixFDList** fd_list)
+{
+       int _fd = -1;
+       g_autoptr(GUnixFDList) _fd_list = NULL;
+       g_autofree gchar *_full_path = NULL;
+       g_autoptr(GError) err = NULL;
 
-       fd = open(m_path, O_RDONLY);
-       if (fd == -1) {
-               LOGE("error opening(%s), errno(%d)", m_path, errno);
+       g_assert(full_path);
+       g_assert(fd);
+       g_assert(fd_list);
+
+       _fd = open(full_path, O_RDONLY);
+       if (_fd == -1) {
+               LOGE("error opening(%s), errno(%d)", full_path, errno);
                return WAV_PLAYER_ERROR_INVALID_OPERATION;
        }
 
-       LOGI("fd(%d) opened for fd-passing", fd);
+       LOGI("fd(%d) opened for fd-passing", _fd);
 
-       fd_list = g_unix_fd_list_new();
-       appended_index = g_unix_fd_list_append(fd_list, fd, &err);
-       if (appended_index == -1) {
+       _fd_list = g_unix_fd_list_new();
+       g_assert(_fd_list);
+       if (g_unix_fd_list_append(_fd_list, _fd, &err) != 0) {
+               g_assert(err);
                LOGE("failed, %s", err->message);
-               close(fd);
+               close(_fd);
                return WAV_PLAYER_ERROR_INVALID_OPERATION;
        }
 
+       *fd = _fd;
+       *fd_list = g_steal_pointer(&_fd_list);
+
+       return WAV_PLAYER_ERROR_NONE;
+}
+
+static int __dbus_play_sound_fd_list(GDBusConnection *conn,
+                                                                       const char *path, int loop_count,
+                                                                       const char *stream_type, int stream_id,
+                                                                       gboolean stop_others, int *id)
+{
+       int fd = -1;
+       int handle_id = -1;
+       int ret = WAV_PLAYER_ERROR_NONE;
+       g_autoptr(GError) err = NULL;
+       g_autoptr(GVariant) reply = NULL;
+       g_autoptr(GUnixFDList) fd_list = NULL;
+       g_autofree gchar *full_path = NULL;
+
+       g_assert(conn);
+       g_assert(path);
+       g_assert(stream_type);
+
+       if ((ret = __convert_to_absolute_path(path, &full_path)))
+               return ret;
+
+       if ((ret = __get_fd_passing_list(full_path, &fd, &fd_list)))
+               return ret;
+
        /* Note: Sending appended index to fd-list is required
                        for receiving fd-passing using non-gdbus implementation */
        reply = g_dbus_connection_call_with_unix_fd_list_sync(conn, PA_BUS_NAME,
-                       PA_SOUND_PLAYER_OBJECT_PATH,
-                       PA_SOUND_PLAYER_INTERFACE,
-                       PA_SOUND_PLAYER_METHOD_NAME_SOUND_PLAY,
-                       g_variant_new("(siisibh)", m_path, loop_count == 0 ? -1 : loop_count,
-                                       getpid(), stream_type, stream_id, (gboolean)stop_others, appended_index),
-                       NULL, G_DBUS_CALL_FLAGS_NONE, -1, fd_list, NULL, NULL, &err);
+                               PA_SOUND_PLAYER_OBJECT_PATH,
+                               PA_SOUND_PLAYER_INTERFACE,
+                               PA_SOUND_PLAYER_METHOD_NAME_SOUND_PLAY,
+                               g_variant_new("(siisibh)", full_path, loop_count == 0 ? -1 : loop_count,
+                                                       getpid(), stream_type, stream_id, (gboolean)stop_others, 0),
+                               NULL, G_DBUS_CALL_FLAGS_NONE, -1, fd_list, NULL, NULL, &err);
+       close(fd);
        if (!reply) {
+               g_assert(err);
                LOGE("g_dbus_connection_call_with_unix_fd_list_sync error (%s)", err->message);
                return __convert_dbus_error(err->message);
        }
 
-       close(fd);
-
-       g_variant_get(reply, "(i)", &handle);
+       g_variant_get(reply, "(i)", &handle_id);
        if (id)
-               *id = handle;
+               *id = handle_id;
 
-       LOGI("handle : %d", handle);
+       LOGI("handle_id(%d)", handle_id);
 
-       if (callback) {
-               dbus_cb_data = g_new0(dbus_cb_data_s, 1);
-               dbus_cb_data->handle = handle;
-               dbus_cb_data->cb = callback;
-               dbus_cb_data->user_data = user_data;
-               dbus_cb_data->subs_id = g_dbus_connection_signal_subscribe(
-                               g_object_ref(conn), NULL,
-                               PA_SOUND_PLAYER_INTERFACE,
-                               PA_SOUND_PLAYER_SIGNAL_EOS,
-                               PA_SOUND_PLAYER_OBJECT_PATH,
-                               NULL, G_DBUS_SIGNAL_FLAGS_NONE,
-                               __internal_complete_cb, dbus_cb_data, NULL);
-               if (dbus_cb_data->subs_id == 0) {
-                       g_object_unref(conn);
-                       g_free(dbus_cb_data);
-                       LOGE("g_dbus_connection_signal_subscribe() failed");
-                       return WAV_PLAYER_ERROR_INVALID_OPERATION;
-               }
+       return WAV_PLAYER_ERROR_NONE;
+}
+
+static int __dbus_subscribe_complete_cb(GDBusConnection *conn, int id,
+                                                                               wav_player_playback_completed_cb callback, void *user_data)
+{
+       dbus_cb_data_s *dbus_cb_data = NULL;
+
+       g_assert(conn);
+       g_assert(id >= 0);
+       g_assert(callback);
+
+       dbus_cb_data = g_new0(dbus_cb_data_s, 1);
+       dbus_cb_data->handle_id = id;
+       dbus_cb_data->cb = callback;
+       dbus_cb_data->user_data = user_data;
+       dbus_cb_data->subs_id = g_dbus_connection_signal_subscribe(
+                       g_object_ref(conn), NULL,
+                       PA_SOUND_PLAYER_INTERFACE,
+                       PA_SOUND_PLAYER_SIGNAL_EOS,
+                       PA_SOUND_PLAYER_OBJECT_PATH,
+                       NULL, G_DBUS_SIGNAL_FLAGS_NONE,
+                       __internal_complete_cb, dbus_cb_data, NULL);
+       if (dbus_cb_data->subs_id == 0) {
+               g_object_unref(conn);
+               g_free(dbus_cb_data);
+               LOGE("g_dbus_connection_signal_subscribe() failed");
+               return WAV_PLAYER_ERROR_INVALID_OPERATION;
        }
 
        return WAV_PLAYER_ERROR_NONE;
 }
 
+int _wav_play_sound(const char *path, sound_stream_info_h stream_info, unsigned loop_count,
+                                       wav_player_playback_completed_cb callback, void *user_data, bool stop_others, int *id)
+{
+       int ret = WAV_PLAYER_ERROR_NONE;
+       char *stream_type = NULL;
+       int stream_id = -1;
+       int handle_id = -1;
+       g_autoptr(GDBusConnection) conn = NULL;
+
+       if (!path || !stream_info) {
+               LOGE("invalid path(%p) or stream_info(%p)", path, stream_info);
+               return WAV_PLAYER_ERROR_INVALID_PARAMETER;
+       }
+
+       LOGI("path(%s), loop(%u), cb(%p) user_data(%p)", path, loop_count, callback, user_data);
+
+       if ((ret = __get_sound_manager_stream_properties(stream_info, &stream_type, &stream_id)))
+               return ret;
+
+       if (!(conn = __get_dbus_connection()))
+               return WAV_PLAYER_ERROR_INVALID_OPERATION;
+
+       if ((ret = __dbus_play_sound_fd_list(conn, path, loop_count == 0 ? -1 : loop_count,
+                                                                               stream_type, stream_id,
+                                                                               stop_others, &handle_id)))
+               return ret;
+
+       if (callback && (ret = __dbus_subscribe_complete_cb(conn, handle_id, callback, user_data)))
+               return ret;
+
+       if (id)
+               *id = handle_id;
+
+       return WAV_PLAYER_ERROR_NONE;
+}
+
 //LCOV_EXCL_START
 int _wav_play_sound_simple(const char *path, const char *stream_role)
 {
-       char m_path[PATH_MAX];
-       int handle;
+       int ret = WAV_PLAYER_ERROR_NONE;
        g_autoptr(GDBusConnection) conn = NULL;
        g_autoptr(GError) err = NULL;
        g_autoptr(GVariant) reply = NULL;
 
        if (!path || !stream_role) {
-               LOGE("invalid params path(%p), stream_role(%p)", path, stream_role);
+               LOGE("invalid path(%p) or stream_role(%p)", path, stream_role);
                return WAV_PLAYER_ERROR_INVALID_PARAMETER;
        }
 
        LOGI("path(%s), stream_role(%s)", path, stream_role);
 
-       /* FIXME : need extraction of duplicated path validation code */
-       m_path[0] = '\0';
-       if (path[0] != '/' && getcwd(m_path, PATH_MAX) != NULL)
-               strncat(m_path, "/", PATH_MAX - strlen(m_path) - 1);
-
-       strncat(m_path, path, PATH_MAX - strlen(m_path) - 1);
-       if (access(m_path, R_OK) != 0) {
-               char str_error[256];
-               strerror_r(errno, str_error, sizeof(str_error));
-               LOGE("file [%s] doesn't exists : [%s][%d]", m_path, str_error, errno);
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
-       }
+       /* simple: no stream properties from stream info */
 
        if (!(conn = __get_dbus_connection()))
                return WAV_PLAYER_ERROR_INVALID_OPERATION;
 
-       reply = g_dbus_connection_call_sync(conn, PA_BUS_NAME,
-                                                                               PA_SOUND_PLAYER_OBJECT_PATH,
-                                                                               PA_SOUND_PLAYER_INTERFACE,
-                                                                               PA_SOUND_PLAYER_METHOD_NAME_SOUND_PLAY,
-                                                                               g_variant_new("(siisib)", m_path, 1,
-                                                                                                         getpid(), stream_role, -1, FALSE),
-                                                                               NULL, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &err);
-       if (!reply) {
-               if (err) {
-                       LOGE("g_dbus_connection_call_sync error (%s)", err->message);
-                       return __convert_dbus_error(err->message);
-               }
-               return WAV_PLAYER_ERROR_INVALID_OPERATION;
-       }
-
-       g_variant_get(reply, "(i)", &handle);
+       if ((ret = __dbus_play_sound_fd_list(conn, path, 1,
+                                                                               stream_role, -1,
+                                                                               FALSE, NULL)))
+               return ret;
 
-       LOGI("handle : %d", handle);
+       /* simple: no user complete callback */
 
        return WAV_PLAYER_ERROR_NONE;
 }
@@ -299,7 +363,7 @@ int _wav_stop_sound(int id)
        g_autoptr(GError) err = NULL;
        g_autoptr(GVariant) reply = NULL;
 
-       LOGI("handle(%d)", id);
+       LOGI("stopping: handle_id(%d)", id);
 
        if (!(conn = __get_dbus_connection()))
                return WAV_PLAYER_ERROR_INVALID_OPERATION;
@@ -312,11 +376,12 @@ int _wav_stop_sound(int id)
                                g_variant_new("(i)", id),
                                NULL, G_DBUS_CALL_FLAGS_NONE, -1, NULL, &err);
        if (!reply) {
+               g_assert(err);
                LOGE("g_dbus_connection_call_sync error (%s)", err->message);
                return __convert_dbus_error(err->message);
        }
 
-       LOGI("stop sound. handle(%d)", id);
+       LOGI("stopped: handle_id(%d)", id);
 
        return WAV_PLAYER_ERROR_NONE;
 }
index 6a0c0d0..93b51bb 100644 (file)
@@ -48,10 +48,10 @@ void help(void)
                   "                               7: ringtone voip\n"
                   "                               8: voip\n"
                   "                               9: ringtone voip\n"
-                  "                             107: solo (internal)\n"
+                  "                             107: (internal) solo\n"
                   "  -i, --iterate             Number of times to play (default:1)\n"
-                  "  -q, --quickplay           Play quick\n"
-                  "  -o, --stopothers          Play after stopping others\n"
+                  "  -q, --quickplay           (internal) Play quickly without stream info creation\n"
+                  "  -o, --stopothers          (internal) Play after stopping others\n"
                   "  -h, --help                Help\n");
 }
 
@@ -122,6 +122,9 @@ int wav_play_test(const char *file_path, int iterate, int stream_type, bool stop
        if (stop_others) {
                ret = wav_player_start_loop_stop_others(file_path, stream_info, iterate, __player_stop_cb, (void*)stream_info, &gid);
                g_message("wav_player_start_loop_stop_others(id=%d) ret = %d", gid, ret);
+       } else if (iterate == 1) {
+               ret = wav_player_start_new(file_path, stream_info, __player_stop_cb, (void*)stream_info, &gid);
+               g_message("wav_player_start_new(id=%d) ret = %d", gid, ret);
        } else {
                ret = wav_player_start_loop(file_path, stream_info, iterate, __player_stop_cb, (void*)stream_info, &gid);
                g_message("wav_player_start_loop(id=%d) ret = %d", gid, ret);