From 26c91d0527728398b84e6d4bcb486e57d3ee296a Mon Sep 17 00:00:00 2001 From: Muthu Shiam Sankar N Date: Thu, 18 Dec 2025 16:44:43 +0530 Subject: [PATCH 1/4] To clear Coverity warnings Summary: To clear Coverity warnings Type: Fix Test Plan: UT/CT, Fullstack Jira: ENTDAI-1354/RDKECOREMW-947 --- common/source/LinuxUtils.cpp | 34 ++++++++++++++----- ipc/common/source/NamedSocket.cpp | 34 ++++++++++++++----- media/client/main/source/MediaPipeline.cpp | 2 +- media/public/include/IMediaPipeline.h | 8 ++--- .../server/gstplayer/source/GstDecryptor.cpp | 2 +- .../gstplayer/source/GstGenericPlayer.cpp | 10 ++++-- media/server/gstplayer/source/GstSrc.cpp | 2 ++ .../ipc/source/MediaPipelineModuleService.cpp | 6 ++-- media/server/main/interface/IMainThread.h | 1 - media/server/main/source/ActiveRequests.cpp | 2 ++ media/server/main/source/NeedMediaData.cpp | 2 +- .../common/source/SessionServerAppManager.cpp | 4 +-- wrappers/source/OcdmSession.cpp | 2 +- 13 files changed, 76 insertions(+), 33 deletions(-) diff --git a/common/source/LinuxUtils.cpp b/common/source/LinuxUtils.cpp index b8906830a..a4a40eee1 100644 --- a/common/source/LinuxUtils.cpp +++ b/common/source/LinuxUtils.cpp @@ -32,14 +32,23 @@ constexpr gid_t kNoGroupChange = -1; // -1 means chown() won't change the group uid_t getFileOwnerId(const std::string &fileOwner) { uid_t ownerId = kNoOwnerChange; - const size_t kBufferSize = sysconf(_SC_GETPW_R_SIZE_MAX); - if (!fileOwner.empty() && kBufferSize > 0) + long buffersize = sysconf(_SC_GETPW_R_SIZE_MAX); + size_t BufferSize = 0; + if (buffersize == -1) + { + RIALTO_COMMON_LOG_SYS_WARN(errno, "Invalid Buffer Size '%s'", fileOwner.c_str()); + } + if (buffersize > 0) + { + BufferSize = static_cast(buffersize); + } + if (!fileOwner.empty() && BufferSize > 0) { errno = 0; passwd passwordStruct{}; passwd *passwordResult = nullptr; - char buffer[kBufferSize]; - int result = getpwnam_r(fileOwner.c_str(), &passwordStruct, buffer, kBufferSize, &passwordResult); + std::vector buffer(BufferSize); + int result = getpwnam_r(fileOwner.c_str(), &passwordStruct, buffer.data(), BufferSize, &passwordResult); if (result == 0 && passwordResult) { ownerId = passwordResult->pw_uid; @@ -55,14 +64,23 @@ uid_t getFileOwnerId(const std::string &fileOwner) gid_t getFileGroupId(const std::string &fileGroup) { gid_t groupId = kNoGroupChange; - const size_t kBufferSize = sysconf(_SC_GETPW_R_SIZE_MAX); - if (!fileGroup.empty() && kBufferSize > 0) + long buffersize = sysconf(_SC_GETPW_R_SIZE_MAX); + size_t BufferSize = 0; + if (buffersize == -1) + { + RIALTO_COMMON_LOG_SYS_WARN(errno, "Invalid Buffer Size '%s'", fileGroup.c_str()); + } + if (buffersize > 0) + { + BufferSize = static_cast(buffersize); + } + if (!fileGroup.empty() && BufferSize > 0) { errno = 0; group groupStruct{}; group *groupResult = nullptr; - char buffer[kBufferSize]; - int result = getgrnam_r(fileGroup.c_str(), &groupStruct, buffer, kBufferSize, &groupResult); + std::vector buffer(BufferSize); + int result = getgrnam_r(fileGroup.c_str(), &groupStruct, buffer.data(), BufferSize, &groupResult); if (result == 0 && groupResult) { groupId = groupResult->gr_gid; diff --git a/ipc/common/source/NamedSocket.cpp b/ipc/common/source/NamedSocket.cpp index f3565aa5b..b3a8e5904 100644 --- a/ipc/common/source/NamedSocket.cpp +++ b/ipc/common/source/NamedSocket.cpp @@ -268,14 +268,23 @@ bool NamedSocket::getSocketLock() uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const { uid_t ownerId = kNoOwnerChange; - const size_t kBufferSize = sysconf(_SC_GETPW_R_SIZE_MAX); - if (!socketOwner.empty() && kBufferSize > 0) + long buffersize = sysconf(_SC_GETPW_R_SIZE_MAX); + size_t BufferSize = 0; + if (buffersize == -1) + { + RIALTO_IPC_LOG_SYS_WARN(errno, "Invalid Buffer Size '%s'", socketOwner.c_str()); + } + if (buffersize > 0) + { + BufferSize = static_cast(buffersize); + } + if (!socketOwner.empty() && BufferSize > 0) { errno = 0; passwd passwordStruct{}; passwd *passwordResult = nullptr; - char buffer[kBufferSize]; - int result = getpwnam_r(socketOwner.c_str(), &passwordStruct, buffer, kBufferSize, &passwordResult); + std::vector buffer(BufferSize); + int result = getpwnam_r(socketOwner.c_str(), &passwordStruct, buffer.data(), BufferSize, &passwordResult); if (result == 0 && passwordResult) { ownerId = passwordResult->pw_uid; @@ -291,14 +300,23 @@ uid_t NamedSocket::getSocketOwnerId(const std::string &socketOwner) const gid_t NamedSocket::getSocketGroupId(const std::string &socketGroup) const { gid_t groupId = kNoGroupChange; - const size_t kBufferSize = sysconf(_SC_GETPW_R_SIZE_MAX); - if (!socketGroup.empty() && kBufferSize > 0) + long buffersize = sysconf(_SC_GETPW_R_SIZE_MAX); + size_t BufferSize = 0; + if (buffersize == -1) + { + RIALTO_IPC_LOG_SYS_WARN(errno, "Invalid Buffer Size '%s'", socketGroup.c_str()); + } + if (buffersize > 0) + { + BufferSize = static_cast(buffersize); + } + if (!socketGroup.empty() && BufferSize > 0) { errno = 0; group groupStruct{}; group *groupResult = nullptr; - char buffer[kBufferSize]; - int result = getgrnam_r(socketGroup.c_str(), &groupStruct, buffer, kBufferSize, &groupResult); + std::vector buffer(BufferSize); + int result = getgrnam_r(socketGroup.c_str(), &groupStruct, buffer.data(), BufferSize, &groupResult); if (result == 0 && groupResult) { groupId = groupResult->gr_gid; diff --git a/media/client/main/source/MediaPipeline.cpp b/media/client/main/source/MediaPipeline.cpp index 280aff919..3a05ec487 100644 --- a/media/client/main/source/MediaPipeline.cpp +++ b/media/client/main/source/MediaPipeline.cpp @@ -184,7 +184,7 @@ MediaPipeline::MediaPipeline(std::weak_ptr client, const V const std::shared_ptr &mediaFrameWriterFactory, IClientController &clientController) : m_mediaPipelineClient(client), m_clientController{clientController}, m_currentAppState{ApplicationState::UNKNOWN}, - m_mediaFrameWriterFactory(mediaFrameWriterFactory), m_currentState(State::IDLE) + m_mediaFrameWriterFactory(mediaFrameWriterFactory), m_currentState(State::IDLE), m_attachingSource(false) { RIALTO_CLIENT_LOG_DEBUG("entry:"); diff --git a/media/public/include/IMediaPipeline.h b/media/public/include/IMediaPipeline.h index 1064ccc1e..789115750 100644 --- a/media/public/include/IMediaPipeline.h +++ b/media/public/include/IMediaPipeline.h @@ -543,7 +543,7 @@ class IMediaPipeline * * @retval the media key session id. */ - const int32_t getMediaKeySessionId() const { return m_mediaKeySessionId; } + int32_t getMediaKeySessionId() const { return m_mediaKeySessionId; } /** * @brief Returns the key id. Empty if unencrypted. @@ -571,14 +571,14 @@ class IMediaPipeline * * @retval the initWithLast15 value. */ - const uint32_t getInitWithLast15() const { return m_initWithLast15; } + uint32_t getInitWithLast15() const { return m_initWithLast15; } /** * @brief Returns the segment alignment * * @retval the segment alignment */ - const SegmentAlignment getSegmentAlignment() const { return m_alignment; } + SegmentAlignment getSegmentAlignment() const { return m_alignment; } /** * @brief Gets the codec data @@ -602,7 +602,7 @@ class IMediaPipeline * * @retval if the encryption pattern has been set */ - const bool getEncryptionPattern(uint32_t &crypt, uint32_t &skip) const + bool getEncryptionPattern(uint32_t &crypt, uint32_t &skip) const { crypt = m_crypt; skip = m_skip; diff --git a/media/server/gstplayer/source/GstDecryptor.cpp b/media/server/gstplayer/source/GstDecryptor.cpp index 0882b36d3..6164a61dc 100644 --- a/media/server/gstplayer/source/GstDecryptor.cpp +++ b/media/server/gstplayer/source/GstDecryptor.cpp @@ -233,7 +233,7 @@ GstRialtoDecryptorPrivate::GstRialtoDecryptorPrivate( GstBaseTransform *parentElement, const std::shared_ptr &gstWrapperFactory, const std::shared_ptr &glibWrapperFactory) - : m_decryptorElement(parentElement) + : m_decryptorElement(parentElement), m_decryptionService(nullptr) { if ((!gstWrapperFactory) || (!(m_gstWrapper = gstWrapperFactory->getGstWrapper()))) { diff --git a/media/server/gstplayer/source/GstGenericPlayer.cpp b/media/server/gstplayer/source/GstGenericPlayer.cpp index 1b0976a73..7e83f911a 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -1448,7 +1448,7 @@ bool GstGenericPlayer::setSyncOff() if (m_glibWrapper->gObjectClassFindProperty(G_OBJECT_GET_CLASS(decoder), "sync-off")) { - gboolean syncOffGboolean{decoder ? TRUE : FALSE}; + gboolean syncOffGboolean{syncOff ? TRUE : FALSE}; m_glibWrapper->gObjectSet(decoder, "sync-off", syncOffGboolean, nullptr); result = true; } @@ -1546,8 +1546,12 @@ bool GstGenericPlayer::setRenderFrame() RIALTO_SERVER_LOG_INFO("Rendering preroll"); m_glibWrapper->gObjectSet(sink, kStepOnPrerollPropertyName.c_str(), 1, nullptr); - m_gstWrapper->gstElementSendEvent(sink, m_gstWrapper->gstEventNewStep(GST_FORMAT_BUFFERS, 1, 1.0, true, - false)); + if (!m_gstWrapper->gstElementSendEvent(sink, m_gstWrapper->gstEventNewStep(GST_FORMAT_BUFFERS, 1, 1.0, true, + false))) + { + RIALTO_SERVER_LOG_ERROR("Failed to send new step event to sink element"); + } + m_glibWrapper->gObjectSet(sink, kStepOnPrerollPropertyName.c_str(), 0, nullptr); result = true; } diff --git a/media/server/gstplayer/source/GstSrc.cpp b/media/server/gstplayer/source/GstSrc.cpp index ff7b1e227..c544fa04a 100644 --- a/media/server/gstplayer/source/GstSrc.cpp +++ b/media/server/gstplayer/source/GstSrc.cpp @@ -48,7 +48,9 @@ static void gstRialtoSrcFinalize(GObject *object) { GstRialtoSrc *src = GST_RIALTO_SRC(object); GstRialtoSrcPrivate *priv = src->priv; + GST_OBJECT_LOCK(src); g_free(priv->uri); + GST_OBJECT_UNLOCK(src); priv->~GstRialtoSrcPrivate(); GST_CALL_PARENT(G_OBJECT_CLASS, finalize, (object)); } diff --git a/media/server/ipc/source/MediaPipelineModuleService.cpp b/media/server/ipc/source/MediaPipelineModuleService.cpp index 77c1387aa..d6c3a1ac3 100644 --- a/media/server/ipc/source/MediaPipelineModuleService.cpp +++ b/media/server/ipc/source/MediaPipelineModuleService.cpp @@ -424,10 +424,10 @@ void MediaPipelineModuleService::attachSource(::google::protobuf::RpcController std::shared_ptr codecData{}; if (request->has_codec_data()) { - auto codecDataProto = request->codec_data(); + const auto &kCodecDataProto = request->codec_data(); codecData = std::make_shared(); - codecData->data = std::vector(codecDataProto.data().begin(), codecDataProto.data().end()); - codecData->type = convertCodecDataType(codecDataProto.type()); + codecData->data = std::vector(kCodecDataProto.data().begin(), kCodecDataProto.data().end()); + codecData->type = convertCodecDataType(kCodecDataProto.type()); } std::unique_ptr mediaSource; firebolt::rialto::SourceConfigType configType = convertConfigType(request->config_type()); diff --git a/media/server/main/interface/IMainThread.h b/media/server/main/interface/IMainThread.h index 6a4ac3490..9fe0b9124 100644 --- a/media/server/main/interface/IMainThread.h +++ b/media/server/main/interface/IMainThread.h @@ -20,7 +20,6 @@ #ifndef FIREBOLT_RIALTO_SERVER_I_MAIN_THREAD_H_ #define FIREBOLT_RIALTO_SERVER_I_MAIN_THREAD_H_ -#include "IMainThread.h" #include #include #include diff --git a/media/server/main/source/ActiveRequests.cpp b/media/server/main/source/ActiveRequests.cpp index 3431fc672..d4d5fe254 100644 --- a/media/server/main/source/ActiveRequests.cpp +++ b/media/server/main/source/ActiveRequests.cpp @@ -104,6 +104,7 @@ AddSegmentStatus ActiveRequests::addSegment(std::uint32_t requestId, return AddSegmentStatus::ERROR; } + std::unique_lock lock{m_mutex}; auto requestIter{m_requestMap.find(requestId)}; if (requestIter != m_requestMap.end()) { @@ -115,6 +116,7 @@ AddSegmentStatus ActiveRequests::addSegment(std::uint32_t requestId, const IMediaPipeline::MediaSegmentVector &ActiveRequests::getSegments(std::uint32_t requestId) const { + std::unique_lock lock{m_mutex}; auto requestIter{m_requestMap.find(requestId)}; if (requestIter != m_requestMap.end()) { diff --git a/media/server/main/source/NeedMediaData.cpp b/media/server/main/source/NeedMediaData.cpp index 3d24cdc4e..85d03b01d 100644 --- a/media/server/main/source/NeedMediaData.cpp +++ b/media/server/main/source/NeedMediaData.cpp @@ -31,7 +31,7 @@ NeedMediaData::NeedMediaData(std::weak_ptr client, IActive const ISharedMemoryBuffer &shmBuffer, int sessionId, MediaSourceType mediaSourceType, std::int32_t sourceId, PlaybackState currentPlaybackState) : m_client{client}, m_activeRequests{activeRequests}, m_mediaSourceType{mediaSourceType}, m_frameCount{kMaxFrames}, - m_sourceId{sourceId} + m_sourceId{sourceId}, m_maxMediaBytes{0} { if (MediaSourceType::AUDIO != mediaSourceType && MediaSourceType::VIDEO != mediaSourceType && MediaSourceType::SUBTITLE != mediaSourceType) diff --git a/serverManager/common/source/SessionServerAppManager.cpp b/serverManager/common/source/SessionServerAppManager.cpp index 7ce152a51..df2d26082 100644 --- a/serverManager/common/source/SessionServerAppManager.cpp +++ b/serverManager/common/source/SessionServerAppManager.cpp @@ -486,7 +486,7 @@ bool SessionServerAppManager::configureSessionServerWithSocketName(const std::un const auto kSocketPermissions{kSessionServer->getSessionManagementSocketPermissions()}; const auto kSocketOwner{kSessionServer->getSessionManagementSocketOwner()}; const auto kSocketGroup{kSessionServer->getSessionManagementSocketGroup()}; - const auto kAppName{kSessionServer->getAppName()}; + const auto &kAppName{kSessionServer->getAppName()}; const firebolt::rialto::common::MaxResourceCapabilitites kMaxResource{kSessionServer->getMaxPlaybackSessions(), kSessionServer->getMaxWebAudioPlayers()}; @@ -508,7 +508,7 @@ bool SessionServerAppManager::configureSessionServerWithSocketFd(const std::uniq const auto kInitialState{kSessionServer->getInitialState()}; const auto kSocketFd{kSessionServer->getSessionManagementSocketFd()}; const auto kClientDisplayName{kSessionServer->getClientDisplayName()}; - const auto kAppName{kSessionServer->getAppName()}; + const auto &kAppName{kSessionServer->getAppName()}; const firebolt::rialto::common::MaxResourceCapabilitites kMaxResource{kSessionServer->getMaxPlaybackSessions(), kSessionServer->getMaxWebAudioPlayers()}; diff --git a/wrappers/source/OcdmSession.cpp b/wrappers/source/OcdmSession.cpp index a236826a9..0c27e6e72 100644 --- a/wrappers/source/OcdmSession.cpp +++ b/wrappers/source/OcdmSession.cpp @@ -70,7 +70,7 @@ const char *convertInitDataType(const firebolt::rialto::InitDataType &initDataTy } } } -const firebolt::rialto::KeyStatus convertKeyStatus(const KeyStatus &ocdmKeyStatus) +firebolt::rialto::KeyStatus convertKeyStatus(const KeyStatus &ocdmKeyStatus) { switch (ocdmKeyStatus) { From 99315d1e56390a9c5a0dfbd1e5cb590406ca7728 Mon Sep 17 00:00:00 2001 From: Muthu Shiam Sankar N Date: Thu, 18 Dec 2025 19:24:47 +0530 Subject: [PATCH 2/4] To clear Coverity warnings Summary: To clear Coverity warnings Type: Fix Test Plan: UT/CT, Fullstack Jira: ENTDAI-1354/RDKECOREMW-947 --- ipc/server/source/IpcServerImpl.cpp | 8 ++++---- logging/source/EnvVariableParser.cpp | 2 +- media/client/ipc/source/IpcClient.cpp | 2 +- media/client/main/source/ClientController.cpp | 2 +- media/client/main/source/Control.cpp | 2 +- media/client/main/source/MediaPipeline.cpp | 2 +- media/server/gstplayer/source/CapsBuilder.cpp | 8 ++++---- media/server/gstplayer/source/GstCapabilities.cpp | 2 +- .../gstplayer/source/tasks/generic/AttachSamples.cpp | 4 ++-- .../source/tasks/generic/CheckAudioUnderflow.cpp | 2 +- media/server/gstplayer/source/tasks/generic/Eos.cpp | 2 +- media/server/gstplayer/source/tasks/generic/Flush.cpp | 2 +- .../gstplayer/source/tasks/generic/ProcessAudioGap.cpp | 2 +- .../gstplayer/source/tasks/generic/RemoveSource.cpp | 2 +- .../gstplayer/source/tasks/generic/ReportPosition.cpp | 2 +- media/server/gstplayer/source/tasks/generic/SetMute.cpp | 2 +- .../gstplayer/source/tasks/generic/SetPlaybackRate.cpp | 2 +- .../server/gstplayer/source/tasks/generic/SetPosition.cpp | 2 +- .../source/tasks/generic/SetTextTrackIdentifier.cpp | 2 +- media/server/gstplayer/source/tasks/generic/SetVolume.cpp | 4 ++-- .../gstplayer/source/tasks/generic/SetupElement.cpp | 2 +- .../source/tasks/generic/UpdatePlaybackGroup.cpp | 2 +- media/server/gstplayer/source/tasks/webAudio/Eos.cpp | 2 +- .../gstplayer/source/tasks/webAudio/HandleBusMessage.cpp | 4 ++-- media/server/gstplayer/source/tasks/webAudio/SetCaps.cpp | 6 +++--- .../server/gstplayer/source/tasks/webAudio/SetVolume.cpp | 2 +- .../gstplayer/source/tasks/webAudio/WriteBuffer.cpp | 2 +- media/server/ipc/source/MediaPipelineModuleService.cpp | 4 ++-- media/server/main/source/MainThread.cpp | 8 ++++---- media/server/main/source/MediaKeySession.cpp | 4 ++-- media/server/main/source/MediaKeysCapabilities.cpp | 2 +- media/server/main/source/MediaKeysServerInternal.cpp | 2 +- media/server/main/source/MediaPipelineCapabilities.cpp | 2 +- media/server/main/source/MediaPipelineServerInternal.cpp | 4 ++-- media/server/service/source/CdmService.cpp | 2 +- media/server/service/source/MediaPipelineService.cpp | 2 +- media/server/service/source/WebAudioPlayerService.cpp | 2 +- serverManager/serverManagerSim/HttpRequest.cpp | 4 ++-- 38 files changed, 56 insertions(+), 56 deletions(-) diff --git a/ipc/server/source/IpcServerImpl.cpp b/ipc/server/source/IpcServerImpl.cpp index f6cd2da73..b6a9b3abb 100644 --- a/ipc/server/source/IpcServerImpl.cpp +++ b/ipc/server/source/IpcServerImpl.cpp @@ -277,8 +277,8 @@ bool ServerImpl::addSocket(const std::string &socketPath, } // store the client connected / disconnected callbacks - socket.connectedCb = clientConnectedCb; - socket.disconnectedCb = clientDisconnectedCb; + socket.connectedCb = std::move(clientConnectedCb); + socket.disconnectedCb = std::move(clientDisconnectedCb); // add to the internal map std::lock_guard locker(m_socketsLock); @@ -323,8 +323,8 @@ bool ServerImpl::addSocket(int fd, std::function locker(m_socketsLock); diff --git a/logging/source/EnvVariableParser.cpp b/logging/source/EnvVariableParser.cpp index 5ef89d89c..171a2803a 100644 --- a/logging/source/EnvVariableParser.cpp +++ b/logging/source/EnvVariableParser.cpp @@ -70,7 +70,7 @@ std::vector split(std::string s, const std::string &delimiter) result.push_back(s.substr(0, pos)); s.erase(0, pos + delimiter.length()); } - result.push_back(s); + result.push_back(std::move(s)); return result; } diff --git a/media/client/ipc/source/IpcClient.cpp b/media/client/ipc/source/IpcClient.cpp index 53c5b8d6a..c0cd9840f 100644 --- a/media/client/ipc/source/IpcClient.cpp +++ b/media/client/ipc/source/IpcClient.cpp @@ -191,7 +191,7 @@ std::shared_ptr IpcClient::createBlockingClosure() // check which thread we're being called from, this determines if we pump // event loop from within the wait() method or not if (m_ipcThread.get_id() == std::this_thread::get_id()) - return m_blockingClosureFactory->createBlockingClosurePoll(ipcChannel); + return m_blockingClosureFactory->createBlockingClosurePoll(std::move(ipcChannel)); else return m_blockingClosureFactory->createBlockingClosureSemaphore(); } diff --git a/media/client/main/source/ClientController.cpp b/media/client/main/source/ClientController.cpp index eac82154c..783ac3153 100644 --- a/media/client/main/source/ClientController.cpp +++ b/media/client/main/source/ClientController.cpp @@ -265,7 +265,7 @@ void ClientController::changeStateAndNotifyClients(ApplicationState state) std::shared_ptr clientLocked{client.lock()}; if (clientLocked) { - currentClients.push_back(clientLocked); + currentClients.push_back(std::move(clientLocked)); } else { diff --git a/media/client/main/source/Control.cpp b/media/client/main/source/Control.cpp index 215558fe8..e23b2e8fa 100644 --- a/media/client/main/source/Control.cpp +++ b/media/client/main/source/Control.cpp @@ -76,7 +76,7 @@ bool Control::registerClient(std::weak_ptr client, ApplicationSt std::shared_ptr lockedClient = client.lock(); if (lockedClient && m_clientController.registerClient(lockedClient, appState)) { - m_clientsToUnregister.push_back(lockedClient); + m_clientsToUnregister.push_back(std::move(lockedClient)); return true; } RIALTO_CLIENT_LOG_WARN("Unable to register client"); diff --git a/media/client/main/source/MediaPipeline.cpp b/media/client/main/source/MediaPipeline.cpp index 3a05ec487..661cbc0f4 100644 --- a/media/client/main/source/MediaPipeline.cpp +++ b/media/client/main/source/MediaPipeline.cpp @@ -808,7 +808,7 @@ void MediaPipeline::notifyNeedMediaData(int32_t sourceId, size_t frameCount, uin RIALTO_CLIENT_LOG_INFO("NeedMediaData received in state != RUNNING, ignoring request id %u", requestId); break; } - m_needDataRequestMap[requestId] = needDataRequest; + m_needDataRequestMap[requestId] = std::move(needDataRequest); } std::shared_ptr client = m_mediaPipelineClient.lock(); diff --git a/media/server/gstplayer/source/CapsBuilder.cpp b/media/server/gstplayer/source/CapsBuilder.cpp index 4491b3656..0b6cfd32f 100644 --- a/media/server/gstplayer/source/CapsBuilder.cpp +++ b/media/server/gstplayer/source/CapsBuilder.cpp @@ -27,7 +27,7 @@ namespace firebolt::rialto::server MediaSourceCapsBuilder::MediaSourceCapsBuilder(std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const firebolt::rialto::IMediaPipeline::MediaSourceAV &source) - : m_gstWrapper(gstWrapper), m_glibWrapper(glibWrapper), m_attachedSource(source) + : m_gstWrapper(std::move(gstWrapper)), m_glibWrapper(std::move(glibWrapper)), m_attachedSource(source) { } GstCaps *MediaSourceCapsBuilder::buildCaps() @@ -94,7 +94,7 @@ void MediaSourceCapsBuilder::addStreamFormatToCaps(GstCaps *caps) const MediaSourceAudioCapsBuilder::MediaSourceAudioCapsBuilder( std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const IMediaPipeline::MediaSourceAudio &source) - : MediaSourceCapsBuilder(gstWrapper, glibWrapper, source), m_attachedAudioSource(source) + : MediaSourceCapsBuilder(std::move(gstWrapper), std::move(glibWrapper), source), m_attachedAudioSource(source) { } @@ -228,7 +228,7 @@ void MediaSourceAudioCapsBuilder::addFlacSpecificData(GstCaps *caps) const MediaSourceVideoCapsBuilder::MediaSourceVideoCapsBuilder( std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const IMediaPipeline::MediaSourceVideo &source) - : MediaSourceCapsBuilder(gstWrapper, glibWrapper, source), m_attachedVideoSource(source) + : MediaSourceCapsBuilder(std::move(gstWrapper), std::move(glibWrapper), source), m_attachedVideoSource(source) { } @@ -247,7 +247,7 @@ MediaSourceVideoDolbyVisionCapsBuilder::MediaSourceVideoDolbyVisionCapsBuilder( std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const IMediaPipeline::MediaSourceVideoDolbyVision &source) - : MediaSourceVideoCapsBuilder(gstWrapper, glibWrapper, source), m_attachedDolbySource(source) + : MediaSourceVideoCapsBuilder(std::move(gstWrapper), std::move(glibWrapper), source), m_attachedDolbySource(source) { } diff --git a/media/server/gstplayer/source/GstCapabilities.cpp b/media/server/gstplayer/source/GstCapabilities.cpp index 13173441d..a40e98ed5 100644 --- a/media/server/gstplayer/source/GstCapabilities.cpp +++ b/media/server/gstplayer/source/GstCapabilities.cpp @@ -279,7 +279,7 @@ std::vector GstCapabilities::getSupportedProperties(MediaSourceType if (it != propertiesToLookFor.end()) { RIALTO_SERVER_LOG_DEBUG("Found property '%s'", kPropName.c_str()); - propertiesFound.push_back(kPropName); + propertiesFound.push_back(std::move(kPropName)); propertiesToLookFor.erase(it); } } diff --git a/media/server/gstplayer/source/tasks/generic/AttachSamples.cpp b/media/server/gstplayer/source/tasks/generic/AttachSamples.cpp index ef4605d4e..650b1ed7f 100644 --- a/media/server/gstplayer/source/tasks/generic/AttachSamples.cpp +++ b/media/server/gstplayer/source/tasks/generic/AttachSamples.cpp @@ -42,7 +42,7 @@ AttachSamples::AttachSamples(GenericPlayerContext &context, dynamic_cast(*mediaSegment); VideoData videoData = {gstBuffer, videoSegment.getWidth(), videoSegment.getHeight(), videoSegment.getFrameRate(), videoSegment.getCodecData()}; - m_videoData.push_back(videoData); + m_videoData.push_back(std::move(videoData)); } catch (const std::exception &e) { @@ -62,7 +62,7 @@ AttachSamples::AttachSamples(GenericPlayerContext &context, audioSegment.getCodecData(), audioSegment.getClippingStart(), audioSegment.getClippingEnd()}; - m_audioData.push_back(audioData); + m_audioData.push_back(std::move(audioData)); } catch (const std::exception &e) { diff --git a/media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp b/media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp index c027c69a2..6fafb8150 100644 --- a/media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp +++ b/media/server/gstplayer/source/tasks/generic/CheckAudioUnderflow.cpp @@ -32,7 +32,7 @@ namespace firebolt::rialto::server::tasks::generic CheckAudioUnderflow::CheckAudioUnderflow(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, IGstGenericPlayerClient *client, std::shared_ptr gstWrapper) - : m_context{context}, m_player(player), m_gstPlayerClient{client}, m_gstWrapper{gstWrapper} + : m_context{context}, m_player(player), m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)} { } diff --git a/media/server/gstplayer/source/tasks/generic/Eos.cpp b/media/server/gstplayer/source/tasks/generic/Eos.cpp index 371487570..30e3d42a4 100644 --- a/media/server/gstplayer/source/tasks/generic/Eos.cpp +++ b/media/server/gstplayer/source/tasks/generic/Eos.cpp @@ -28,7 +28,7 @@ namespace firebolt::rialto::server::tasks::generic Eos::Eos(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, std::shared_ptr gstWrapper, const firebolt::rialto::MediaSourceType &type) - : m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_type{type} + : m_context{context}, m_player{player}, m_gstWrapper{std::move(gstWrapper)}, m_type{type} { RIALTO_SERVER_LOG_DEBUG("Constructing Eos"); } diff --git a/media/server/gstplayer/source/tasks/generic/Flush.cpp b/media/server/gstplayer/source/tasks/generic/Flush.cpp index 9c7c5196f..6cbb1e392 100644 --- a/media/server/gstplayer/source/tasks/generic/Flush.cpp +++ b/media/server/gstplayer/source/tasks/generic/Flush.cpp @@ -27,7 +27,7 @@ namespace firebolt::rialto::server::tasks::generic Flush::Flush(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, IGstGenericPlayerClient *client, std::shared_ptr gstWrapper, const MediaSourceType &type, bool resetTime) - : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{gstWrapper}, m_type{type}, + : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)}, m_type{type}, m_resetTime{resetTime} { RIALTO_SERVER_LOG_DEBUG("Constructing Flush"); diff --git a/media/server/gstplayer/source/tasks/generic/ProcessAudioGap.cpp b/media/server/gstplayer/source/tasks/generic/ProcessAudioGap.cpp index 13f5b870b..24d0c7a16 100644 --- a/media/server/gstplayer/source/tasks/generic/ProcessAudioGap.cpp +++ b/media/server/gstplayer/source/tasks/generic/ProcessAudioGap.cpp @@ -28,7 +28,7 @@ ProcessAudioGap::ProcessAudioGap( const std::shared_ptr rdkGstreamerUtilsWrapper, std::int64_t position, std::uint32_t duration, std::int64_t discontinuityGap, bool audioAac) : m_context{context}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, - m_rdkGstreamerUtilsWrapper{rdkGstreamerUtilsWrapper}, m_position{position}, m_duration{duration}, + m_rdkGstreamerUtilsWrapper{std::move(rdkGstreamerUtilsWrapper)}, m_position{position}, m_duration{duration}, m_discontinuityGap{discontinuityGap}, m_audioAac{audioAac} { RIALTO_SERVER_LOG_DEBUG("Constructing ProcessAudioGap"); diff --git a/media/server/gstplayer/source/tasks/generic/RemoveSource.cpp b/media/server/gstplayer/source/tasks/generic/RemoveSource.cpp index 29127c08c..a09a75ed3 100644 --- a/media/server/gstplayer/source/tasks/generic/RemoveSource.cpp +++ b/media/server/gstplayer/source/tasks/generic/RemoveSource.cpp @@ -27,7 +27,7 @@ RemoveSource::RemoveSource(GenericPlayerContext &context, IGstGenericPlayerPriva IGstGenericPlayerClient *client, std::shared_ptr gstWrapper, const MediaSourceType &type) - : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{gstWrapper}, m_type{type} + : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)}, m_type{type} { RIALTO_SERVER_LOG_DEBUG("Constructing RemoveSource"); } diff --git a/media/server/gstplayer/source/tasks/generic/ReportPosition.cpp b/media/server/gstplayer/source/tasks/generic/ReportPosition.cpp index b7392e366..a676ad80d 100644 --- a/media/server/gstplayer/source/tasks/generic/ReportPosition.cpp +++ b/media/server/gstplayer/source/tasks/generic/ReportPosition.cpp @@ -29,7 +29,7 @@ namespace firebolt::rialto::server::tasks::generic ReportPosition::ReportPosition(GenericPlayerContext &context, IGstGenericPlayerClient *client, const std::shared_ptr &gstWrapper, IGstGenericPlayerPrivate &player) - : m_context{context}, m_gstPlayerClient{client}, m_gstWrapper{gstWrapper}, m_player{player} + : m_context{context}, m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)}, m_player{player} { } diff --git a/media/server/gstplayer/source/tasks/generic/SetMute.cpp b/media/server/gstplayer/source/tasks/generic/SetMute.cpp index da66764a8..1c38a1a9f 100644 --- a/media/server/gstplayer/source/tasks/generic/SetMute.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetMute.cpp @@ -28,7 +28,7 @@ SetMute::SetMute(GenericPlayerContext &context, IGstGenericPlayerPrivate &player std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const MediaSourceType &mediaSourceType, bool mute) - : m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, + : m_context{context}, m_player{player}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, m_mediaSourceType{mediaSourceType}, m_mute{mute} { RIALTO_SERVER_LOG_DEBUG("Constructing SetMute"); diff --git a/media/server/gstplayer/source/tasks/generic/SetPlaybackRate.cpp b/media/server/gstplayer/source/tasks/generic/SetPlaybackRate.cpp index f5f8f609c..8c34966c9 100644 --- a/media/server/gstplayer/source/tasks/generic/SetPlaybackRate.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetPlaybackRate.cpp @@ -33,7 +33,7 @@ namespace firebolt::rialto::server::tasks::generic SetPlaybackRate::SetPlaybackRate(GenericPlayerContext &context, std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, double rate) - : m_context{context}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_rate{rate} + : m_context{context}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, m_rate{rate} { RIALTO_SERVER_LOG_DEBUG("Constructing SetPlaybackRate"); } diff --git a/media/server/gstplayer/source/tasks/generic/SetPosition.cpp b/media/server/gstplayer/source/tasks/generic/SetPosition.cpp index df9d1b72b..c938b9d79 100644 --- a/media/server/gstplayer/source/tasks/generic/SetPosition.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetPosition.cpp @@ -29,7 +29,7 @@ namespace firebolt::rialto::server::tasks::generic { SetPosition::SetPosition(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, IGstGenericPlayerClient *client, std::shared_ptr gstWrapper, std::int64_t position) - : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{gstWrapper}, m_position{position} + : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)}, m_position{position} { RIALTO_SERVER_LOG_DEBUG("Constructing SetPosition"); } diff --git a/media/server/gstplayer/source/tasks/generic/SetTextTrackIdentifier.cpp b/media/server/gstplayer/source/tasks/generic/SetTextTrackIdentifier.cpp index b29ab0572..b9525bd9c 100644 --- a/media/server/gstplayer/source/tasks/generic/SetTextTrackIdentifier.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetTextTrackIdentifier.cpp @@ -26,7 +26,7 @@ namespace firebolt::rialto::server::tasks::generic SetTextTrackIdentifier::SetTextTrackIdentifier(GenericPlayerContext &context, std::shared_ptr glibWrapper, const std::string &textTrackIdentifier) - : m_context{context}, m_glibWrapper{glibWrapper}, m_textTrackIdentifier{textTrackIdentifier} + : m_context{context}, m_glibWrapper{std::move(glibWrapper)}, m_textTrackIdentifier{textTrackIdentifier} { RIALTO_SERVER_LOG_DEBUG("Constructing SetTextTrackIdentifier"); } diff --git a/media/server/gstplayer/source/tasks/generic/SetVolume.cpp b/media/server/gstplayer/source/tasks/generic/SetVolume.cpp index 0ceeae94f..1449ac46a 100644 --- a/media/server/gstplayer/source/tasks/generic/SetVolume.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetVolume.cpp @@ -31,8 +31,8 @@ SetVolume::SetVolume(GenericPlayerContext &context, IGstGenericPlayerPrivate &pl std::shared_ptr glibWrapper, std::shared_ptr rdkGstreamerUtilsWrapper, double targetVolume, uint32_t volumeDuration, firebolt::rialto::EaseType easeType) - : m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, - m_rdkGstreamerUtilsWrapper{rdkGstreamerUtilsWrapper}, m_targetVolume{targetVolume}, + : m_context{context}, m_player{player}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, + m_rdkGstreamerUtilsWrapper{std::move(rdkGstreamerUtilsWrapper)}, m_targetVolume{targetVolume}, m_volumeDuration{volumeDuration}, m_easeType{easeType} { RIALTO_SERVER_LOG_DEBUG("Constructing SetVolume"); diff --git a/media/server/gstplayer/source/tasks/generic/SetupElement.cpp b/media/server/gstplayer/source/tasks/generic/SetupElement.cpp index 99588430a..e6070c842 100644 --- a/media/server/gstplayer/source/tasks/generic/SetupElement.cpp +++ b/media/server/gstplayer/source/tasks/generic/SetupElement.cpp @@ -132,7 +132,7 @@ SetupElement::SetupElement(GenericPlayerContext &context, std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, IGstGenericPlayerPrivate &player, GstElement *element) - : m_context{context}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_player{player}, m_element{element} + : m_context{context}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, m_player{player}, m_element{element} { RIALTO_SERVER_LOG_DEBUG("Constructing SetupElement"); } diff --git a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp index 36f3f79b7..225952566 100644 --- a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp +++ b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp @@ -26,7 +26,7 @@ UpdatePlaybackGroup::UpdatePlaybackGroup(GenericPlayerContext &context, IGstGene std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, GstElement *typefind, const GstCaps *caps) - : m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_typefind{typefind}, + : m_context{context}, m_player{player}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, m_typefind{typefind}, m_caps{caps} { RIALTO_SERVER_LOG_DEBUG("Constructing UpdatePlaybackGroup"); diff --git a/media/server/gstplayer/source/tasks/webAudio/Eos.cpp b/media/server/gstplayer/source/tasks/webAudio/Eos.cpp index be11e5405..dca53a5e0 100644 --- a/media/server/gstplayer/source/tasks/webAudio/Eos.cpp +++ b/media/server/gstplayer/source/tasks/webAudio/Eos.cpp @@ -25,7 +25,7 @@ namespace firebolt::rialto::server::tasks::webaudio { Eos::Eos(WebAudioPlayerContext &context, std::shared_ptr gstWrapper) - : m_context{context}, m_gstWrapper{gstWrapper} + : m_context{context}, m_gstWrapper{std::move(gstWrapper)} { RIALTO_SERVER_LOG_DEBUG("Constructing Eos"); } diff --git a/media/server/gstplayer/source/tasks/webAudio/HandleBusMessage.cpp b/media/server/gstplayer/source/tasks/webAudio/HandleBusMessage.cpp index e8ffad051..923d5ff8c 100644 --- a/media/server/gstplayer/source/tasks/webAudio/HandleBusMessage.cpp +++ b/media/server/gstplayer/source/tasks/webAudio/HandleBusMessage.cpp @@ -30,8 +30,8 @@ HandleBusMessage::HandleBusMessage(WebAudioPlayerContext &context, IGstWebAudioP std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, GstMessage *message) - : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{gstWrapper}, - m_glibWrapper{glibWrapper}, m_message{message} + : m_context{context}, m_player{player}, m_gstPlayerClient{client}, m_gstWrapper{std::move(gstWrapper)}, + m_glibWrapper{std::move(glibWrapper)}, m_message{message} { RIALTO_SERVER_LOG_DEBUG("Constructing HandleBusMessage"); } diff --git a/media/server/gstplayer/source/tasks/webAudio/SetCaps.cpp b/media/server/gstplayer/source/tasks/webAudio/SetCaps.cpp index 5d42d4c31..4defb90d8 100644 --- a/media/server/gstplayer/source/tasks/webAudio/SetCaps.cpp +++ b/media/server/gstplayer/source/tasks/webAudio/SetCaps.cpp @@ -35,7 +35,7 @@ class WebAudioCapsBuilder public: WebAudioCapsBuilder(std::shared_ptr gstWrapper, std::shared_ptr glibWrapper) - : m_gstWrapper(gstWrapper), m_glibWrapper(glibWrapper) + : m_gstWrapper(std::move(gstWrapper)), m_glibWrapper(std::move(glibWrapper)) { } virtual ~WebAudioCapsBuilder() = default; @@ -52,7 +52,7 @@ class WebAudioPcmCapsBuilder : public WebAudioCapsBuilder WebAudioPcmCapsBuilder(std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const WebAudioPcmConfig &pcmConfig) - : WebAudioCapsBuilder(gstWrapper, glibWrapper), m_pcmConfig(pcmConfig) + : WebAudioCapsBuilder(std::move(gstWrapper), std::move(glibWrapper)), m_pcmConfig(pcmConfig) { } ~WebAudioPcmCapsBuilder() override = default; @@ -108,7 +108,7 @@ class WebAudioPcmCapsBuilder : public WebAudioCapsBuilder SetCaps::SetCaps(WebAudioPlayerContext &context, std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, const std::string &audioMimeType, std::weak_ptr webAudioConfig) - : m_context{context}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_audioMimeType{audioMimeType} + : m_context{context}, m_gstWrapper{std::move(gstWrapper)}, m_glibWrapper{std::move(glibWrapper)}, m_audioMimeType{audioMimeType} { RIALTO_SERVER_LOG_DEBUG("Constructing SetCaps"); diff --git a/media/server/gstplayer/source/tasks/webAudio/SetVolume.cpp b/media/server/gstplayer/source/tasks/webAudio/SetVolume.cpp index 88c94bfe6..09f54d71b 100644 --- a/media/server/gstplayer/source/tasks/webAudio/SetVolume.cpp +++ b/media/server/gstplayer/source/tasks/webAudio/SetVolume.cpp @@ -26,7 +26,7 @@ namespace firebolt::rialto::server::tasks::webaudio { SetVolume::SetVolume(WebAudioPlayerContext &context, std::shared_ptr gstWrapper, double volume) - : m_context{context}, m_gstWrapper{gstWrapper}, m_volume{volume} + : m_context{context}, m_gstWrapper{std::move(gstWrapper)}, m_volume{volume} { RIALTO_SERVER_LOG_DEBUG("Constructing SetVolume"); } diff --git a/media/server/gstplayer/source/tasks/webAudio/WriteBuffer.cpp b/media/server/gstplayer/source/tasks/webAudio/WriteBuffer.cpp index 8d7aebd32..cb1ad0221 100644 --- a/media/server/gstplayer/source/tasks/webAudio/WriteBuffer.cpp +++ b/media/server/gstplayer/source/tasks/webAudio/WriteBuffer.cpp @@ -29,7 +29,7 @@ namespace firebolt::rialto::server::tasks::webaudio WriteBuffer::WriteBuffer(WebAudioPlayerContext &context, std::shared_ptr gstWrapper, uint8_t *mainPtr, uint32_t mainLength, uint8_t *wrapPtr, uint32_t wrapLength) - : m_context{context}, m_gstWrapper{gstWrapper}, m_mainPtr{mainPtr}, m_mainLength{mainLength}, m_wrapPtr{wrapPtr}, + : m_context{context}, m_gstWrapper{std::move(gstWrapper)}, m_mainPtr{mainPtr}, m_mainLength{mainLength}, m_wrapPtr{wrapPtr}, m_wrapLength{wrapLength} { RIALTO_SERVER_LOG_DEBUG("Constructing WriteBuffer"); diff --git a/media/server/ipc/source/MediaPipelineModuleService.cpp b/media/server/ipc/source/MediaPipelineModuleService.cpp index d6c3a1ac3..eccca95a9 100644 --- a/media/server/ipc/source/MediaPipelineModuleService.cpp +++ b/media/server/ipc/source/MediaPipelineModuleService.cpp @@ -471,8 +471,8 @@ void MediaPipelineModuleService::attachSource(::google::protobuf::RpcController { framed = kConfig.framed(); } - AudioConfig audioConfig{numberofchannels, sampleRate, codecSpecificConfig, format, - layout, channelMask, streamHeaders, framed}; + AudioConfig audioConfig{numberofchannels, sampleRate, std::move(codecSpecificConfig), format, + layout, channelMask, std::move(streamHeaders), framed}; mediaSource = std::make_unique(request->mime_type(), hasDrm, audioConfig, diff --git a/media/server/main/source/MainThread.cpp b/media/server/main/source/MainThread.cpp index 30c2661d0..f799b0f0c 100644 --- a/media/server/main/source/MainThread.cpp +++ b/media/server/main/source/MainThread.cpp @@ -140,10 +140,10 @@ void MainThread::enqueueTask(uint32_t clientId, Task task) { std::shared_ptr newTask = std::make_shared(); newTask->clientId = clientId; - newTask->task = task; + newTask->task = std::move(task); { std::unique_lock lock(m_taskQueueMutex); - m_taskQueue.push_back(newTask); + m_taskQueue.push_back(std::move(newTask)); } m_taskQueueCv.notify_one(); } @@ -152,7 +152,7 @@ void MainThread::enqueueTaskAndWait(uint32_t clientId, Task task) { std::shared_ptr newTask = std::make_shared(); newTask->clientId = clientId; - newTask->task = task; + newTask->task = std::move(task); newTask->mutex = std::make_unique(); newTask->cv = std::make_unique(); @@ -172,7 +172,7 @@ void MainThread::enqueuePriorityTaskAndWait(uint32_t clientId, Task task) { std::shared_ptr newTask = std::make_shared(); newTask->clientId = clientId; - newTask->task = task; + newTask->task = std::move(task); newTask->mutex = std::make_unique(); newTask->cv = std::make_unique(); diff --git a/media/server/main/source/MediaKeySession.cpp b/media/server/main/source/MediaKeySession.cpp index 414856cd8..11016a4e9 100644 --- a/media/server/main/source/MediaKeySession.cpp +++ b/media/server/main/source/MediaKeySession.cpp @@ -387,7 +387,7 @@ void MediaKeySession::onProcessChallenge(const char url[], const uint8_t challen { std::string urlStr = url; std::vector challengeVec = std::vector{challenge, challenge + challengeLength}; - auto task = [&, urlStr, challengeVec]() + auto task = [&, urlStr = std::move(urlStr), challengeVec = std::move(challengeVec)]() { std::shared_ptr client = m_mediaKeysClient.lock(); if (client) @@ -409,7 +409,7 @@ void MediaKeySession::onProcessChallenge(const char url[], const uint8_t challen void MediaKeySession::onKeyUpdated(const uint8_t keyId[], const uint8_t keyIdLength) { std::vector keyIdVec = std::vector{keyId, keyId + keyIdLength}; - auto task = [&, keyIdVec]() + auto task = [&, keyIdVec = std::move(keyIdVec)]() { std::shared_ptr client = m_mediaKeysClient.lock(); if (client) diff --git a/media/server/main/source/MediaKeysCapabilities.cpp b/media/server/main/source/MediaKeysCapabilities.cpp index f0ad87a81..fda6de8a0 100644 --- a/media/server/main/source/MediaKeysCapabilities.cpp +++ b/media/server/main/source/MediaKeysCapabilities.cpp @@ -93,7 +93,7 @@ namespace firebolt::rialto::server { MediaKeysCapabilities::MediaKeysCapabilities(std::shared_ptr ocdmFactory, std::shared_ptr ocdmSystemFactory) - : m_ocdmSystemFactory{ocdmSystemFactory} + : m_ocdmSystemFactory{std::move(ocdmSystemFactory)} { RIALTO_SERVER_LOG_DEBUG("entry:"); diff --git a/media/server/main/source/MediaKeysServerInternal.cpp b/media/server/main/source/MediaKeysServerInternal.cpp index 7fd1b58bf..526761f6c 100644 --- a/media/server/main/source/MediaKeysServerInternal.cpp +++ b/media/server/main/source/MediaKeysServerInternal.cpp @@ -105,7 +105,7 @@ MediaKeysServerInternal::MediaKeysServerInternal( const std::string &keySystem, const std::shared_ptr &mainThreadFactory, std::shared_ptr ocdmSystemFactory, std::shared_ptr mediaKeySessionFactory) - : m_mediaKeySessionFactory(mediaKeySessionFactory), m_kKeySystem(keySystem) + : m_mediaKeySessionFactory(std::move(mediaKeySessionFactory)), m_kKeySystem(keySystem) { RIALTO_SERVER_LOG_DEBUG("entry:"); diff --git a/media/server/main/source/MediaPipelineCapabilities.cpp b/media/server/main/source/MediaPipelineCapabilities.cpp index 67cd7c19a..e190b5e60 100644 --- a/media/server/main/source/MediaPipelineCapabilities.cpp +++ b/media/server/main/source/MediaPipelineCapabilities.cpp @@ -62,7 +62,7 @@ std::unique_ptr MediaPipelineCapabilitiesFactory::cr namespace firebolt::rialto::server { MediaPipelineCapabilities::MediaPipelineCapabilities(std::shared_ptr gstCapabilitiesFactory) - : m_kGstCapabilitiesFactory{gstCapabilitiesFactory} + : m_kGstCapabilitiesFactory{std::move(gstCapabilitiesFactory)} { RIALTO_SERVER_LOG_DEBUG("entry:"); diff --git a/media/server/main/source/MediaPipelineServerInternal.cpp b/media/server/main/source/MediaPipelineServerInternal.cpp index f76396bec..814181e52 100644 --- a/media/server/main/source/MediaPipelineServerInternal.cpp +++ b/media/server/main/source/MediaPipelineServerInternal.cpp @@ -134,9 +134,9 @@ MediaPipelineServerInternal::MediaPipelineServerInternal( const std::shared_ptr &shmBuffer, const std::shared_ptr &mainThreadFactory, std::shared_ptr timerFactory, std::unique_ptr &&dataReaderFactory, std::unique_ptr &&activeRequests, IDecryptionService &decryptionService) - : m_mediaPipelineClient(client), m_kGstPlayerFactory(gstPlayerFactory), m_kVideoRequirements(videoRequirements), + : m_mediaPipelineClient(std::move(client)), m_kGstPlayerFactory(gstPlayerFactory), m_kVideoRequirements(videoRequirements), m_sessionId{sessionId}, m_shmBuffer{shmBuffer}, m_dataReaderFactory{std::move(dataReaderFactory)}, - m_timerFactory{timerFactory}, m_activeRequests{std::move(activeRequests)}, m_decryptionService{decryptionService}, + m_timerFactory{std::move(timerFactory)}, m_activeRequests{std::move(activeRequests)}, m_decryptionService{decryptionService}, m_currentPlaybackState{PlaybackState::UNKNOWN}, m_wasAllSourcesAttachedCalled{false} { RIALTO_SERVER_LOG_DEBUG("entry:"); diff --git a/media/server/service/source/CdmService.cpp b/media/server/service/source/CdmService.cpp index 8ea716f61..f435916cb 100644 --- a/media/server/service/source/CdmService.cpp +++ b/media/server/service/source/CdmService.cpp @@ -31,7 +31,7 @@ namespace firebolt::rialto::server::service { CdmService::CdmService(std::shared_ptr &&mediaKeysFactory, std::shared_ptr &&mediaKeysCapabilitiesFactory) - : m_mediaKeysFactory{mediaKeysFactory}, m_mediaKeysCapabilitiesFactory{mediaKeysCapabilitiesFactory}, + : m_mediaKeysFactory{std::move(mediaKeysFactory)}, m_mediaKeysCapabilitiesFactory{std::move(mediaKeysCapabilitiesFactory)}, m_isActive{false} { RIALTO_SERVER_LOG_DEBUG("CdmService is constructed"); diff --git a/media/server/service/source/MediaPipelineService.cpp b/media/server/service/source/MediaPipelineService.cpp index 5e7c40063..e26a82c3b 100644 --- a/media/server/service/source/MediaPipelineService.cpp +++ b/media/server/service/source/MediaPipelineService.cpp @@ -32,7 +32,7 @@ MediaPipelineService::MediaPipelineService( IPlaybackService &playbackService, std::shared_ptr &&mediaPipelineFactory, std::shared_ptr &&mediaPipelineCapabilitiesFactory, IDecryptionService &decryptionService) - : m_playbackService{playbackService}, m_mediaPipelineFactory{mediaPipelineFactory}, + : m_playbackService{playbackService}, m_mediaPipelineFactory{std::move(mediaPipelineFactory)}, m_mediaPipelineCapabilities{mediaPipelineCapabilitiesFactory->createMediaPipelineCapabilities()}, m_decryptionService{decryptionService} { diff --git a/media/server/service/source/WebAudioPlayerService.cpp b/media/server/service/source/WebAudioPlayerService.cpp index e2182c201..b3831e63e 100644 --- a/media/server/service/source/WebAudioPlayerService.cpp +++ b/media/server/service/source/WebAudioPlayerService.cpp @@ -34,7 +34,7 @@ namespace firebolt::rialto::server::service { WebAudioPlayerService::WebAudioPlayerService(IPlaybackService &playbackService, std::shared_ptr &&webAudioPlayerFactory) - : m_playbackService{playbackService}, m_webAudioPlayerFactory{webAudioPlayerFactory} + : m_playbackService{playbackService}, m_webAudioPlayerFactory{std::move(webAudioPlayerFactory)} { RIALTO_SERVER_LOG_DEBUG("WebAudioPlayerService is constructed"); } diff --git a/serverManager/serverManagerSim/HttpRequest.cpp b/serverManager/serverManagerSim/HttpRequest.cpp index ba2bfb50c..65d1da8a4 100644 --- a/serverManager/serverManagerSim/HttpRequest.cpp +++ b/serverManager/serverManagerSim/HttpRequest.cpp @@ -31,13 +31,13 @@ std::vector splitUri(std::string uri) const std::string token = uri.substr(0, pos); if (!token.empty()) { - result.push_back(token); + result.push_back(std::move(token)); } uri.erase(0, pos + 1); } if (!uri.empty()) { - result.push_back(uri); + result.push_back(std::move(uri)); } return result; } From 3541e2c51778d9eaa9c7faa6fde0cca7aeee9d98 Mon Sep 17 00:00:00 2001 From: Muthu Shiam Sankar N Date: Mon, 22 Dec 2025 10:34:53 +0530 Subject: [PATCH 3/4] To clear Coverity warnings Summary: To clear Coverity warnings Type: Fix Test Plan: UT/CT, Fullstack Jira: ENTDAI-1354/RDKECOREMW-947 --- common/source/EventThread.cpp | 4 +-- common/source/Timer.cpp | 10 ++++-- ipc/client/source/IpcChannelImpl.cpp | 11 ------- ipc/server/source/IpcServerImpl.cpp | 36 ++++++++++++---------- media/client/main/source/MediaPipeline.cpp | 8 ++++- 5 files changed, 37 insertions(+), 32 deletions(-) diff --git a/common/source/EventThread.cpp b/common/source/EventThread.cpp index 1ad60188b..7c6da9a16 100644 --- a/common/source/EventThread.cpp +++ b/common/source/EventThread.cpp @@ -54,13 +54,13 @@ EventThread::EventThread(std::string threadName) : m_kThreadName(std::move(threa EventThread::~EventThread() { + { std::unique_lock locker(m_lock); m_shutdown = true; m_cond.notify_all(); - - locker.unlock(); + } if (m_thread.joinable()) m_thread.join(); diff --git a/common/source/Timer.cpp b/common/source/Timer.cpp index ae109a692..a7a8bbce5 100644 --- a/common/source/Timer.cpp +++ b/common/source/Timer.cpp @@ -59,15 +59,21 @@ Timer::Timer(const std::chrono::milliseconds &timeout, const std::function lock{m_mutex}; if (!m_cv.wait_for(lock, m_timeout, [this]() { return !m_active; })) { if (m_active && m_callback) { - lock.unlock(); - m_callback(); + Callback = true; } } + } + if (Callback) + { + m_callback(); + } } while (timerType == TimerType::PERIODIC && m_active); m_active = false; }); diff --git a/ipc/client/source/IpcChannelImpl.cpp b/ipc/client/source/IpcChannelImpl.cpp index 85c6fea52..7b068b2b6 100644 --- a/ipc/client/source/IpcChannelImpl.cpp +++ b/ipc/client/source/IpcChannelImpl.cpp @@ -590,9 +590,6 @@ void ChannelImpl::processTimeoutEvent() updateTimeoutTimer(); } - // drop the lock and now terminate the timed out method calls - locker.unlock(); - for (auto &call : timedOuts) { completeWithError(&call, "Timed out"); @@ -736,9 +733,6 @@ void ChannelImpl::processReplyFromServer(const transport::MethodCallReply &reply // update the timeout timer now a method call has been processed updateTimeoutTimer(); - // can now drop the lock - locker.unlock(); - // this is an actual reply so try and read it if (!methodCall.response->ParseFromString(reply.reply_message())) { @@ -787,9 +781,6 @@ void ChannelImpl::processErrorFromServer(const transport::MethodCallError &error // update the timeout timer now a method call has been processed updateTimeoutTimer(); - // can now drop the lock - locker.unlock(); - RIALTO_IPC_LOG_DEBUG("error{ serial %" PRIu64 " } - %s", kSerialId, error.error_reason().c_str()); // complete the call with an error @@ -1162,12 +1153,10 @@ void ChannelImpl::CallMethod(const google::protobuf::MethodDescriptor *method, / if (m_sock < 0) { - locker.unlock(); completeWithError(&methodCall, "Not connected"); } else if (sendmsg(m_sock, header, MSG_NOSIGNAL) != static_cast(kRequiredDataLen)) { - locker.unlock(); completeWithError(&methodCall, "Failed to send message"); } else diff --git a/ipc/server/source/IpcServerImpl.cpp b/ipc/server/source/IpcServerImpl.cpp index b6a9b3abb..096007db3 100644 --- a/ipc/server/source/IpcServerImpl.cpp +++ b/ipc/server/source/IpcServerImpl.cpp @@ -616,6 +616,11 @@ void ServerImpl::processNewConnection(uint64_t socketId) { RIALTO_IPC_LOG_DEBUG("processing new connection"); + int clientSock = 0; + std::string SockPath; + std::function &)> connectedCb; + std::function &)> disconnectedCb; + { std::unique_lock socketLocker(m_socketsLock); // find matching socket object @@ -632,21 +637,20 @@ void ServerImpl::processNewConnection(uint64_t socketId) struct sockaddr clientAddr = {0}; socklen_t clientAddrLen = sizeof(clientAddr); - int clientSock = accept4(kSocket.sockFd, &clientAddr, &clientAddrLen, SOCK_NONBLOCK | SOCK_CLOEXEC); + clientSock = accept4(kSocket.sockFd, &clientAddr, &clientAddrLen, SOCK_NONBLOCK | SOCK_CLOEXEC); if (clientSock < 0) { RIALTO_IPC_LOG_SYS_ERROR(errno, "failed to accept client connection"); return; } - const std::string kSockPath = kSocket.sockPath; - std::function &)> connectedCb = kSocket.connectedCb; - std::function &)> disconnectedCb = kSocket.disconnectedCb; - - socketLocker.unlock(); + SockPath = kSocket.sockPath; + connectedCb = kSocket.connectedCb; + disconnectedCb = kSocket.disconnectedCb; + } // attempt to add the socket to the client list - auto client = addClientSocket(clientSock, kSockPath, std::move(disconnectedCb)); + auto client = addClientSocket(clientSock, SockPath, std::move(disconnectedCb)); if (!client) { close(clientSock); @@ -736,6 +740,9 @@ static std::vector readMessageFds(const struct msghdr *msg, size */ void ServerImpl::processClientSocket(uint64_t clientId, unsigned events) { + int SockFd = 0; + std::shared_ptr clientObj = nullptr; + { // take the lock while accessing the client list std::unique_lock locker(m_clientsLock); @@ -754,13 +761,11 @@ void ServerImpl::processClientSocket(uint64_t clientId, unsigned events) } // get the socket that corresponds to the client connection - const int kSockFd = it->second.sock; + SockFd = it->second.sock; // get the client object - std::shared_ptr clientObj = it->second.client; - - // can safely release the lock now we have the clientId and client object - locker.unlock(); + clientObj = it->second.client; + } // if there was an error disconnect the socket if (events & EPOLLERR) @@ -786,7 +791,7 @@ void ServerImpl::processClientSocket(uint64_t clientId, unsigned events) msg.msg_controllen = sizeof(m_recvCtrlBuf); // read one message - ssize_t rd = TEMP_FAILURE_RETRY(recvmsg(kSockFd, &msg, MSG_CMSG_CLOEXEC)); + ssize_t rd = TEMP_FAILURE_RETRY(recvmsg(SockFd, &msg, MSG_CMSG_CLOEXEC)); if (rd < 0) { if (errno != EWOULDBLOCK) @@ -1303,9 +1308,10 @@ bool ServerImpl::isClientConnected(uint64_t clientId) const */ void ServerImpl::disconnectClient(uint64_t clientId) { + { std::unique_lock locker(m_clientsLock); m_condemnedClients.insert(clientId); - locker.unlock(); + } wakeEventLoop(); } @@ -1407,8 +1413,6 @@ bool ServerImpl::sendEvent(uint64_t clientId, const std::shared_ptrGetTypeName().c_str(), eventMessage->ShortDebugString().c_str()); diff --git a/media/client/main/source/MediaPipeline.cpp b/media/client/main/source/MediaPipeline.cpp index 661cbc0f4..024a1e980 100644 --- a/media/client/main/source/MediaPipeline.cpp +++ b/media/client/main/source/MediaPipeline.cpp @@ -545,12 +545,18 @@ bool MediaPipeline::flush(int32_t sourceId, bool resetTime, bool &async) { RIALTO_CLIENT_LOG_DEBUG("entry:"); + bool clearData = false; + { std::unique_lock flushLock{m_flushMutex}; if (m_mediaPipelineIpc->flush(sourceId, resetTime, async)) { m_attachedSources.setFlushing(sourceId, true); - flushLock.unlock(); + clearData = true; + } + } + if (clearData) + { // Clear all need datas for flushed source std::lock_guard lock{m_needDataRequestMapMutex}; for (auto it = m_needDataRequestMap.begin(); it != m_needDataRequestMap.end();) From 5f67e401e7093c4f2215bb8f34beac4a3b994e36 Mon Sep 17 00:00:00 2001 From: muthushiamsankar Date: Mon, 22 Dec 2025 13:07:53 +0530 Subject: [PATCH 4/4] Performance improvements in play request and flush (#428) (#431) Summary: Performance improvements in play request and flush Type: Fix Test Plan: UT/CT, Fullstack Jira: ENTDAI-1750 ENTDAI-1738 ENTDAI-485 Co-authored-by: Marcin Wojciechowski <105790697+skywojciechowskim@users.noreply.github.com> --- media/client/ipc/include/MediaPipelineIpc.h | 2 +- .../client/ipc/interface/IMediaPipelineIpc.h | 4 ++- media/client/ipc/source/MediaPipelineIpc.cpp | 4 ++- media/client/main/include/MediaPipeline.h | 2 +- .../client/main/include/MediaPipelineProxy.h | 2 +- media/client/main/source/MediaPipeline.cpp | 4 +-- media/public/include/IMediaPipeline.h | 7 ++-- .../gstplayer/include/GstGenericPlayer.h | 10 ++++-- .../include/IGstGenericPlayerPrivate.h | 4 +-- .../gstplayer/interface/IGstGenericPlayer.h | 11 +++--- .../gstplayer/source/GstGenericPlayer.cpp | 35 ++++++++++++++----- .../ipc/source/MediaPipelineModuleService.cpp | 4 ++- .../include/MediaPipelineServerInternal.h | 6 ++-- .../source/MediaPipelineServerInternal.cpp | 10 +++--- .../service/include/IMediaPipelineService.h | 2 +- .../service/source/MediaPipelineService.cpp | 4 +-- .../service/source/MediaPipelineService.h | 2 +- proto/mediapipelinemodule.proto | 4 ++- .../tests/base/MediaPipelineTestMethods.cpp | 6 ++-- .../ipc/mediaPipelineIpc/PlayPauseTest.cpp | 12 ++++--- .../mediaPipeline/MediaPipelineProxyTest.cpp | 5 +-- .../main/mediaPipeline/PlayPauseTest.cpp | 10 +++--- .../client/mocks/ipc/MediaPipelineIpcMock.h | 2 +- .../main/MediaPipelineAndControlClientMock.h | 2 +- .../genericPlayer/GstGenericPlayerTest.cpp | 31 ++++++++++++++-- .../common/GenericTasksTestsBase.cpp | 8 ++--- ...MediaPipelineModuleServiceTestsFixture.cpp | 4 +-- .../MiscellaneousFunctionsTest.cpp | 12 ++++--- .../base/MediaPipelineTestBase.cpp | 7 ++++ .../base/MediaPipelineTestBase.h | 1 + .../mocks/gstplayer/GstGenericPlayerMock.h | 2 +- .../gstplayer/GstGenericPlayerPrivateMock.h | 2 +- .../main/MediaPipelineServerInternalMock.h | 2 +- .../mocks/service/MediaPipelineServiceMock.h | 2 +- .../MediaPipelineServiceTestsFixture.cpp | 10 +++--- 35 files changed, 159 insertions(+), 76 deletions(-) diff --git a/media/client/ipc/include/MediaPipelineIpc.h b/media/client/ipc/include/MediaPipelineIpc.h index 5e50dd4f4..db45d00d9 100644 --- a/media/client/ipc/include/MediaPipelineIpc.h +++ b/media/client/ipc/include/MediaPipelineIpc.h @@ -79,7 +79,7 @@ class MediaPipelineIpc : public IMediaPipelineIpc, public IpcModule bool setVideoWindow(uint32_t x, uint32_t y, uint32_t width, uint32_t height) override; - bool play() override; + bool play(bool &async) override; bool pause() override; diff --git a/media/client/ipc/interface/IMediaPipelineIpc.h b/media/client/ipc/interface/IMediaPipelineIpc.h index 1d055d2b7..782a1931b 100644 --- a/media/client/ipc/interface/IMediaPipelineIpc.h +++ b/media/client/ipc/interface/IMediaPipelineIpc.h @@ -126,9 +126,11 @@ class IMediaPipelineIpc /** * @brief Request play on the playback session. * + * @param[out] async : True if play method call is asynchronous + * * @retval true on success. */ - virtual bool play() = 0; + virtual bool play(bool &async) = 0; /** * @brief Request pause on the playback session. diff --git a/media/client/ipc/source/MediaPipelineIpc.cpp b/media/client/ipc/source/MediaPipelineIpc.cpp index 23c0375da..ff1b09847 100644 --- a/media/client/ipc/source/MediaPipelineIpc.cpp +++ b/media/client/ipc/source/MediaPipelineIpc.cpp @@ -348,7 +348,7 @@ bool MediaPipelineIpc::setVideoWindow(uint32_t x, uint32_t y, uint32_t width, ui return true; } -bool MediaPipelineIpc::play() +bool MediaPipelineIpc::play(bool &async) { if (!reattachChannelIfRequired()) { @@ -375,6 +375,8 @@ bool MediaPipelineIpc::play() return false; } + async = response.async(); + return true; } diff --git a/media/client/main/include/MediaPipeline.h b/media/client/main/include/MediaPipeline.h index 74bf8d13b..13075a511 100644 --- a/media/client/main/include/MediaPipeline.h +++ b/media/client/main/include/MediaPipeline.h @@ -118,7 +118,7 @@ class MediaPipeline : public IMediaPipelineAndIControlClient, public IMediaPipel bool allSourcesAttached() override; - bool play() override; + bool play(bool &async) override; bool pause() override; diff --git a/media/client/main/include/MediaPipelineProxy.h b/media/client/main/include/MediaPipelineProxy.h index cf0283cd2..b38371963 100644 --- a/media/client/main/include/MediaPipelineProxy.h +++ b/media/client/main/include/MediaPipelineProxy.h @@ -52,7 +52,7 @@ class MediaPipelineProxy : public IMediaPipelineAndIControlClient bool allSourcesAttached() override { return m_mediaPipeline->allSourcesAttached(); } - bool play() override { return m_mediaPipeline->play(); } + bool play(bool &async) override { return m_mediaPipeline->play(async); } bool pause() override { return m_mediaPipeline->pause(); } diff --git a/media/client/main/source/MediaPipeline.cpp b/media/client/main/source/MediaPipeline.cpp index 024a1e980..a3ba3e465 100644 --- a/media/client/main/source/MediaPipeline.cpp +++ b/media/client/main/source/MediaPipeline.cpp @@ -252,11 +252,11 @@ bool MediaPipeline::allSourcesAttached() return m_mediaPipelineIpc->allSourcesAttached(); } -bool MediaPipeline::play() +bool MediaPipeline::play(bool &async) { RIALTO_CLIENT_LOG_DEBUG("entry:"); - return m_mediaPipelineIpc->play(); + return m_mediaPipelineIpc->play(async); } bool MediaPipeline::pause() diff --git a/media/public/include/IMediaPipeline.h b/media/public/include/IMediaPipeline.h index 789115750..7fa3aa892 100644 --- a/media/public/include/IMediaPipeline.h +++ b/media/public/include/IMediaPipeline.h @@ -1119,16 +1119,15 @@ class IMediaPipeline /** * @brief Starts playback of the media. * - * This method is considered to be asynchronous and MUST NOT block - * but should request playback and then return. - * * Once the backend is successfully playing it should notify the * media player client of playback state * IMediaPipelineClient::PlaybackState::PLAYING. * + * @param[out] async : True if play method call is asynchronous + * * @retval true on success. */ - virtual bool play() = 0; + virtual bool play(bool &async) = 0; /** * @brief Pauses playback of the media. diff --git a/media/server/gstplayer/include/GstGenericPlayer.h b/media/server/gstplayer/include/GstGenericPlayer.h index 22fd800e0..fd633cd48 100644 --- a/media/server/gstplayer/include/GstGenericPlayer.h +++ b/media/server/gstplayer/include/GstGenericPlayer.h @@ -36,6 +36,7 @@ #include "tasks/IGenericPlayerTaskFactory.h" #include "tasks/IPlayerTask.h" #include +#include #include #include #include @@ -108,7 +109,7 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva void attachSource(const std::unique_ptr &mediaSource) override; void removeSource(const MediaSourceType &mediaSourceType) override; void allSourcesAttached() override; - void play() override; + void play(bool &async) override; void pause() override; void stop() override; void attachSamples(const IMediaPipeline::MediaSegmentVector &mediaSegments) override; @@ -168,7 +169,7 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva void updateVideoCaps(int32_t width, int32_t height, Fraction frameRate, const std::shared_ptr &codecData) override; void addAudioClippingToBuffer(GstBuffer *buffer, uint64_t clippingStart, uint64_t clippingEnd) const override; - bool changePipelineState(GstState newState) override; + GstStateChangeReturn changePipelineState(GstState newState) override; int64_t getPosition(GstElement *element) override; void startPositionReportingAndCheckAudioUnderflowTimer() override; void stopPositionReportingAndCheckAudioUnderflowTimer() override; @@ -427,6 +428,11 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva * @brief The postponed flush tasks */ std::vector> m_postponedFlushes{}; + + /** + * @brief The ongoing state change operations counter + */ + std::atomic m_ongoingStateChangesNumber{0}; }; } // namespace firebolt::rialto::server diff --git a/media/server/gstplayer/include/IGstGenericPlayerPrivate.h b/media/server/gstplayer/include/IGstGenericPlayerPrivate.h index 1848c8960..7ffb26b2c 100644 --- a/media/server/gstplayer/include/IGstGenericPlayerPrivate.h +++ b/media/server/gstplayer/include/IGstGenericPlayerPrivate.h @@ -174,9 +174,9 @@ class IGstGenericPlayerPrivate * * @param[in] newState : The desired state. * - * @retval true on success. + * @retval state change status */ - virtual bool changePipelineState(GstState newState) = 0; + virtual GstStateChangeReturn changePipelineState(GstState newState) = 0; /** * @brief Gets the current position of the element diff --git a/media/server/gstplayer/interface/IGstGenericPlayer.h b/media/server/gstplayer/interface/IGstGenericPlayer.h index e01edd364..d07b56f81 100644 --- a/media/server/gstplayer/interface/IGstGenericPlayer.h +++ b/media/server/gstplayer/interface/IGstGenericPlayer.h @@ -105,14 +105,15 @@ class IGstGenericPlayer /** * @brief Starts playback of the media. * - * This method is considered to be asynchronous and MUST NOT block - * but should request playback and then return. - * * Once the backend is successfully playing it should notify the - * media player client of playback state PlaybackState::PLAYING. + * media player client of playback state + * IMediaPipelineClient::PlaybackState::PLAYING. * + * @param[out] async : True if play method call is asynchronous + * + * @retval true on success. */ - virtual void play() = 0; + virtual void play(bool &async) = 0; /** * @brief Pauses playback of the media. diff --git a/media/server/gstplayer/source/GstGenericPlayer.cpp b/media/server/gstplayer/source/GstGenericPlayer.cpp index 7e83f911a..cb15c5151 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -1186,16 +1186,32 @@ void GstGenericPlayer::cancelUnderflow(firebolt::rialto::MediaSourceType mediaSo } } -void GstGenericPlayer::play() +void GstGenericPlayer::play(bool &async) { - if (m_workerThread) + if (0 == m_ongoingStateChangesNumber) { - m_workerThread->enqueueTask(m_taskFactory->createPlay(*this)); + // Operation called on main thread, because PAUSED->PLAYING change is synchronous and needs to be done fast. + // + // m_context.pipeline can be used, because it's modified only in GstGenericPlayer + // constructor and destructor. GstGenericPlayer is created/destructed on main thread, so we won't have a crash here. + ++m_ongoingStateChangesNumber; + async = (changePipelineState(GST_STATE_PLAYING) == GST_STATE_CHANGE_ASYNC); + RIALTO_SERVER_LOG_MIL("State change to PLAYING requested"); + } + else + { + ++m_ongoingStateChangesNumber; + async = true; + if (m_workerThread) + { + m_workerThread->enqueueTask(m_taskFactory->createPlay(*this)); + } } } void GstGenericPlayer::pause() { + ++m_ongoingStateChangesNumber; if (m_workerThread) { m_workerThread->enqueueTask(m_taskFactory->createPause(m_context, *this)); @@ -1204,29 +1220,32 @@ void GstGenericPlayer::pause() void GstGenericPlayer::stop() { + ++m_ongoingStateChangesNumber; if (m_workerThread) { m_workerThread->enqueueTask(m_taskFactory->createStop(m_context, *this)); } } -bool GstGenericPlayer::changePipelineState(GstState newState) +GstStateChangeReturn GstGenericPlayer::changePipelineState(GstState newState) { if (!m_context.pipeline) { RIALTO_SERVER_LOG_ERROR("Change state failed - pipeline is nullptr"); if (m_gstPlayerClient) m_gstPlayerClient->notifyPlaybackState(PlaybackState::FAILURE); - return false; + --m_ongoingStateChangesNumber; + return GST_STATE_CHANGE_FAILURE; } - if (m_gstWrapper->gstElementSetState(m_context.pipeline, newState) == GST_STATE_CHANGE_FAILURE) + const GstStateChangeReturn result{m_gstWrapper->gstElementSetState(m_context.pipeline, newState)}; + if (result == GST_STATE_CHANGE_FAILURE) { RIALTO_SERVER_LOG_ERROR("Change state failed - Gstreamer returned an error"); if (m_gstPlayerClient) m_gstPlayerClient->notifyPlaybackState(PlaybackState::FAILURE); - return false; } - return true; + --m_ongoingStateChangesNumber; + return result; } int64_t GstGenericPlayer::getPosition(GstElement *element) diff --git a/media/server/ipc/source/MediaPipelineModuleService.cpp b/media/server/ipc/source/MediaPipelineModuleService.cpp index eccca95a9..8bc38bac8 100644 --- a/media/server/ipc/source/MediaPipelineModuleService.cpp +++ b/media/server/ipc/source/MediaPipelineModuleService.cpp @@ -566,11 +566,13 @@ void MediaPipelineModuleService::play(::google::protobuf::RpcController *control ::firebolt::rialto::PlayResponse *response, ::google::protobuf::Closure *done) { RIALTO_SERVER_LOG_DEBUG("entry:"); - if (!m_mediaPipelineService.play(request->session_id())) + bool async{false}; + if (!m_mediaPipelineService.play(request->session_id(), async)) { RIALTO_SERVER_LOG_ERROR("Play failed"); controller->SetFailed("Operation failed"); } + response->set_async(async); done->Run(); } diff --git a/media/server/main/include/MediaPipelineServerInternal.h b/media/server/main/include/MediaPipelineServerInternal.h index e546ee795..cfd0cd2a7 100644 --- a/media/server/main/include/MediaPipelineServerInternal.h +++ b/media/server/main/include/MediaPipelineServerInternal.h @@ -99,7 +99,7 @@ class MediaPipelineServerInternal : public IMediaPipelineServerInternal, public bool allSourcesAttached() override; - bool play() override; + bool play(bool &async) override; bool pause() override; @@ -346,9 +346,11 @@ class MediaPipelineServerInternal : public IMediaPipelineServerInternal, public /** * @brief Play internally, only to be called on the main thread. * + * @param[out] async : True if play method call is asynchronous + * * @retval true on success. */ - bool playInternal(); + bool playInternal(bool &async); /** * @brief Pause internally, only to be called on the main thread. diff --git a/media/server/main/source/MediaPipelineServerInternal.cpp b/media/server/main/source/MediaPipelineServerInternal.cpp index 814181e52..d4e6c57df 100644 --- a/media/server/main/source/MediaPipelineServerInternal.cpp +++ b/media/server/main/source/MediaPipelineServerInternal.cpp @@ -335,18 +335,18 @@ bool MediaPipelineServerInternal::allSourcesAttachedInternal() return true; } -bool MediaPipelineServerInternal::play() +bool MediaPipelineServerInternal::play(bool &async) { RIALTO_SERVER_LOG_DEBUG("entry:"); bool result; - auto task = [&]() { result = playInternal(); }; + auto task = [&]() { result = playInternal(async); }; - m_mainThread->enqueueTaskAndWait(m_mainThreadClientId, task); + m_mainThread->enqueuePriorityTaskAndWait(m_mainThreadClientId, task); return result; } -bool MediaPipelineServerInternal::playInternal() +bool MediaPipelineServerInternal::playInternal(bool &async) { if (!m_gstPlayer) { @@ -354,7 +354,7 @@ bool MediaPipelineServerInternal::playInternal() return false; } - m_gstPlayer->play(); + m_gstPlayer->play(async); return true; } diff --git a/media/server/service/include/IMediaPipelineService.h b/media/server/service/include/IMediaPipelineService.h index 7ba9e8a1c..ae1fc5e04 100644 --- a/media/server/service/include/IMediaPipelineService.h +++ b/media/server/service/include/IMediaPipelineService.h @@ -48,7 +48,7 @@ class IMediaPipelineService virtual bool attachSource(int sessionId, const std::unique_ptr &source) = 0; virtual bool removeSource(int sessionId, std::int32_t sourceId) = 0; virtual bool allSourcesAttached(int sessionId) = 0; - virtual bool play(int sessionId) = 0; + virtual bool play(int sessionId, bool &async) = 0; virtual bool pause(int sessionId) = 0; virtual bool stop(int sessionId) = 0; virtual bool setPlaybackRate(int sessionId, double rate) = 0; diff --git a/media/server/service/source/MediaPipelineService.cpp b/media/server/service/source/MediaPipelineService.cpp index e26a82c3b..e977c296f 100644 --- a/media/server/service/source/MediaPipelineService.cpp +++ b/media/server/service/source/MediaPipelineService.cpp @@ -169,7 +169,7 @@ bool MediaPipelineService::allSourcesAttached(int sessionId) return mediaPipelineIter->second->allSourcesAttached(); } -bool MediaPipelineService::play(int sessionId) +bool MediaPipelineService::play(int sessionId, bool &async) { RIALTO_SERVER_LOG_INFO("MediaPipelineService requested to play, session id: %d", sessionId); @@ -180,7 +180,7 @@ bool MediaPipelineService::play(int sessionId) RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId); return false; } - return mediaPipelineIter->second->play(); + return mediaPipelineIter->second->play(async); } bool MediaPipelineService::pause(int sessionId) diff --git a/media/server/service/source/MediaPipelineService.h b/media/server/service/source/MediaPipelineService.h index d68edadff..bf9ec234f 100644 --- a/media/server/service/source/MediaPipelineService.h +++ b/media/server/service/source/MediaPipelineService.h @@ -59,7 +59,7 @@ class MediaPipelineService : public IMediaPipelineService bool attachSource(int sessionId, const std::unique_ptr &source) override; bool removeSource(int sessionId, std::int32_t sourceId) override; bool allSourcesAttached(int sessionId) override; - bool play(int sessionId) override; + bool play(int sessionId, bool &async) override; bool pause(int sessionId) override; bool stop(int sessionId) override; bool setPlaybackRate(int sessionId, double rate) override; diff --git a/proto/mediapipelinemodule.proto b/proto/mediapipelinemodule.proto index f56a19970..1807810de 100644 --- a/proto/mediapipelinemodule.proto +++ b/proto/mediapipelinemodule.proto @@ -261,7 +261,8 @@ message SetVideoWindowResponse { * @fn void play(int session_id) * @brief Starts playback on a session. * - * @param[in] session_id The id of the A/V session. + * @param[in] session_id The id of the A/V session. + * @param[out] async True if play method call is asynchronous * * This method is asynchronous. Once the backend is successfully playing it will notify the media player client of * playback state. @@ -271,6 +272,7 @@ message PlayRequest { optional int32 session_id = 1 [default = -1]; } message PlayResponse { + optional bool async = 1 [default = true]; } /** diff --git a/tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp b/tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp index 51ef2b5d4..e91d67bbb 100644 --- a/tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp +++ b/tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp @@ -892,7 +892,8 @@ void MediaPipelineTestMethods::shouldNotifyPlaybackStateFailure() void MediaPipelineTestMethods::playFailure() { - EXPECT_EQ(m_mediaPipeline->play(), false); + bool async{false}; + EXPECT_EQ(m_mediaPipeline->play(async), false); } void MediaPipelineTestMethods::pauseFailure() @@ -2033,7 +2034,8 @@ void MediaPipelineTestMethods::haveDataInternal(const std::unique_ptr &mediaPipeline, const bool status) { - EXPECT_EQ(mediaPipeline->play(), status); + bool async{false}; + EXPECT_EQ(mediaPipeline->play(async), status); } void MediaPipelineTestMethods::sendNotifyPlaybackStateInternal(const int32_t sessionId, const PlaybackState &state) diff --git a/tests/unittests/media/client/ipc/mediaPipelineIpc/PlayPauseTest.cpp b/tests/unittests/media/client/ipc/mediaPipelineIpc/PlayPauseTest.cpp index 2db4edf0b..dec195144 100644 --- a/tests/unittests/media/client/ipc/mediaPipelineIpc/PlayPauseTest.cpp +++ b/tests/unittests/media/client/ipc/mediaPipelineIpc/PlayPauseTest.cpp @@ -43,12 +43,13 @@ class RialtoClientMediaPipelineIpcPlayPauseTest : public MediaPipelineIpcTestBas */ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlaySuccess) { + bool async{false}; expectIpcApiCallSuccess(); EXPECT_CALL(*m_channelMock, CallMethod(methodMatcher("play"), m_controllerMock.get(), playRequestMatcher(m_sessionId), _, m_blockingClosureMock.get())); - EXPECT_EQ(m_mediaPipelineIpc->play(), true); + EXPECT_EQ(m_mediaPipelineIpc->play(async), true); } /** @@ -56,10 +57,11 @@ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlaySuccess) */ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlayChannelDisconnected) { + bool async{false}; expectIpcApiCallDisconnected(); expectUnsubscribeEvents(); - EXPECT_EQ(m_mediaPipelineIpc->play(), false); + EXPECT_EQ(m_mediaPipelineIpc->play(async), false); // Reattach channel on destroySession EXPECT_CALL(*m_ipcClientMock, getChannel()).WillOnce(Return(m_channelMock)).RetiresOnSaturation(); @@ -71,13 +73,14 @@ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlayChannelDisconnected) */ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlayReconnectChannel) { + bool async{false}; expectIpcApiCallReconnected(); expectUnsubscribeEvents(); expectSubscribeEvents(); EXPECT_CALL(*m_channelMock, CallMethod(methodMatcher("play"), _, _, _, _)); - EXPECT_EQ(m_mediaPipelineIpc->play(), true); + EXPECT_EQ(m_mediaPipelineIpc->play(async), true); } /** @@ -85,11 +88,12 @@ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlayReconnectChannel) */ TEST_F(RialtoClientMediaPipelineIpcPlayPauseTest, PlayFailure) { + bool async{false}; expectIpcApiCallFailure(); EXPECT_CALL(*m_channelMock, CallMethod(methodMatcher("play"), _, _, _, _)); - EXPECT_EQ(m_mediaPipelineIpc->play(), false); + EXPECT_EQ(m_mediaPipelineIpc->play(async), false); } /** diff --git a/tests/unittests/media/client/main/mediaPipeline/MediaPipelineProxyTest.cpp b/tests/unittests/media/client/main/mediaPipeline/MediaPipelineProxyTest.cpp index be98cad21..6e22bdf4d 100644 --- a/tests/unittests/media/client/main/mediaPipeline/MediaPipelineProxyTest.cpp +++ b/tests/unittests/media/client/main/mediaPipeline/MediaPipelineProxyTest.cpp @@ -100,8 +100,9 @@ TEST_F(RialtoClientMediaPipelineProxyTest, TestPassthrough) ///////////////////////////////////////////// - EXPECT_CALL(*mediaPipelineMock, play()).WillOnce(Return(true)); - EXPECT_TRUE(proxy->play()); + bool async{false}; + EXPECT_CALL(*mediaPipelineMock, play(_)).WillOnce(Return(true)); + EXPECT_TRUE(proxy->play(async)); ///////////////////////////////////////////// diff --git a/tests/unittests/media/client/main/mediaPipeline/PlayPauseTest.cpp b/tests/unittests/media/client/main/mediaPipeline/PlayPauseTest.cpp index a58b80891..8c78c550f 100644 --- a/tests/unittests/media/client/main/mediaPipeline/PlayPauseTest.cpp +++ b/tests/unittests/media/client/main/mediaPipeline/PlayPauseTest.cpp @@ -42,9 +42,10 @@ class RialtoClientMediaPipelinePlayPauseTest : public MediaPipelineTestBase */ TEST_F(RialtoClientMediaPipelinePlayPauseTest, PlaySuccess) { - EXPECT_CALL(*m_mediaPipelineIpcMock, play()).WillOnce(Return(true)); + bool async{false}; + EXPECT_CALL(*m_mediaPipelineIpcMock, play(_)).WillOnce(Return(true)); - EXPECT_EQ(m_mediaPipeline->play(), true); + EXPECT_EQ(m_mediaPipeline->play(async), true); } /** @@ -52,9 +53,10 @@ TEST_F(RialtoClientMediaPipelinePlayPauseTest, PlaySuccess) */ TEST_F(RialtoClientMediaPipelinePlayPauseTest, PlayFailure) { - EXPECT_CALL(*m_mediaPipelineIpcMock, play()).WillOnce(Return(false)); + bool async{false}; + EXPECT_CALL(*m_mediaPipelineIpcMock, play(_)).WillOnce(Return(false)); - EXPECT_EQ(m_mediaPipeline->play(), false); + EXPECT_EQ(m_mediaPipeline->play(async), false); } /** diff --git a/tests/unittests/media/client/mocks/ipc/MediaPipelineIpcMock.h b/tests/unittests/media/client/mocks/ipc/MediaPipelineIpcMock.h index ed99bfb8e..32e72ba2c 100644 --- a/tests/unittests/media/client/mocks/ipc/MediaPipelineIpcMock.h +++ b/tests/unittests/media/client/mocks/ipc/MediaPipelineIpcMock.h @@ -39,7 +39,7 @@ class MediaPipelineIpcMock : public IMediaPipelineIpc MOCK_METHOD(bool, allSourcesAttached, (), (override)); MOCK_METHOD(bool, load, (MediaType type, const std::string &mimeType, const std::string &url), (override)); MOCK_METHOD(bool, setVideoWindow, (uint32_t x, uint32_t y, uint32_t width, uint32_t height), (override)); - MOCK_METHOD(bool, play, (), (override)); + MOCK_METHOD(bool, play, (bool &async), (override)); MOCK_METHOD(bool, pause, (), (override)); MOCK_METHOD(bool, stop, (), (override)); MOCK_METHOD(bool, haveData, (MediaSourceStatus status, uint32_t numFrames, uint32_t requestId), (override)); diff --git a/tests/unittests/media/client/mocks/main/MediaPipelineAndControlClientMock.h b/tests/unittests/media/client/mocks/main/MediaPipelineAndControlClientMock.h index 0d1427fd4..c69140210 100644 --- a/tests/unittests/media/client/mocks/main/MediaPipelineAndControlClientMock.h +++ b/tests/unittests/media/client/mocks/main/MediaPipelineAndControlClientMock.h @@ -39,7 +39,7 @@ class MediaPipelineAndControlClientMock : public IMediaPipelineAndIControlClient MOCK_METHOD(bool, allSourcesAttached, (), (override)); - MOCK_METHOD(bool, play, (), (override)); + MOCK_METHOD(bool, play, (bool &async), (override)); MOCK_METHOD(bool, pause, (), (override)); MOCK_METHOD(bool, stop, (), (override)); diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp index b2c5158b6..97a5e0df6 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp @@ -165,13 +165,40 @@ TEST_F(GstGenericPlayerTest, shouldAllSourcesAttached) m_sut->allSourcesAttached(); } -TEST_F(GstGenericPlayerTest, shouldPlay) +TEST_F(GstGenericPlayerTest, shouldPlayOnWorkerThread) { + // Pause first + std::unique_ptr pauseTask{std::make_unique>()}; + EXPECT_CALL(dynamic_cast &>(*pauseTask), execute()); + EXPECT_CALL(m_taskFactoryMock, createPause(_, _)).WillOnce(Return(ByMove(std::move(pauseTask)))); + + m_sut->pause(); + + // ... + + bool async = false; std::unique_ptr task{std::make_unique>()}; EXPECT_CALL(dynamic_cast &>(*task), execute()); EXPECT_CALL(m_taskFactoryMock, createPlay(_)).WillOnce(Return(ByMove(std::move(task)))); - m_sut->play(); + m_sut->play(async); + EXPECT_TRUE(async); +} + +TEST_F(GstGenericPlayerTest, shouldPlayImmediatelySynchronously) +{ + bool async = false; + EXPECT_CALL(*m_gstWrapperMock, gstElementSetState(_, GST_STATE_PLAYING)).WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); + m_sut->play(async); + EXPECT_FALSE(async); +} + +TEST_F(GstGenericPlayerTest, shouldPlayImmediatelyAsynchronously) +{ + bool async = false; + EXPECT_CALL(*m_gstWrapperMock, gstElementSetState(_, GST_STATE_PLAYING)).WillOnce(Return(GST_STATE_CHANGE_ASYNC)); + m_sut->play(async); + EXPECT_TRUE(async); } TEST_F(GstGenericPlayerTest, shouldPause) diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp index f40976337..87ddcf090 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp @@ -2273,7 +2273,7 @@ void GenericTasksTestsBase::shouldStopGstPlayer() videoStreamIt->second.isDataNeeded = true; audioStreamIt->second.isDataNeeded = true; EXPECT_CALL(testContext->m_gstPlayer, stopPositionReportingAndCheckAudioUnderflowTimer()); - EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_NULL)); + EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_NULL)).WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); } void GenericTasksTestsBase::triggerStop() @@ -2589,12 +2589,12 @@ void GenericTasksTestsBase::checkNoEos() void GenericTasksTestsBase::shouldChangeStatePlayingSuccess() { - EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PLAYING)).WillOnce(Return(true)); + EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PLAYING)).WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); } void GenericTasksTestsBase::shouldChangeStatePlayingFailure() { - EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PLAYING)).WillOnce(Return(false)); + EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PLAYING)).WillOnce(Return(GST_STATE_CHANGE_FAILURE)); } void GenericTasksTestsBase::triggerPlay() @@ -2612,7 +2612,7 @@ void GenericTasksTestsBase::triggerPing() void GenericTasksTestsBase::shouldPause() { EXPECT_CALL(testContext->m_gstPlayer, stopPositionReportingAndCheckAudioUnderflowTimer()); - EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PAUSED)); + EXPECT_CALL(testContext->m_gstPlayer, changePipelineState(GST_STATE_PAUSED)).WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); } void GenericTasksTestsBase::triggerPause() diff --git a/tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp b/tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp index ac3576a74..31ec98768 100644 --- a/tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp +++ b/tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp @@ -388,13 +388,13 @@ void MediaPipelineModuleServiceTests::mediaPipelineServiceWillFailAllSourcesAtta void MediaPipelineModuleServiceTests::mediaPipelineServiceWillPlay() { expectRequestSuccess(); - EXPECT_CALL(m_mediaPipelineServiceMock, play(kHardcodedSessionId)).WillOnce(Return(true)); + EXPECT_CALL(m_mediaPipelineServiceMock, play(kHardcodedSessionId, _)).WillOnce(Return(true)); } void MediaPipelineModuleServiceTests::mediaPipelineServiceWillFailToPlay() { expectRequestFailure(); - EXPECT_CALL(m_mediaPipelineServiceMock, play(kHardcodedSessionId)).WillOnce(Return(false)); + EXPECT_CALL(m_mediaPipelineServiceMock, play(kHardcodedSessionId, _)).WillOnce(Return(false)); } void MediaPipelineModuleServiceTests::mediaPipelineServiceWillPause() diff --git a/tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp b/tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp index ce00bdee8..aa4f3e2f3 100644 --- a/tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp +++ b/tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp @@ -43,11 +43,12 @@ class RialtoServerMediaPipelineMiscellaneousFunctionsTest : public MediaPipeline */ TEST_F(RialtoServerMediaPipelineMiscellaneousFunctionsTest, PlaySuccess) { + bool async{false}; loadGstPlayer(); - mainThreadWillEnqueueTaskAndWait(); + mainThreadWillEnqueuePriorityTaskAndWait(); - EXPECT_CALL(*m_gstPlayerMock, play()); - EXPECT_TRUE(m_mediaPipeline->play()); + EXPECT_CALL(*m_gstPlayerMock, play(_)); + EXPECT_TRUE(m_mediaPipeline->play(async)); } /** @@ -55,8 +56,9 @@ TEST_F(RialtoServerMediaPipelineMiscellaneousFunctionsTest, PlaySuccess) */ TEST_F(RialtoServerMediaPipelineMiscellaneousFunctionsTest, PlayFailureDueToUninitializedPlayer) { - mainThreadWillEnqueueTaskAndWait(); - EXPECT_FALSE(m_mediaPipeline->play()); + bool async{false}; + mainThreadWillEnqueuePriorityTaskAndWait(); + EXPECT_FALSE(m_mediaPipeline->play(async)); } /** diff --git a/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp b/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp index 491aec0e1..1543184af 100644 --- a/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp +++ b/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.cpp @@ -82,6 +82,13 @@ void MediaPipelineTestBase::mainThreadWillEnqueueTaskAndWait() .RetiresOnSaturation(); } +void MediaPipelineTestBase::mainThreadWillEnqueuePriorityTaskAndWait() +{ + EXPECT_CALL(*m_mainThreadMock, enqueuePriorityTaskAndWait(m_kMainThreadClientId, _)) + .WillOnce(Invoke([](uint32_t clientId, firebolt::rialto::server::IMainThread::Task task) { task(); })) + .RetiresOnSaturation(); +} + void MediaPipelineTestBase::loadGstPlayer() { mainThreadWillEnqueueTaskAndWait(); diff --git a/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.h b/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.h index d31474d53..893353141 100644 --- a/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.h +++ b/tests/unittests/media/server/main/mediaPipeline/base/MediaPipelineTestBase.h @@ -90,6 +90,7 @@ class MediaPipelineTestBase : public ::testing::Test void destroyMediaPipeline(); void mainThreadWillEnqueueTask(); void mainThreadWillEnqueueTaskAndWait(); + void mainThreadWillEnqueuePriorityTaskAndWait(); void loadGstPlayer(); int attachSource(MediaSourceType sourceType, const std::string &mimeType); void setEos(MediaSourceType sourceType); diff --git a/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h b/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h index de4697646..bc2c3f07f 100644 --- a/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h +++ b/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h @@ -36,7 +36,7 @@ class GstGenericPlayerMock : public IGstGenericPlayer MOCK_METHOD(void, attachSource, (const std::unique_ptr &mediaSource), (override)); MOCK_METHOD(void, removeSource, (const MediaSourceType &mediaSourceType), (override)); MOCK_METHOD(void, allSourcesAttached, (), (override)); - MOCK_METHOD(void, play, (), (override)); + MOCK_METHOD(void, play, (bool &async), (override)); MOCK_METHOD(void, pause, (), (override)); MOCK_METHOD(void, stop, (), (override)); MOCK_METHOD(void, attachSamples, (const IMediaPipeline::MediaSegmentVector &mediaSegments), (override)); diff --git a/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h b/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h index ceab057ff..e1f4b3efb 100644 --- a/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h +++ b/tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h @@ -55,7 +55,7 @@ class GstGenericPlayerPrivateMock : public IGstGenericPlayerPrivate MOCK_METHOD(void, updateVideoCaps, (int32_t width, int32_t height, Fraction frameRate, const std::shared_ptr &codecData), (override)); - MOCK_METHOD(bool, changePipelineState, (GstState newState), (override)); + MOCK_METHOD(GstStateChangeReturn, changePipelineState, (GstState newState), (override)); MOCK_METHOD(int64_t, getPosition, (GstElement * element), (override)); MOCK_METHOD(void, startPositionReportingAndCheckAudioUnderflowTimer, (), (override)); MOCK_METHOD(void, stopPositionReportingAndCheckAudioUnderflowTimer, (), (override)); diff --git a/tests/unittests/media/server/mocks/main/MediaPipelineServerInternalMock.h b/tests/unittests/media/server/mocks/main/MediaPipelineServerInternalMock.h index 61ee32b61..27a12a686 100644 --- a/tests/unittests/media/server/mocks/main/MediaPipelineServerInternalMock.h +++ b/tests/unittests/media/server/mocks/main/MediaPipelineServerInternalMock.h @@ -36,7 +36,7 @@ class MediaPipelineServerInternalMock : public IMediaPipelineServerInternal MOCK_METHOD(bool, attachSource, (const std::unique_ptr &source), (override)); MOCK_METHOD(bool, removeSource, (int32_t id), (override)); MOCK_METHOD(bool, allSourcesAttached, (), (override)); - MOCK_METHOD(bool, play, (), (override)); + MOCK_METHOD(bool, play, (bool &async), (override)); MOCK_METHOD(bool, pause, (), (override)); MOCK_METHOD(bool, stop, (), (override)); MOCK_METHOD(bool, setPlaybackRate, (double rate), (override)); diff --git a/tests/unittests/media/server/mocks/service/MediaPipelineServiceMock.h b/tests/unittests/media/server/mocks/service/MediaPipelineServiceMock.h index 69812124d..f7b226777 100644 --- a/tests/unittests/media/server/mocks/service/MediaPipelineServiceMock.h +++ b/tests/unittests/media/server/mocks/service/MediaPipelineServiceMock.h @@ -38,7 +38,7 @@ class MediaPipelineServiceMock : public IMediaPipelineService MOCK_METHOD(bool, attachSource, (int, const std::unique_ptr &), (override)); MOCK_METHOD(bool, removeSource, (int, std::int32_t), (override)); MOCK_METHOD(bool, allSourcesAttached, (int), (override)); - MOCK_METHOD(bool, play, (int), (override)); + MOCK_METHOD(bool, play, (int, bool &), (override)); MOCK_METHOD(bool, pause, (int), (override)); MOCK_METHOD(bool, stop, (int), (override)); MOCK_METHOD(bool, setPlaybackRate, (int, double), (override)); diff --git a/tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp b/tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp index 9b9e441dd..421cb1d8b 100644 --- a/tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp +++ b/tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp @@ -138,12 +138,12 @@ void MediaPipelineServiceTests::mediaPipelineWillFailToAllSourcesAttached() void MediaPipelineServiceTests::mediaPipelineWillPlay() { - EXPECT_CALL(m_mediaPipelineMock, play()).WillOnce(Return(true)); + EXPECT_CALL(m_mediaPipelineMock, play(_)).WillOnce(Return(true)); } void MediaPipelineServiceTests::mediaPipelineWillFailToPlay() { - EXPECT_CALL(m_mediaPipelineMock, play()).WillOnce(Return(false)); + EXPECT_CALL(m_mediaPipelineMock, play(_)).WillOnce(Return(false)); } void MediaPipelineServiceTests::mediaPipelineWillPause() @@ -624,12 +624,14 @@ void MediaPipelineServiceTests::allSourcesAttachedShouldFail() void MediaPipelineServiceTests::playShouldSucceed() { - EXPECT_TRUE(m_sut->play(kSessionId)); + bool isAsync{false}; + EXPECT_TRUE(m_sut->play(kSessionId, isAsync)); } void MediaPipelineServiceTests::playShouldFail() { - EXPECT_FALSE(m_sut->play(kSessionId)); + bool isAsync{false}; + EXPECT_FALSE(m_sut->play(kSessionId, isAsync)); } void MediaPipelineServiceTests::pauseShouldSucceed()