From: Seungbae Shin Date: Tue, 22 Aug 2023 06:45:18 +0000 (+0900) Subject: A minor refactoring for duplicated code X-Git-Tag: accepted/tizen/8.0/unified/20231005.092926^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=refs%2Fheads%2Ftizen_8.0;p=platform%2Fcore%2Fapi%2Fwav-player.git A minor refactoring for duplicated code 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 --- diff --git a/packaging/capi-media-wav-player.spec b/packaging/capi-media-wav-player.spec index 961133d..09e9c05 100755 --- a/packaging/capi-media-wav-player.spec +++ b/packaging/capi-media-wav-player.spec @@ -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 diff --git a/src/wav_player_private.c b/src/wav_player_private.c index ca131e6..98b291a 100644 --- a/src/wav_player_private.c +++ b/src/wav_player_private.c @@ -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; } diff --git a/test/wav_player_test.c b/test/wav_player_test.c index 6a0c0d0..93b51bb 100644 --- a/test/wav_player_test.c +++ b/test/wav_player_test.c @@ -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);