From 87a5f4e2cccaa365d0c97b321332737d22cb430b Mon Sep 17 00:00:00 2001 From: varatharajan568 <130632918+varatharajan568@users.noreply.github.com> Date: Mon, 23 Feb 2026 15:28:40 +0530 Subject: [PATCH 01/23] Update SocInterface.cpp --- vendor/SocInterface.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/vendor/SocInterface.cpp b/vendor/SocInterface.cpp index 6c3c7149..ccf8499f 100644 --- a/vendor/SocInterface.cpp +++ b/vendor/SocInterface.cpp @@ -26,7 +26,6 @@ #include "vendor/realtek/RealtekSocInterface.h" #endif - /** * @brief Checks if the input string starts with the given prefix. * From a762d558f0d75a9b3b5d5e8eda937470a22d174a Mon Sep 17 00:00:00 2001 From: varatharajan568 <130632918+varatharajan568@users.noreply.github.com> Date: Tue, 24 Feb 2026 14:45:53 +0530 Subject: [PATCH 02/23] Update install_dependencies.sh --- scripts/install_dependencies.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/install_dependencies.sh b/scripts/install_dependencies.sh index dac9e629..adda9273 100644 --- a/scripts/install_dependencies.sh +++ b/scripts/install_dependencies.sh @@ -110,7 +110,6 @@ function pip_install_package_fn() function install_pkgs_linux_fn() { - sudo apt update install_package_fn git install_package_fn cmake install_package_fn gcc From 7da2bda30239a411681d6591a7e780fed4251f01 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Mon, 3 Aug 2026 13:48:12 +0000 Subject: [PATCH 03/23] VPAAMP-867 DirectRialto - Changes required Reason for Change: Changes required for DirectRialto Test Procedure: Build and run AAMP with DirectRialto enabled Priority: P1 Risks: Medium --- CMakeLists.txt | 4 +- closedcaptions/PlayerCCManager.cpp | 37 ++- closedcaptions/PlayerCCManager.h | 22 +- .../direct-rialto/IDirectRialtoCC.h | 64 ++++++ .../PlayerDirectRialtoCCManager.cpp | 172 ++++++++++++++ .../PlayerDirectRialtoCCManager.h | 110 +++++++++ drm/DrmSession.cpp | 132 ----------- drm/DrmSession.h | 63 ++--- drm/DrmSessionManager.cpp | 26 ++- drm/DrmSessionManager.h | 20 +- drm/ocdm/opencdmsessionadapter.cpp | 4 +- drm/ocdm/opencdmsessionadapter.h | 2 +- test/utests/fakes/CMakeLists.txt | 1 + test/utests/fakes/FakeDRMSession.cpp | 4 - test/utests/fakes/FakeDRMSessionManager.cpp | 2 +- .../fakes/Fakeopencdmsessionadapter.cpp | 8 +- test/utests/mocks/MockOpenCdmSessionAdapter.h | 2 +- test/utests/tests/CMakeLists.txt | 1 + test/utests/tests/DrmAes/CMakeLists.txt | 1 - .../tests/DrmHelperTests/CMakeLists.txt | 1 - test/utests/tests/DrmOcdmTests/CMakeLists.txt | 1 - .../tests/DrmSecureClient/CMakeLists.txt | 1 - .../DrmSessionManagerTests/CMakeLists.txt | 1 - .../DrmSessionManagerTestCases.cpp | 14 +- test/utests/tests/DrmTests/CMakeLists.txt | 1 - .../tests/OCDMSessionAdapter/CMakeLists.txt | 1 - .../OCDMSessionAdapter/FunctionalTests.cpp | 12 +- .../CMakeLists.txt | 1 - .../FunctionalTests.cpp | 3 +- .../CMakeLists.txt | 68 ++++++ .../PlayerDirectRialtoCCManagerBaseStubs.cpp | 80 +++++++ .../PlayerDirectRialtoCCManagerTestCases.cpp | 217 ++++++++++++++++++ .../PlayerDirectRialtoCCManagerTests.cpp | 31 +++ 33 files changed, 894 insertions(+), 213 deletions(-) create mode 100755 closedcaptions/direct-rialto/IDirectRialtoCC.h create mode 100755 closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp create mode 100755 closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h delete mode 100644 drm/DrmSession.cpp create mode 100644 test/utests/tests/PlayerDirectRialtoCCManagerTests/CMakeLists.txt create mode 100644 test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp create mode 100644 test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp create mode 100644 test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTests.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 69596a8c..ffcf5373 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -241,7 +241,6 @@ endif() set(LIBPLAYERGSTINTERFACE_DRM_SOURCES drm/PlayerHlsDrmSessionInterface.cpp drm/DrmSessionManager.cpp - drm/DrmSession.cpp drm/DrmSessionFactory.cpp drm/helper/DrmHelper.cpp drm/helper/DrmHelperFactory.cpp @@ -412,7 +411,7 @@ if (CMAKE_SUBTITLE_SUPPORT) ) - set(LIBPLAYERGSTINTERFACE_SOURCES ${LIBPLAYERGSTINTERFACE_SOURCES} closedcaptions/subtec/PlayerSubtecCCManager.cpp closedcaptions/rialto/PlayerRialtoCCManager.cpp) + set(LIBPLAYERGSTINTERFACE_SOURCES ${LIBPLAYERGSTINTERFACE_SOURCES} closedcaptions/subtec/PlayerSubtecCCManager.cpp closedcaptions/rialto/PlayerRialtoCCManager.cpp closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp) endif() add_library(playergstinterface SHARED ${SOURCES} ${LIBPLAYERGSTINTERFACE_HEADERS} ${LIBPLAYERGSTINTERFACE_SOURCES} ${LIBPLAYERGSTINTERFACE_DRM_SOURCES} ${LIBPLAYERGSTINTERFACE_HELP_SOURCES}) @@ -426,6 +425,7 @@ target_include_directories(playergstinterface PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/playerjsonobject ${CMAKE_CURRENT_SOURCE_DIR}/closedcaptions ${CMAKE_CURRENT_SOURCE_DIR}/closedcaptions/subtec + ${CMAKE_CURRENT_SOURCE_DIR}/closedcaptions/direct-rialto ${CMAKE_CURRENT_SOURCE_DIR}/vendor) if (CMAKE_SUBTITLE_SUPPORT) diff --git a/closedcaptions/PlayerCCManager.cpp b/closedcaptions/PlayerCCManager.cpp index d61b379e..bfd0f689 100644 --- a/closedcaptions/PlayerCCManager.cpp +++ b/closedcaptions/PlayerCCManager.cpp @@ -34,6 +34,7 @@ #include "PlayerCCManager.h" #include "PlayerSubtecCCManager.h" #include "PlayerRialtoCCManager.h" +#include "PlayerDirectRialtoCCManager.h" #define CHAR_CODE_1 49 @@ -823,9 +824,10 @@ bool PlayerCCManagerBase::IsOOBCCRenderingSupported() PlayerCCManagerBase *PlayerCCManager::mInstance = NULL; /** - * @brief Indicates whether mInstance should be a Rialto or a Subtec class. + * @brief Determines which CC manager subclass to instantiate. */ -bool PlayerCCManager::mIsRialto = false; +PlayerCCManager::CCManagerType PlayerCCManager::mCCManagerType = + PlayerCCManager::CCManagerType::SubtecCCManager; /** * @brief Get the singleton instance @@ -835,7 +837,12 @@ PlayerCCManagerBase *PlayerCCManager::GetInstance() if (mInstance == NULL) { #if defined(SUBTITLE_SUPPORTED) - if (mIsRialto) + if (mCCManagerType == CCManagerType::DirectRialtoCCManager) + { + MW_LOG_INFO("PlayerCCManager::Creating DirectRialto CC manager"); + mInstance = new PlayerDirectRialtoCCManager(); + } + else if (mCCManagerType == CCManagerType::RialtoCCManager) { MW_LOG_INFO("PlayerCCManager::Creating Rialto CC manager"); mInstance = new PlayerRialtoCCManager(); @@ -871,18 +878,30 @@ void PlayerCCManagerBase::ResetState() } /** - * @brief Set the variant required + * @brief Set the CC manager variant */ -void PlayerCCManager::SetRialto(bool bIsRialto) +void PlayerCCManager::SetRialto(bool bIsRialto, bool bIsDirectRialto) { + CCManagerType newType = CCManagerType::SubtecCCManager; + + if (bIsDirectRialto) + { + newType = CCManagerType::DirectRialtoCCManager; + } + else if (bIsRialto) + { + newType = CCManagerType::RialtoCCManager; + } + if (mInstance == NULL) { - MW_LOG_INFO("PlayerCCManager::IsRialto:%d", bIsRialto); - mIsRialto = bIsRialto; + MW_LOG_INFO("PlayerCCManager::CCManagerType:%d", static_cast(newType)); + mCCManagerType = newType; } - else if (mIsRialto != bIsRialto) + else if (mCCManagerType != newType) { - MW_LOG_ERR("PlayerCCManager::IsRialto:%d while incompatible singleton instance exists", bIsRialto); + MW_LOG_ERR("PlayerCCManager::CCManagerType:%d while incompatible singleton instance exists", + static_cast(newType)); } } diff --git a/closedcaptions/PlayerCCManager.h b/closedcaptions/PlayerCCManager.h index 6990f53c..af4683cb 100644 --- a/closedcaptions/PlayerCCManager.h +++ b/closedcaptions/PlayerCCManager.h @@ -276,10 +276,15 @@ class PlayerCCManager /** * @fn SetRialto + * @brief Configure which CC manager subclass GetInstance() will create. * + * @param[in] bIsRialto true when using the Rialto GStreamer sink + * (PlayerRialtoCCManager). + * @param[in] bIsDirectRialto true when using the direct-Rialto path + * (PlayerDirectRialtoCCManager). * @return void */ - static void SetRialto(bool bIsRialto); + static void SetRialto(bool bIsRialto, bool bIsDirectRialto = false); /** * @fn DestroyInstance @@ -289,8 +294,19 @@ class PlayerCCManager static void DestroyInstance(); private: - static PlayerCCManagerBase *mInstance; /**< Singleton instance */ - static bool mIsRialto; /**< Determines which class to instantiate */ + /** + * @enum CCManagerType + * @brief Identifies which PlayerCCManagerBase subclass to instantiate. + */ + enum class CCManagerType + { + SubtecCCManager, ///< Use PlayerSubtecCCManager (default) + RialtoCCManager, ///< Use PlayerRialtoCCManager + DirectRialtoCCManager ///< Use PlayerDirectRialtoCCManager + }; + + static PlayerCCManagerBase *mInstance; /**< Singleton instance */ + static CCManagerType mCCManagerType; /**< Determines which class to instantiate */ }; class PlayerFakeCCManager : public PlayerCCManagerBase diff --git a/closedcaptions/direct-rialto/IDirectRialtoCC.h b/closedcaptions/direct-rialto/IDirectRialtoCC.h new file mode 100755 index 00000000..46b202a4 --- /dev/null +++ b/closedcaptions/direct-rialto/IDirectRialtoCC.h @@ -0,0 +1,64 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file IDirectRialtoCC.h + * @brief Narrow control interface for closed-caption operations in the + * direct-rialto path. + * + * AampRialtoPlayer implements this interface and passes itself (cast to + * IDirectRialtoCC*) to PlayerDirectRialtoCCManager::Initialize() so that + * the CC manager can drive track selection and muting through the Rialto + * IMediaPipeline API without depending on GStreamer. + */ + +#ifndef IDIRECT_RIALTO_CC_H +#define IDIRECT_RIALTO_CC_H + +#include + +/** + * @interface IDirectRialtoCC + * @brief Minimal CC-control interface implemented by AampRialtoPlayer. + * + * Decouples PlayerDirectRialtoCCManager (which needs no Rialto headers) from + * AampRialtoPlayer (which owns the IMediaPipeline) so that neither class + * needs to include the other's full header. + */ +class IDirectRialtoCC +{ +public: + virtual ~IDirectRialtoCC() = default; + + /** + * @brief Set the active CC text-track identifier on the pipeline. + * @param id Track identifier string (e.g. "CC1", "SERVICE1"). + * @return true on success. + */ + virtual bool setTextTrackIdentifier(const std::string &id) = 0; + + /** + * @brief Mute or un-mute CC rendering via the pipeline. + * @param muted true to mute; false to un-mute. + * @return true on success. + */ + virtual bool setCCMute(bool muted) = 0; +}; + +#endif // IDIRECT_RIALTO_CC_H diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp new file mode 100755 index 00000000..35e73268 --- /dev/null +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -0,0 +1,172 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file PlayerDirectRialtoCCManager.cpp + * @brief Implementation of PlayerDirectRialtoCCManager. + */ + +#include "PlayerDirectRialtoCCManager.h" +#include "PlayerLogManager.h" + +#include +#include + +// --------------------------------------------------------------------------- +// Private helpers +// --------------------------------------------------------------------------- + +/*static*/ +std::string PlayerDirectRialtoCCManager::mapTrackIdentifier( + const std::string &track, CCFormat format) +{ + // If the track string already has an alphabetic prefix (e.g. "CC1", + // "SERVICE3"), use it as-is. If it starts with a digit, prepend the + // appropriate prefix so the Rialto server can identify the CC service. + // This mirrors the logic in PlayerRialtoCCManager::SetTrack(). + if (!track.empty() && std::isdigit(static_cast(track[0]))) + { + std::string prefix; + if (format == eCLOSEDCAPTION_FORMAT_608) + { + prefix = "CC"; + } + else if (format == eCLOSEDCAPTION_FORMAT_708) + { + prefix = "SERVICE"; + } + return prefix + track; + } + return track; +} + +// --------------------------------------------------------------------------- +// PlayerCCManagerBase overrides +// --------------------------------------------------------------------------- + +int PlayerDirectRialtoCCManager::Initialize(void *handle) +{ + MW_LOG_INFO("ENTRY handle=%p", handle); + + auto *newControl = static_cast(handle); + const bool changedHandle = (newControl != m_control); + m_control = newControl; + + if (m_control == nullptr) + { + MW_LOG_WARN("Initialize called with null handle"); + MW_LOG_INFO("EXIT"); + return 0; + } + + if (GetTrack().empty()) + { + // Apps expect CC1 as the default; apply it so the first frame + // renders without an explicit SetTextTrack() call. + MW_LOG_INFO("Setting default track CC1"); + (void) SetTrack("CC1"); + } + else if (changedHandle) + { + // Re-apply the cached track on the new handle (e.g. re-tune). + (void) SetTrack(GetTrack(), mTrackFormat); + } + + MW_LOG_INFO("EXIT"); + return 0; +} + +int PlayerDirectRialtoCCManager::GetId() +{ + std::lock_guard lock(m_idLock); + ++m_id; + m_idSet.insert(m_id); + MW_LOG_INFO("id=%d users=%zu", m_id, m_idSet.size()); + return m_id; +} + +void PlayerDirectRialtoCCManager::Release(int id) +{ + std::lock_guard lock(m_idLock); + if (m_idSet.erase(id) > 0) + { + MW_LOG_INFO("id=%d users=%zu", id, m_idSet.size()); + if (m_idSet.empty()) + { + ResetState(); + } + } + else + { + MW_LOG_WARN("id=%d not found", id); + } +} + +int PlayerDirectRialtoCCManager::SetTrack( + const std::string &track, CCFormat format) +{ + // Cache for re-application after Initialize(). + mTrack = track; + mTrackFormat = format; + + MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); + + if (m_control == nullptr) + { + MW_LOG_INFO("No control handle — track cached"); + return 0; + } + + const std::string identifier = mapTrackIdentifier(track, format); + MW_LOG_INFO("setTextTrackIdentifier=\"%s\"", identifier.c_str()); + m_control->setTextTrackIdentifier(identifier); + return 0; +} + +void PlayerDirectRialtoCCManager::StartRendering() +{ + MW_LOG_INFO("ENTRY — unmuting CC"); + if (m_control == nullptr) + { + MW_LOG_WARN("No control handle — cannot unmute"); + return; + } + m_control->setCCMute(false); + MW_LOG_INFO("EXIT"); +} + +void PlayerDirectRialtoCCManager::StopRendering() +{ + MW_LOG_INFO("ENTRY — muting CC"); + if (m_control == nullptr) + { + MW_LOG_WARN("No control handle — cannot mute"); + return; + } + m_control->setCCMute(true); + MW_LOG_INFO("EXIT"); +} + +void PlayerDirectRialtoCCManager::ResetState() +{ + MW_LOG_INFO("ENTRY"); + PlayerCCManagerBase::ResetState(); + m_control = nullptr; + MW_LOG_INFO("EXIT"); +} diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h new file mode 100755 index 00000000..936f1f50 --- /dev/null +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -0,0 +1,110 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file PlayerDirectRialtoCCManager.h + * @brief PlayerCCManagerBase subclass for the direct-rialto backend. + * + * Replaces PlayerRialtoCCManager (which calls g_object_set on a GstElement*) + * with a GStreamer-free implementation that forwards track selection and + * mute/un-mute through the IDirectRialtoCC control interface implemented by + * AampRialtoPlayer. + * + * Lifecycle: + * 1. PlayerCCManager::GetInstance() creates this instance when + * mCCManagerType == DirectRialtoCCManager, which is set by calling + * PlayerCCManager::SetRialto(true, true) during player construction. + * 2. When the first PLAYING playback-state arrives, AampRialtoPlayer passes + * itself (as IDirectRialtoCC*) via NotifyFirstFrameReceived(), which + * flows through priv_aamp::InitializeCC() → PlayerCCManagerBase::Init() + * → Initialize() here, storing the control pointer. + * 3. Subsequent SetTrack() / StartRendering() / StopRendering() calls from + * priv_aamp drive track selection and muting through the stored pointer. + */ + +#ifndef PLAYER_DIRECT_RIALTO_CC_MANAGER_H +#define PLAYER_DIRECT_RIALTO_CC_MANAGER_H + +#include "PlayerCCManager.h" +#include "IDirectRialtoCC.h" + +#include +#include + +/** + * @class PlayerDirectRialtoCCManager + * @brief Closed-caption manager for the direct-rialto path. + */ +class PlayerDirectRialtoCCManager : public PlayerCCManagerBase +{ +public: + PlayerDirectRialtoCCManager() = default; + ~PlayerDirectRialtoCCManager() override = default; + + /// @copydoc PlayerCCManagerBase::GetId + int GetId() override; + + /// @copydoc PlayerCCManagerBase::Release + void Release(int iID) override; + + /// @copydoc PlayerCCManagerBase::SetTrack + int SetTrack(const std::string &track, + CCFormat format = eCLOSEDCAPTION_FORMAT_DEFAULT) override; + +protected: + /// @copydoc PlayerCCManagerBase::Initialize + int Initialize(void *handle) override; + + /// @copydoc PlayerCCManagerBase::StartRendering + void StartRendering() override; + + /// @copydoc PlayerCCManagerBase::StopRendering + void StopRendering() override; + + /// @copydoc PlayerCCManagerBase::SetDigitalChannel + int SetDigitalChannel(unsigned int id) override { return 0; } + + /// @copydoc PlayerCCManagerBase::SetAnalogChannel + int SetAnalogChannel(unsigned int id) override { return 0; } + + /// @copydoc PlayerCCManagerBase::ResetState + void ResetState() override; + +private: + /** + * @brief Map a CC track string to the text-track-identifier expected by + * the Rialto server, mirroring PlayerRialtoCCManager::SetTrack(). + * + * If the track string starts with a digit, a prefix is prepended based + * on the format: "CC" for 608, "SERVICE" for 708. + */ + static std::string mapTrackIdentifier(const std::string &track, + CCFormat format); + + /// Non-owning pointer to the AampRialtoPlayer CC control interface. + /// Null until Initialize() is called (first PLAYING state). + IDirectRialtoCC *m_control{nullptr}; + + /// Guards mId / mIdSet. + std::mutex m_idLock; + int m_id{0}; + std::set m_idSet; +}; + +#endif // PLAYER_DIRECT_RIALTO_CC_MANAGER_H diff --git a/drm/DrmSession.cpp b/drm/DrmSession.cpp deleted file mode 100644 index 35d10c95..00000000 --- a/drm/DrmSession.cpp +++ /dev/null @@ -1,132 +0,0 @@ -/* - * If not stated otherwise in this file or this component's license file the - * following copyright and licenses apply: - * - * Copyright 2025 RDK Management - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. -*/ - -/** -* @file DrmSession.cpp -* @brief Source file for DrmSession. -*/ - -#include "DrmSession.h" -#include "PlayerLogManager.h" -#include - -/** - * @brief Constructor for DrmSession. - */ -DrmSession::DrmSession(const string &keySystem) : m_keySystem(keySystem),m_OutputProtectionEnabled(false) - , mContentSecurityManagerSession() - , mLifecycleMutex() - , mLifecycleCV() - , mActiveOperations(0) - , mMarkedForDestruction(false) -{ -} - -/** - * @brief Destructor for DrmSession.. - */ -DrmSession::~DrmSession() -{ -} - -/** - * @brief DELIA-70726 fix: Acquire lifecycle guard before use in decrypt(). - */ -bool DrmSession::AcquireForUse() -{ - std::lock_guard lock(mLifecycleMutex); - if (mMarkedForDestruction) - { - return false; - } - mActiveOperations++; - return true; -} - -/** - * @brief DELIA-70726 fix: Release lifecycle guard after use in decrypt(). - */ -void DrmSession::ReleaseAfterUse() -{ - std::lock_guard lock(mLifecycleMutex); - if (mActiveOperations > 0) - { - mActiveOperations--; - } - if (mActiveOperations == 0) - { - mLifecycleCV.notify_all(); - } -} - -/** - * @brief DELIA-70726 fix: Block deletion of this session until any decrypt() - * call already in progress (having acquired the guard) has finished. - */ -void DrmSession::PrepareForDestruction(uint32_t timeoutMs) -{ - std::unique_lock lock(mLifecycleMutex); - mMarkedForDestruction = true; - if (mActiveOperations > 0) - { - MW_LOG_WARN("DrmSession::PrepareForDestruction : waiting for %d in-flight decrypt operation(s) to complete before delete", mActiveOperations); - mLifecycleCV.wait_for(lock, std::chrono::milliseconds(timeoutMs), [this]() { return mActiveOperations == 0; }); - if (mActiveOperations > 0) - { - MW_LOG_ERR("DrmSession::PrepareForDestruction : timed out waiting for in-flight decrypt operation(s); proceeding with destruction"); - } - } -} - -/** - * @brief Get the DRM System, ie, UUID for PlayReady WideVine etc.. - */ -string DrmSession::getKeySystem() -{ - return m_keySystem; -} - -/** - * @brief Function to decrypt GStreamer stream buffer. - */ -int DrmSession::decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps) -{ - MW_LOG_ERR("GST decrypt method not implemented"); - return -1; -} - -/** - * @brief Function to decrypt stream buffer. - */ -int DrmSession::decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData) -{ - MW_LOG_ERR("Standard decrypt method not implemented"); - return -1; -} - -/** - * @brief Get the list of usable key IDs from the DRM session - * @retval Reference to vector of usable key IDs - * @note Default implementation returns the reference to an empty vector - */ -const std::vector>& DrmSession::getUsableKeys() const -{ - static const std::vector> emptyVector; - return emptyVector; -} diff --git a/drm/DrmSession.h b/drm/DrmSession.h index c760c012..3fc429f5 100755 --- a/drm/DrmSession.h +++ b/drm/DrmSession.h @@ -87,6 +87,32 @@ class DrmSession bool mMarkedForDestruction; public: + /** + * @fn DrmSession + * @param keySystem : DRM key system uuid + */ + DrmSession(const string &keySystem) : + m_keySystem(keySystem), + m_OutputProtectionEnabled(false), + mContentSecurityManagerSession() { }; + + /** + * @brief Copy constructor disabled + * + */ + DrmSession(const DrmSession&) = delete; + + /** + * @fn ~DrmSession + */ + virtual ~DrmSession() = default; + + /** + * @brief assignment operator disabled + * + */ + DrmSession& operator=(const DrmSession&) = delete; + /** * @fn AcquireForUse * @brief Must be called by any external caller (e.g. the GStreamer @@ -151,7 +177,7 @@ class DrmSession * @param caps : Caps of the media that is currently being decrypted * @retval Returns status of decrypt request. */ - virtual int decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps = NULL); + virtual int decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps = NULL) { return -1; }; /** * @fn decrypt @@ -162,7 +188,7 @@ class DrmSession * @param ppOpaqueData : pointer to opaque buffer in case of SVP. * @retval Returns status of decrypt request. */ - virtual int decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData); + virtual int decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData) { return -1; }; /** * @brief Get the current state of DRM Session. @@ -187,36 +213,24 @@ class DrmSession /** * @brief Get the list of usable key IDs from the DRM session - * @retval Reference to vector of usable key IDs - * @note Default implementation returns the reference to an empty vector + * @retval Snapshot copy of usable key IDs, taken under the session's + * internal lock where applicable. Callers receive their own + * independent copy and need not hold any external lock. */ - virtual const std::vector>& getUsableKeys() const; + virtual std::vector> getUsableKeys() const { return {}; } /** - * @fn DrmSession - * @param keySystem : DRM key system uuid - */ - DrmSession(const string &keySystem); - /** - * @brief Copy constructor disabled - * - */ - DrmSession(const DrmSession&) = delete; - /** - * @brief assignment operator disabled - * - */ - DrmSession& operator=(const DrmSession&) = delete; - /** - * @fn ~DrmSession + * @brief Return the Rialto media key session ID, or -1 if not applicable. */ - virtual ~DrmSession(); + virtual int32_t getMediaKeySessionId() const { return -1; } + + virtual void setKeyId(const std::vector& keyId) {}; /** * @fn getKeySystem * @retval DRM system uuid */ - string getKeySystem(); + string getKeySystem() { return m_keySystem; }; /** * @brief Set the OutputProtection for DRM Session @@ -224,9 +238,6 @@ class DrmSession * @retval void */ void setOutputProtection(bool bValue) { m_OutputProtectionEnabled = bValue;} -#if defined(USE_OPENCDM_ADAPTER) - virtual void setKeyId(const std::vector& keyId) {}; -#endif void setSecManagerSession(ContentSecurityManagerSession session){mContentSecurityManagerSession=session;} ContentSecurityManagerSession getSecManagerSession() const { return mContentSecurityManagerSession;} }; diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 8c3da56b..383e523a 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -48,13 +48,14 @@ KeyIdEntries::KeyIdEntries() : creationTime(0), isFailedKeyEntries(false), isPri /** * @brief DrmSessionManager constructor. */ -DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback) : drmSessionContexts(NULL), +DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator) : drmSessionContexts(NULL), cachedKeyIDs(NULL), accessToken(NULL), accessTokenLen(0), sessionMgrState(SessionMgrState::eSESSIONMGR_ACTIVE), accessTokenMutex(), cachedKeyMutex() ,mEnableAccessAttributes(true) ,mDrmSessionLock() ,mMaxDRMSessions(maxDrmSessions) + ,m_sessionCreator(std::move(creator)) ,playerSecInstance(nullptr) ,mContentSecurityManagerSession() ,mIsVideoOnMute(false) @@ -527,7 +528,18 @@ DrmSession* DrmSessionManager::createDrmSession(int &responseCode, int &err, std } return nullptr; } - code =this->AcquireLicenseCb(responseCode, std::move(drmHelper), selectedSlot, cdmError, (GstMediaType)streamType, metaDataPtr, false); + + if (!AcquireLicenseCb) + { + MW_LOG_WARN("AcquireLicenseCb not registered - cannot acquire license"); + std::lock_guard guard(cachedKeyMutex); + if (cachedKeyIDs) + { + cachedKeyIDs[selectedSlot].isFailedKeyEntries = true; + } + return nullptr; + } + code = AcquireLicenseCb(responseCode, std::move(drmHelper), selectedSlot, cdmError, (GstMediaType)streamType, metaDataPtr, false); if (code != KEY_READY) { MW_LOG_WARN(" Unable to get Ready Status DrmSession : Key State %d ", code); @@ -900,7 +912,15 @@ KeyState DrmSessionManager::getDrmSession(int &err, std::shared_ptr d } this->ProfileUpdateCb(); - drmSessionContexts[sessionSlot].drmSession = DrmSessionFactory::GetDrmSession(drmHelper, Instance); + if (m_sessionCreator) + { + auto owned = m_sessionCreator(drmHelper, Instance); + drmSessionContexts[sessionSlot].drmSession = owned.release(); + } + else + { + drmSessionContexts[sessionSlot].drmSession = DrmSessionFactory::GetDrmSession(drmHelper, Instance); + } if (drmSessionContexts[sessionSlot].drmSession != NULL) { MW_LOG_INFO("Created new DrmSession for DrmSystemId %s", systemId.c_str()); diff --git a/drm/DrmSessionManager.h b/drm/DrmSessionManager.h index 3e8a0d31..7b4dd58c 100644 --- a/drm/DrmSessionManager.h +++ b/drm/DrmSessionManager.h @@ -32,6 +32,7 @@ #include #include #include +#include #include "DrmHelper.h" #include "PlayerSecInterface.h" @@ -39,6 +40,15 @@ #include +/** + * @brief Factory callable type for creating DRM sessions. + * + * Stored per-player in DrmSessionManager to support creator injection + * (e.g. the direct-Rialto path). + */ +using DrmSessionCreator = + std::function(DrmHelperPtr, DrmCallbacks*)>; + #define VIDEO_SESSION 0 #define AUDIO_SESSION 1 @@ -176,6 +186,7 @@ class DrmSessionManager std::mutex mDrmSessionLock; bool mEnableAccessAttributes; int mMaxDRMSessions; + DrmSessionCreator m_sessionCreator; std::function mPlayerSendWatermarkSessionUpdateEventCB; /** * @brief Copy constructor disabled @@ -222,10 +233,7 @@ class DrmSessionManager /** * @fn DrmSessionManager */ - DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback); - - void initializeDrmSessions(); - + DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator = nullptr); /** * @fn watermarkSessionHandlerWrapper * @brief Wrapper function to handle session watermark. @@ -517,12 +525,12 @@ class DrmSessionManager /** * @brief Configuration parameters needed from Player */ - void UpdateDRMConfig( + void UpdateDRMConfig( bool useSecManager, bool enablePROutputProtection, bool propagateURIParam, bool isFakeTune, - bool wideVineKIDWorkaround); + bool wideVineKIDWorkaround); }; diff --git a/drm/ocdm/opencdmsessionadapter.cpp b/drm/ocdm/opencdmsessionadapter.cpp index c5d8d8ed..9753c781 100644 --- a/drm/ocdm/opencdmsessionadapter.cpp +++ b/drm/ocdm/opencdmsessionadapter.cpp @@ -451,9 +451,9 @@ bool OCDMSessionAdapter::verifyOutputProtection() /** * @fn getUsableKeys * @brief Get the list of usable key IDs from the DRM session - * @retval Reference to vector of usable key IDs + * @retval Snapshot copy of usable key IDs, taken under m_usableKeysMutex */ -const std::vector>& OCDMSessionAdapter::getUsableKeys() const +std::vector> OCDMSessionAdapter::getUsableKeys() const { std::lock_guard lock(m_usableKeysMutex); return m_usableKeys; diff --git a/drm/ocdm/opencdmsessionadapter.h b/drm/ocdm/opencdmsessionadapter.h index 36dd5649..25513ff2 100644 --- a/drm/ocdm/opencdmsessionadapter.h +++ b/drm/ocdm/opencdmsessionadapter.h @@ -125,7 +125,7 @@ class OCDMSessionAdapter : public DrmSession void processOCDMChallenge(const char destUrl[], const uint8_t challenge[], const uint16_t challengeSize); void keysUpdatedOCDM(); void keyUpdateOCDM(const uint8_t key[], const uint8_t keySize); - const std::vector>& getUsableKeys() const override; + std::vector> getUsableKeys() const override; long long timeBeforeCallback; private: diff --git a/test/utests/fakes/CMakeLists.txt b/test/utests/fakes/CMakeLists.txt index 2a6d819c..74a81cbc 100644 --- a/test/utests/fakes/CMakeLists.txt +++ b/test/utests/fakes/CMakeLists.txt @@ -35,6 +35,7 @@ include_directories(${UTESTS_ROOT}/drm/ocdm) include_directories(${UTESTS_ROOT}/drm/mocks) include_directories(${UTESTS_ROOT}/mocks) include_directories(${UTESTS_ROOT}/ocdm) +include_directories(${UTESTS_ROOT}/rialto) include_directories(${PLAYER_ROOT}/drm) include_directories(${PLAYER_ROOT}/drm/ocdm) include_directories(${PLAYER_ROOT}/baseConversion) diff --git a/test/utests/fakes/FakeDRMSession.cpp b/test/utests/fakes/FakeDRMSession.cpp index da347f6d..6780396b 100644 --- a/test/utests/fakes/FakeDRMSession.cpp +++ b/test/utests/fakes/FakeDRMSession.cpp @@ -19,7 +19,3 @@ #include "DrmSession.h" -string DrmSession::getKeySystem() -{ - return ""; -} diff --git a/test/utests/fakes/FakeDRMSessionManager.cpp b/test/utests/fakes/FakeDRMSessionManager.cpp index 289cb852..a778e4f0 100644 --- a/test/utests/fakes/FakeDRMSessionManager.cpp +++ b/test/utests/fakes/FakeDRMSessionManager.cpp @@ -22,7 +22,7 @@ #include "MockDrmSessionManager.h" MockDRMSessionManager *g_mockDRMSessionManager = nullptr; -DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback) +DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator) { } diff --git a/test/utests/fakes/Fakeopencdmsessionadapter.cpp b/test/utests/fakes/Fakeopencdmsessionadapter.cpp index 596450ca..e9875f65 100644 --- a/test/utests/fakes/Fakeopencdmsessionadapter.cpp +++ b/test/utests/fakes/Fakeopencdmsessionadapter.cpp @@ -24,7 +24,6 @@ MockOpenCdmSessionAdapter *g_mockOpenCdmSessionAdapter = nullptr; std::vector g_mockKeyId{1,2,3,4,5,6,7,8,9,0,1,2,3,4}; -const std::vector> g_emptyUsableKeys{}; OCDMSessionAdapter::OCDMSessionAdapter(std::shared_ptr drmHelper, DrmCallbacks *callbacks) : DrmSession("ocdmkeysystem"), m_keyId{g_mockKeyId}, m_drmHelper{drmHelper} @@ -79,15 +78,14 @@ bool OCDMSessionAdapter::waitForState(KeyState state, const uint32_t timeout) /** * @brief Get the list of usable key IDs from the DRM session - * @retval Reference to vector of usable key IDs - * @note Default implementation returns the reference to an empty vector + * @retval Snapshot copy of usable key IDs */ -const std::vector>& OCDMSessionAdapter::getUsableKeys() const +std::vector> OCDMSessionAdapter::getUsableKeys() const { if (g_mockOpenCdmSessionAdapter) { return g_mockOpenCdmSessionAdapter->getUsableKeys(); } - return g_emptyUsableKeys; + return {}; } #if defined(USE_OPENCDM_ADAPTER) diff --git a/test/utests/mocks/MockOpenCdmSessionAdapter.h b/test/utests/mocks/MockOpenCdmSessionAdapter.h index 372d3e94..f5aec80d 100644 --- a/test/utests/mocks/MockOpenCdmSessionAdapter.h +++ b/test/utests/mocks/MockOpenCdmSessionAdapter.h @@ -30,7 +30,7 @@ class MockOpenCdmSessionAdapter MOCK_METHOD(bool, verifyOutputProtection, ()); MOCK_METHOD(void, setKeyId, (const std::vector&)); - MOCK_METHOD(const std::vector>&, getUsableKeys, (), (const)); + MOCK_METHOD(std::vector>, getUsableKeys, (), (const)); MOCK_METHOD(void, generateDRMSession, (const uint8_t*, uint32_t, std::string&)); MOCK_METHOD(KeyState, getState, ()); }; diff --git a/test/utests/tests/CMakeLists.txt b/test/utests/tests/CMakeLists.txt index 96b98cd8..8d6c0ab9 100644 --- a/test/utests/tests/CMakeLists.txt +++ b/test/utests/tests/CMakeLists.txt @@ -46,3 +46,4 @@ add_subdirectory(FireBoltTests) add_subdirectory(DrmOcdmTests) add_subdirectory(DrmHelperTests) add_subdirectory(DrmAes) +add_subdirectory(PlayerDirectRialtoCCManagerTests) diff --git a/test/utests/tests/DrmAes/CMakeLists.txt b/test/utests/tests/DrmAes/CMakeLists.txt index 79360faf..ae2d6fae 100755 --- a/test/utests/tests/DrmAes/CMakeLists.txt +++ b/test/utests/tests/DrmAes/CMakeLists.txt @@ -65,7 +65,6 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmHelperTests/CMakeLists.txt b/test/utests/tests/DrmHelperTests/CMakeLists.txt index 56e0ca20..d78c9cbe 100644 --- a/test/utests/tests/DrmHelperTests/CMakeLists.txt +++ b/test/utests/tests/DrmHelperTests/CMakeLists.txt @@ -72,7 +72,6 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmOcdmTests/CMakeLists.txt b/test/utests/tests/DrmOcdmTests/CMakeLists.txt index 7204420d..ff764823 100644 --- a/test/utests/tests/DrmOcdmTests/CMakeLists.txt +++ b/test/utests/tests/DrmOcdmTests/CMakeLists.txt @@ -69,7 +69,6 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmGstSessionAdapter.cpp diff --git a/test/utests/tests/DrmSecureClient/CMakeLists.txt b/test/utests/tests/DrmSecureClient/CMakeLists.txt index ab46343b..48e3f78e 100644 --- a/test/utests/tests/DrmSecureClient/CMakeLists.txt +++ b/test/utests/tests/DrmSecureClient/CMakeLists.txt @@ -66,7 +66,6 @@ if(SEC_CLIENT_HEADERS) ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt b/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt index f42e930b..f91c6819 100644 --- a/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt +++ b/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt @@ -60,7 +60,6 @@ set(UTILITY_SOURCES set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/helper/WidevineDrmHelper.cpp diff --git a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp index 6c0b9c47..acac0af8 100644 --- a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp +++ b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp @@ -728,7 +728,7 @@ TEST_F(DrmSessionManagerComplexTests, ValidateMultiKeySlot_RealWidevinePssh_Thre usableKeys.push_back(RawKeyToKeyId(binaryKey3.data(), binaryKey3.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate with first key (should match after dash normalization) bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[0], 0); @@ -824,7 +824,7 @@ TEST_F(DrmSessionManagerComplexTests, ValidateMultiKeySlot_CreateDrmHelperFromIn usableKeys.push_back(expectedKeyIdAscii); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate slot bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[0], 0); @@ -982,7 +982,7 @@ TEST_F(DrmSessionManagerComplexTests, CreateDrmHelperFromInitData_MultipleKeys_C usableKeys.push_back(RawKeyToKeyId(expectedSdKey.data(), expectedSdKey.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate with HD key (second key) - should succeed bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[1], 0); @@ -1060,7 +1060,7 @@ TEST_F(DrmSessionManagerComplexTests, InitDataFlow_EndToEnd_CreateHelperAndValid usableKeys.push_back(RawKeyToKeyId(keyBinary.data(), keyBinary.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Step 9: Validate slot bool validationResult = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[0], 0); @@ -1116,7 +1116,7 @@ TEST_F(DrmSessionManagerComplexTests, ValidateMultiKeySlot_RealWidevinePssh_Thre usableKeys.push_back(RawKeyToKeyId(binaryKey.data(), binaryKey.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[0], 0); @@ -1178,7 +1178,7 @@ TEST_F(DrmSessionManagerComplexTests, ValidateMultiKeySlot_RealWidevinePssh_Sing usableKeys.push_back(RawKeyToKeyId(binaryKey.data(), binaryKey.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[0], 0); @@ -1234,7 +1234,7 @@ TEST_F(DrmSessionManagerComplexTests, ValidateMultiKeySlot_RealWidevinePssh_Part usableKeys.push_back(RawKeyToKeyId(binaryKey3.data(), binaryKey3.size())); EXPECT_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()) - .WillRepeatedly(ReturnRef(usableKeys)); + .WillRepeatedly(Return(usableKeys)); // Validate with second key (should succeed as it's in usableKeys) bool result = mDrmSessionManager->ValidateMultiKeySlot(keyIDs[1], 0); diff --git a/test/utests/tests/DrmTests/CMakeLists.txt b/test/utests/tests/DrmTests/CMakeLists.txt index bb7b054e..ab3e97a0 100755 --- a/test/utests/tests/DrmTests/CMakeLists.txt +++ b/test/utests/tests/DrmTests/CMakeLists.txt @@ -81,7 +81,6 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt b/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt index 3eb775cf..94ec66a9 100644 --- a/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt +++ b/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt @@ -50,7 +50,6 @@ set(TEST_SOURCES FunctionalTests.cpp set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/externals/PlayerExternalsInterface.cpp ${PLAYER_ROOT}/externals/PlayerExternalUtils.cpp diff --git a/test/utests/tests/OCDMSessionAdapter/FunctionalTests.cpp b/test/utests/tests/OCDMSessionAdapter/FunctionalTests.cpp index e3805f60..0c6f477c 100644 --- a/test/utests/tests/OCDMSessionAdapter/FunctionalTests.cpp +++ b/test/utests/tests/OCDMSessionAdapter/FunctionalTests.cpp @@ -81,7 +81,17 @@ TEST_F(OCDMSessionAdapterTests, generateDRMSession) const char *initDataType = "cenc"; uint8_t initDataTypeLen = strlen(initDataType); - ((*g_mockopencdm).gmock_opencdm_construct_session(ocdmSystem, LicenseType::Temporary, MemBufEq(initDataType, initDataTypeLen), MemBufEq(initData, initDataLen), f_cbInitData, MemBufEq(customData.c_str(), customData.length()), customData.length(),_,_,_))(::testing::internal::GetWithoutMatchers(), nullptr) .InternalExpectedAt("/home/rekha/RDK/latest/l1_Final/aamp/middleware/test/utests/tests/OCDMSessionAdapter/FunctionalTests.cpp", 91, "*g_mockopencdm", "opencdm_construct_session(ocdmSystem, LicenseType::Temporary, MemBufEq(initDataType, initDataTypeLen), MemBufEq(initData, initDataLen), f_cbInitData, MemBufEq(customData.c_str(), customData.length()), customData.length(),_,_,_)").WillOnce(Return(ERROR_NONE)); + EXPECT_CALL(*g_mockopencdm, + opencdm_construct_session( + ocdmSystem, + LicenseType::Temporary, + MemBufEq(initDataType, initDataTypeLen), + MemBufEq(initData, initDataLen), + f_cbInitData, + MemBufEq(customData.c_str(), customData.length()), + customData.length(), + _, _, _)) + .WillOnce(Return(ERROR_NONE)); m_ocdmsessionadapter->generateDRMSession(initData, f_cbInitData, customData); } diff --git a/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt b/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt index 0eb9af4a..013b9340 100644 --- a/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt +++ b/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt @@ -46,7 +46,6 @@ set(TEST_SOURCES FunctionalTests.cpp set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp - ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp ${PLAYER_ROOT}/ProcessHandler.cpp diff --git a/test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp b/test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp index 8d5157bd..9cdad983 100644 --- a/test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp +++ b/test/utests/tests/OcdmBasicSessionAdapterTests/FunctionalTests.cpp @@ -59,7 +59,6 @@ std::shared_ptr drmHelper; DrmInfo drminfo; MockDrmMemorySystem *g_mockMemorySystem; static std::string g_defaultSystemId = "com.widevine.alpha"; -static std::vector> g_emptyKeys; class OcdmBasicSessionAdapterTests : public ::testing::Test { @@ -82,7 +81,7 @@ class OcdmBasicSessionAdapterTests : public ::testing::Test g_mockOpenCdmSessionAdapter = new NiceMock(); // Set default return value for getUsableKeys() to return an empty vector - ON_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()).WillByDefault(testing::ReturnRef(g_emptyKeys)); + ON_CALL(*g_mockOpenCdmSessionAdapter, getUsableKeys()).WillByDefault(testing::Return(std::vector>{})); g_mockMemorySystem = new NiceMock(); } diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/CMakeLists.txt b/test/utests/tests/PlayerDirectRialtoCCManagerTests/CMakeLists.txt new file mode 100644 index 00000000..f49ec9d6 --- /dev/null +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/CMakeLists.txt @@ -0,0 +1,68 @@ +# If not stated otherwise in this file or this component's license file the +# following copyright and licenses apply: +# +# Copyright 2026 RDK Management +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +include(GoogleTest) + +set(PLAYER_ROOT "../../../../") +set(UTESTS_ROOT "../../") +set(EXEC_NAME PlayerDirectRialtoCCManagerTests) + +include_directories(${PLAYER_ROOT} + ${PLAYER_ROOT}/closedcaptions + ${PLAYER_ROOT}/closedcaptions/direct-rialto + ${PLAYER_ROOT}/playerLogManager) + +include_directories(${GTEST_INCLUDE_DIRS}) +include_directories(${GMOCK_INCLUDE_DIRS}) +include_directories(${GLIB_INCLUDE_DIRS}) +include_directories(${GSTREAMER_INCLUDE_DIRS}) +include_directories(SYSTEM ${UTESTS_ROOT}/mocks) + +set(TEST_SOURCES + PlayerDirectRialtoCCManagerTests.cpp + PlayerDirectRialtoCCManagerTestCases.cpp + PlayerDirectRialtoCCManagerBaseStubs.cpp) + +set(PLAYER_SOURCES + ${PLAYER_ROOT}/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp + ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp) + +add_executable(${EXEC_NAME} + ${TEST_SOURCES} + ${PLAYER_SOURCES}) + +set_target_properties(${EXEC_NAME} PROPERTIES FOLDER "utests") + +if (CMAKE_XCODE_BUILD_SYSTEM) + # XCode schema target + xcode_define_schema(${EXEC_NAME}) +endif() + +if (COVERAGE_ENABLED) + include(CodeCoverage) + APPEND_COVERAGE_COMPILER_FLAGS() +endif() + +target_link_libraries(${EXEC_NAME} + fakes + -pthread + ${GLIB_LINK_LIBRARIES} + ${OS_LD_FLAGS} + ${GMOCK_LINK_LIBRARIES} + ${GTEST_LINK_LIBRARIES}) + +player_utest_run_add(${EXEC_NAME}) \ No newline at end of file diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp new file mode 100644 index 00000000..1b30baeb --- /dev/null +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp @@ -0,0 +1,80 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "PlayerCCManager.h" + +int PlayerCCManagerBase::Init(void *handle) +{ + (void) handle; + return 0; +} + +void PlayerCCManagerBase::RestoreCC(bool shouldRestoreCC) +{ + if (!mEnabled && shouldRestoreCC) + { + mEnabled = shouldRestoreCC; + } +} + +bool PlayerCCManagerBase::IsOOBCCRenderingSupported() +{ + return false; +} + +int PlayerCCManagerBase::SetStatus(bool enable) +{ + mEnabled = enable; + return 0; +} + +int PlayerCCManagerBase::SetStyle(const std::string &options) +{ + mOptions = options; + return 0; +} + +int PlayerCCManagerBase::SetTrack(const std::string &track, + const CCFormat format) +{ + mTrack = track; + mTrackFormat = format; + return 0; +} + +void PlayerCCManagerBase::SetTrickplayStatus(bool enable) +{ + mTrickplayStarted = enable; +} + +void PlayerCCManagerBase::SetParentalControlStatus(bool locked) +{ + mParentalCtrlLocked = locked; +} + +void PlayerCCManagerBase::ResetState() +{ + mOptions.clear(); + mTrack.clear(); + mTrackFormat = eCLOSEDCAPTION_FORMAT_DEFAULT; + mLastTextTracks.clear(); + mEnabled = false; + mTrickplayStarted = false; + mParentalCtrlLocked = false; +} \ No newline at end of file diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp new file mode 100644 index 00000000..9a87fbf4 --- /dev/null +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -0,0 +1,217 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file PlayerDirectRialtoCCManagerTestCases.cpp + * @brief L1 unit tests for PlayerDirectRialtoCCManager. + * + * Tests verify the CC manager's orchestration of its IDirectRialtoCC + * dependency using a strict mock. Per the L1 golden rule, every assertion + * verifies how the component under test (PlayerDirectRialtoCCManager) + * behaves, not how the mock behaves. + */ + +#include +#include +#include + +#include "PlayerDirectRialtoCCManager.h" +#include "IDirectRialtoCC.h" + +using ::testing::_; +using ::testing::Return; +using ::testing::StrictMock; + +class MockIDirectRialtoCC : public IDirectRialtoCC +{ +public: + MOCK_METHOD(bool, setTextTrackIdentifier, (const std::string &id), (override)); + MOCK_METHOD(bool, setCCMute, (bool muted), (override)); +}; + +class PlayerDirectRialtoCCManagerTestable : public PlayerDirectRialtoCCManager +{ +public: + using PlayerDirectRialtoCCManager::Initialize; + using PlayerDirectRialtoCCManager::StartRendering; + using PlayerDirectRialtoCCManager::StopRendering; + using PlayerDirectRialtoCCManager::ResetState; +}; + +class PlayerDirectRialtoCCManagerTest : public ::testing::Test +{ +protected: + void SetUp() override + { + m_mock = std::make_unique>(); + } + + void TearDown() override + { + m_mock.reset(); + } + + void InitWithDefaultTrack() + { + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .Times(1) + .WillOnce(Return(true)); + m_mgr.Initialize(m_mock.get()); + } + + PlayerDirectRialtoCCManagerTestable m_mgr; + std::unique_ptr m_mock; +}; + +/** + * @test Initialize_WithNullHandle_DoesNotCallControl + * @brief Passing nullptr must not crash and must not call the mock. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_WithNullHandle_DoesNotCallControl) +{ + EXPECT_NO_FATAL_FAILURE(m_mgr.Initialize(nullptr)); +} + +/** + * @test Initialize_WithHandle_SetsDefaultTrackCC1 + * @brief When no track is cached, Initialize() should apply "CC1" as the + * default text-track identifier. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_WithHandle_SetsDefaultTrackCC1) +{ + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.Initialize(m_mock.get()); +} + +/** + * @test Initialize_WithCachedTrack_ReappliesCachedTrack + * @brief When a track has already been cached via SetTrack(), Initialize() + * with a new handle must re-apply the cached identifier, not "CC1". + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_WithCachedTrack_ReappliesCachedTrack) +{ + m_mgr.SetTrack("CC3"); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC3")) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.Initialize(m_mock.get()); +} + +/** + * @test SetTrack_NumericWith608Format_PrependsCCPrefix + * @brief A numeric track string with 608 format should become "CC". + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + SetTrack_NumericWith608Format_PrependsCCPrefix) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC2")) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.SetTrack("2", eCLOSEDCAPTION_FORMAT_608); +} + +/** + * @test SetTrack_NumericWith708Format_PrependsServicePrefix + * @brief A numeric track string with 708 format should become "SERVICE". + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + SetTrack_NumericWith708Format_PrependsServicePrefix) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("SERVICE3")) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.SetTrack("3", eCLOSEDCAPTION_FORMAT_708); +} + +/** + * @test SetTrack_AlphabeticTrack_PassedThrough + * @brief A track string that already has an alphabetic prefix must be + * forwarded unchanged. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + SetTrack_AlphabeticTrack_PassedThrough) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.SetTrack("CC1"); +} + +/** + * @test StartRendering_CallsCCMuteFalse + * @brief StartRendering() must un-mute CC by calling setCCMute(false). + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + StartRendering_CallsCCMuteFalse) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setCCMute(false)) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.StartRendering(); +} + +/** + * @test StopRendering_CallsCCMuteTrue + * @brief StopRendering() must mute CC by calling setCCMute(true). + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + StopRendering_CallsCCMuteTrue) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setCCMute(true)) + .Times(1) + .WillOnce(Return(true)); + + m_mgr.StopRendering(); +} + +/** + * @test ResetState_ClearsControlHandle + * @brief After ResetState(), StartRendering() must not call the old mock. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + ResetState_ClearsControlHandle) +{ + InitWithDefaultTrack(); + + m_mgr.ResetState(); + + EXPECT_NO_FATAL_FAILURE(m_mgr.StartRendering()); +} \ No newline at end of file diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTests.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTests.cpp new file mode 100644 index 00000000..9be451e7 --- /dev/null +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTests.cpp @@ -0,0 +1,31 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2026 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * @file PlayerDirectRialtoCCManagerTests.cpp + * @brief Test runner for PlayerDirectRialtoCCManager unit tests. + */ + +#include + +int main(int argc, char **argv) +{ + testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} \ No newline at end of file From b9111395cac2b135cfefa26b9e985cd053c6390b Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Mon, 3 Aug 2026 16:25:12 +0000 Subject: [PATCH 04/23] Add IDirectRialtoCC.h to install files --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index ffcf5373..d53d7498 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -214,6 +214,7 @@ install(FILES closedcaptions/CCTrackInfo.h subtec/libsubtec/SubtecPacket.hpp playerisobmff/playerisobmffbuffer.h playerisobmff/playerisobmffbox.h + closedcaptions/direct-rialto/IDirectRialtoCC.h DESTINATION include) set(SOURCES From 2799529ea2c8e420c6d737c16f7440a965f69ac3 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Tue, 4 Aug 2026 10:40:21 +0000 Subject: [PATCH 05/23] Restore DrmSession.cpp --- drm/DrmSession.cpp | 121 +++++++++++++++++++++++++++++++++++++++++++++ drm/DrmSession.h | 6 +-- 2 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 drm/DrmSession.cpp diff --git a/drm/DrmSession.cpp b/drm/DrmSession.cpp new file mode 100644 index 00000000..a735cb6f --- /dev/null +++ b/drm/DrmSession.cpp @@ -0,0 +1,121 @@ +/* + * If not stated otherwise in this file or this component's license file the + * following copyright and licenses apply: + * + * Copyright 2025 RDK Management + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. +*/ + +/** +* @file DrmSession.cpp +* @brief Source file for DrmSession. +*/ + +#include "DrmSession.h" +#include "PlayerLogManager.h" +#include + +/** + * @brief Constructor for DrmSession. + */ +DrmSession::DrmSession(const string &keySystem) : m_keySystem(keySystem),m_OutputProtectionEnabled(false) + , mContentSecurityManagerSession() + , mLifecycleMutex() + , mLifecycleCV() + , mActiveOperations(0) + , mMarkedForDestruction(false) +{ +} + +/** + * @brief Destructor for DrmSession.. + */ +DrmSession::~DrmSession() +{ +} + +/** + * @brief DELIA-70726 fix: Acquire lifecycle guard before use in decrypt(). + */ +bool DrmSession::AcquireForUse() +{ + std::lock_guard lock(mLifecycleMutex); + if (mMarkedForDestruction) + { + return false; + } + mActiveOperations++; + return true; +} + +/** + * @brief DELIA-70726 fix: Release lifecycle guard after use in decrypt(). + */ +void DrmSession::ReleaseAfterUse() +{ + std::lock_guard lock(mLifecycleMutex); + if (mActiveOperations > 0) + { + mActiveOperations--; + } + if (mActiveOperations == 0) + { + mLifecycleCV.notify_all(); + } +} + +/** + * @brief DELIA-70726 fix: Block deletion of this session until any decrypt() + * call already in progress (having acquired the guard) has finished. + */ +void DrmSession::PrepareForDestruction(uint32_t timeoutMs) +{ + std::unique_lock lock(mLifecycleMutex); + mMarkedForDestruction = true; + if (mActiveOperations > 0) + { + MW_LOG_WARN("DrmSession::PrepareForDestruction : waiting for %d in-flight decrypt operation(s) to complete before delete", mActiveOperations); + mLifecycleCV.wait_for(lock, std::chrono::milliseconds(timeoutMs), [this]() { return mActiveOperations == 0; }); + if (mActiveOperations > 0) + { + MW_LOG_ERR("DrmSession::PrepareForDestruction : timed out waiting for in-flight decrypt operation(s); proceeding with destruction"); + } + } +} + +/** + * @brief Get the DRM System, ie, UUID for PlayReady WideVine etc.. + */ +string DrmSession::getKeySystem() +{ + return m_keySystem; +} + +/** + * @brief Function to decrypt GStreamer stream buffer. + */ +int DrmSession::decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps) +{ + MW_LOG_ERR("GST decrypt method not implemented"); + return -1; +} + +/** + * @brief Function to decrypt stream buffer. + */ +int DrmSession::decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData) +{ + MW_LOG_ERR("Standard decrypt method not implemented"); + return -1; +} diff --git a/drm/DrmSession.h b/drm/DrmSession.h index 3fc429f5..3927a451 100755 --- a/drm/DrmSession.h +++ b/drm/DrmSession.h @@ -177,7 +177,7 @@ class DrmSession * @param caps : Caps of the media that is currently being decrypted * @retval Returns status of decrypt request. */ - virtual int decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps = NULL) { return -1; }; + virtual int decrypt(GstBuffer* keyIDBuffer, GstBuffer* ivBuffer, GstBuffer* buffer, unsigned subSampleCount, GstBuffer* subSamplesBuffer, GstCaps* caps = NULL); /** * @fn decrypt @@ -188,7 +188,7 @@ class DrmSession * @param ppOpaqueData : pointer to opaque buffer in case of SVP. * @retval Returns status of decrypt request. */ - virtual int decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData) { return -1; }; + virtual int decrypt(const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData); /** * @brief Get the current state of DRM Session. @@ -230,7 +230,7 @@ class DrmSession * @fn getKeySystem * @retval DRM system uuid */ - string getKeySystem() { return m_keySystem; }; + string getKeySystem(); /** * @brief Set the OutputProtection for DRM Session From b83aeabe279ebb3637e617c7949430cf2983f931 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Tue, 4 Aug 2026 10:40:21 +0000 Subject: [PATCH 06/23] VPAAMP-867 Restore DrmSession.cpp Reason for Change: Fix AAMP compilation failure after rebase Summary of Changes: - Restore the file DrmSession.cpp - Revert some of the changes made to DrmSession.h - Restore fakes, CMake files and references to DrmSession.cpp Test Procedure: Build AAMP-CLI and AAMP L1 tests Priority: P1 Risks: Low --- CMakeLists.txt | 1 + drm/DrmSession.h | 7 ++----- test/utests/fakes/FakeDRMSession.cpp | 4 ++++ test/utests/tests/DrmAes/CMakeLists.txt | 1 + test/utests/tests/DrmHelperTests/CMakeLists.txt | 1 + test/utests/tests/DrmOcdmTests/CMakeLists.txt | 1 + test/utests/tests/DrmSecureClient/CMakeLists.txt | 1 + test/utests/tests/DrmSessionManagerTests/CMakeLists.txt | 1 + test/utests/tests/DrmTests/CMakeLists.txt | 1 + test/utests/tests/OCDMSessionAdapter/CMakeLists.txt | 1 + .../tests/OcdmBasicSessionAdapterTests/CMakeLists.txt | 1 + 11 files changed, 15 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d53d7498..b3137469 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -242,6 +242,7 @@ endif() set(LIBPLAYERGSTINTERFACE_DRM_SOURCES drm/PlayerHlsDrmSessionInterface.cpp drm/DrmSessionManager.cpp + drm/DrmSession.cpp drm/DrmSessionFactory.cpp drm/helper/DrmHelper.cpp drm/helper/DrmHelperFactory.cpp diff --git a/drm/DrmSession.h b/drm/DrmSession.h index 3927a451..d96290f0 100755 --- a/drm/DrmSession.h +++ b/drm/DrmSession.h @@ -91,10 +91,7 @@ class DrmSession * @fn DrmSession * @param keySystem : DRM key system uuid */ - DrmSession(const string &keySystem) : - m_keySystem(keySystem), - m_OutputProtectionEnabled(false), - mContentSecurityManagerSession() { }; + DrmSession(const string &keySystem); /** * @brief Copy constructor disabled @@ -105,7 +102,7 @@ class DrmSession /** * @fn ~DrmSession */ - virtual ~DrmSession() = default; + virtual ~DrmSession(); /** * @brief assignment operator disabled diff --git a/test/utests/fakes/FakeDRMSession.cpp b/test/utests/fakes/FakeDRMSession.cpp index 6780396b..da347f6d 100644 --- a/test/utests/fakes/FakeDRMSession.cpp +++ b/test/utests/fakes/FakeDRMSession.cpp @@ -19,3 +19,7 @@ #include "DrmSession.h" +string DrmSession::getKeySystem() +{ + return ""; +} diff --git a/test/utests/tests/DrmAes/CMakeLists.txt b/test/utests/tests/DrmAes/CMakeLists.txt index ae2d6fae..79360faf 100755 --- a/test/utests/tests/DrmAes/CMakeLists.txt +++ b/test/utests/tests/DrmAes/CMakeLists.txt @@ -65,6 +65,7 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmHelperTests/CMakeLists.txt b/test/utests/tests/DrmHelperTests/CMakeLists.txt index d78c9cbe..56e0ca20 100644 --- a/test/utests/tests/DrmHelperTests/CMakeLists.txt +++ b/test/utests/tests/DrmHelperTests/CMakeLists.txt @@ -72,6 +72,7 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmOcdmTests/CMakeLists.txt b/test/utests/tests/DrmOcdmTests/CMakeLists.txt index ff764823..7204420d 100644 --- a/test/utests/tests/DrmOcdmTests/CMakeLists.txt +++ b/test/utests/tests/DrmOcdmTests/CMakeLists.txt @@ -69,6 +69,7 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmGstSessionAdapter.cpp diff --git a/test/utests/tests/DrmSecureClient/CMakeLists.txt b/test/utests/tests/DrmSecureClient/CMakeLists.txt index 48e3f78e..ab46343b 100644 --- a/test/utests/tests/DrmSecureClient/CMakeLists.txt +++ b/test/utests/tests/DrmSecureClient/CMakeLists.txt @@ -66,6 +66,7 @@ if(SEC_CLIENT_HEADERS) ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt b/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt index f91c6819..f42e930b 100644 --- a/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt +++ b/test/utests/tests/DrmSessionManagerTests/CMakeLists.txt @@ -60,6 +60,7 @@ set(UTILITY_SOURCES set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/helper/WidevineDrmHelper.cpp diff --git a/test/utests/tests/DrmTests/CMakeLists.txt b/test/utests/tests/DrmTests/CMakeLists.txt index ab3e97a0..bb7b054e 100755 --- a/test/utests/tests/DrmTests/CMakeLists.txt +++ b/test/utests/tests/DrmTests/CMakeLists.txt @@ -81,6 +81,7 @@ set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp ${PLAYER_ROOT}/drm/helper/DrmHelperFactory.cpp ${PLAYER_ROOT}/drm/DrmSessionManager.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmJsonObject.cpp ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp diff --git a/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt b/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt index 94ec66a9..3eb775cf 100644 --- a/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt +++ b/test/utests/tests/OCDMSessionAdapter/CMakeLists.txt @@ -50,6 +50,7 @@ set(TEST_SOURCES FunctionalTests.cpp set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/ocdm/opencdmsessionadapter.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/drm/DrmUtils.cpp ${PLAYER_ROOT}/externals/PlayerExternalsInterface.cpp ${PLAYER_ROOT}/externals/PlayerExternalUtils.cpp diff --git a/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt b/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt index 013b9340..0eb9af4a 100644 --- a/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt +++ b/test/utests/tests/OcdmBasicSessionAdapterTests/CMakeLists.txt @@ -46,6 +46,7 @@ set(TEST_SOURCES FunctionalTests.cpp set(PLAYER_SOURCES ${PLAYER_ROOT}/drm/ocdm/OcdmBasicSessionAdapter.cpp ${PLAYER_ROOT}/drm/helper/DrmHelper.cpp + ${PLAYER_ROOT}/drm/DrmSession.cpp ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp ${PLAYER_ROOT}/playerLogManager/PlayerLogManager.cpp ${PLAYER_ROOT}/ProcessHandler.cpp From 14edcaa1bc14605f46d3b5610156292b0095aa95 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 4 Aug 2026 21:51:02 +0000 Subject: [PATCH 07/23] Fix race in IsKeyIdProcessed() causing false-positive DRM session status IsKeyIdProcessed() only held cachedKeyMutex, so it could observe a slot another thread had just claimed (before createDrmSession() finished acquiring the license) and report it as already succeeded. It also only checked the per-entry isFailedKeyId flag, missing whole-slot failures tracked in isFailedKeyEntries. Now also takes mDrmSessionLock (same order createDrmSession() already uses) so it blocks until any in-flight session creation for that key has resolved, and checks isFailedKeyEntries too. Add regression tests to DrmSessionManagerTestCases.cpp. --- drm/DrmSessionManager.cpp | 8 ++- drm/DrmSessionManager.h | 4 ++ .../tests/ClosedCaptionsTests/CMakeLists.txt | 1 + .../DrmSessionManagerTestCases.cpp | 72 +++++++++++++++++++ 4 files changed, 84 insertions(+), 1 deletion(-) diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 383e523a..5b8f0dc8 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -352,6 +352,12 @@ string _extractSubstring(string parentStr, string startStr, string endStr) bool DrmSessionManager::IsKeyIdProcessed(std::vector keyIdArray, bool &status) { bool ret = false; + // createDrmSession() holds mDrmSessionLock for its whole duration (slot + // claim through license acquisition). Taking it here first blocks until + // any in-flight createDrmSession() call for this keyId has fully + // resolved, so the cachedKeyIDs state read below is a final outcome + // rather than a slot that has merely been claimed by another thread. + std::lock_guard sessionGuard(mDrmSessionLock); std::lock_guard guard(cachedKeyMutex); for (int sessionSlot = 0; sessionSlot < mMaxDRMSessions; sessionSlot++) { @@ -365,7 +371,7 @@ bool DrmSessionManager::IsKeyIdProcessed(std::vector keyIdArray, bool & if (it != keyIDSlot.end()) { std::string debugStr = PlayerLogManager::getHexDebugStr(keyIdArray); - status = !it->isFailedKeyId; + status = !it->isFailedKeyId && !cachedKeyIDs[sessionSlot].isFailedKeyEntries; MW_LOG_INFO("Session created/in progress with same keyID %s at slot %d, status=%d", debugStr.c_str(), sessionSlot, status); ret = true; break; diff --git a/drm/DrmSessionManager.h b/drm/DrmSessionManager.h index 7b4dd58c..09ab52e9 100644 --- a/drm/DrmSessionManager.h +++ b/drm/DrmSessionManager.h @@ -323,6 +323,10 @@ class DrmSessionManager * @param[out] status - processed status of the key id success/fail * @return bool - true if keyId is already marked as failed or processed, * false if key is not cached + * @note Blocks until any concurrent createDrmSession() call has + * completed (shares mDrmSessionLock with createDrmSession), + * so callers get a final, non-racy outcome rather than a + * snapshot of a slot that has merely been claimed. */ bool IsKeyIdProcessed(std::vector keyIdArray, bool &status); /** diff --git a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt index 296294f6..3184fac5 100644 --- a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt +++ b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt @@ -29,6 +29,7 @@ include_directories(${PLAYER_ROOT}/subtec/subtecparser) include_directories(${PLAYER_ROOT}/playerJsonObject) include_directories(${PLAYER_ROOT}/playerLogManager) include_directories(${PLAYER_ROOT}/mp4demux) +include_directories(${PLAYER_ROOT}/closedcaptions/direct-rialto) include_directories(${PLAYER_ROOT}/closedcaptions/rialto) include_directories(${GTEST_INCLUDE_DIRS}) include_directories(${GMOCK_INCLUDE_DIRS}) diff --git a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp index acac0af8..7cec2429 100644 --- a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp +++ b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp @@ -492,6 +492,78 @@ TEST_F(DrmSessionManagerComplexTests, KeyIdEntries_PrimaryKeyFails_MarksEntireFa EXPECT_TRUE(entries.isFailedKeyEntries); } +/** + * @brief IsKeyIdProcessed should report success when a cached keyId's own + * entry and slot are both unmarked as failed. + */ +TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_CachedKeyNotFailed_ReportsSuccess) +{ + std::vector keyId = {0x01, 0x02, 0x03, 0x04}; + + KeyIdEntries entries; + KeyIdEntry entry; + entry.keyId = keyId; + entry.isFailedKeyId = false; + entries.data.push_back(entry); + entries.isFailedKeyEntries = false; + mDrmSessionManager->AddKeyIdEntryToCache(0, entries); + + bool status = false; + bool found = mDrmSessionManager->IsKeyIdProcessed(keyId, status); + + EXPECT_TRUE(found); + EXPECT_TRUE(status); +} + +/** + * @brief Regression test: a keyId whose per-entry isFailedKeyId flag is + * false must still be reported as failed when the whole slot has been + * marked isFailedKeyEntries (set on session/license failure paths that + * only set the slot-level flag). Prior to the fix, IsKeyIdProcessed only + * checked the per-entry flag and could incorrectly report a failed + * session as processed/successful. + */ +TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_SlotMarkedFailed_ReportsFailureEvenIfEntryFlagClear) +{ + std::vector keyId = {0x05, 0x06, 0x07, 0x08}; + + KeyIdEntries entries; + KeyIdEntry entry; + entry.keyId = keyId; + entry.isFailedKeyId = false; // per-entry flag never set on this path + entries.data.push_back(entry); + entries.isFailedKeyEntries = true; // whole slot failed (e.g. license/session creation failure) + mDrmSessionManager->AddKeyIdEntryToCache(0, entries); + + bool status = true; // pre-set to ensure it's actually written to false + bool found = mDrmSessionManager->IsKeyIdProcessed(keyId, status); + + EXPECT_TRUE(found); + EXPECT_FALSE(status); +} + +/** + * @brief IsKeyIdProcessed should report "not found" for a keyId that has + * never been cached in any slot. + */ +TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_UnknownKeyId_ReportsNotFound) +{ + std::vector cachedKeyId = {0x01, 0x02, 0x03, 0x04}; + std::vector unknownKeyId = {0xAA, 0xBB, 0xCC, 0xDD}; + + KeyIdEntries entries; + KeyIdEntry entry; + entry.keyId = cachedKeyId; + entry.isFailedKeyId = false; + entries.data.push_back(entry); + mDrmSessionManager->AddKeyIdEntryToCache(0, entries); + + bool status = false; + bool found = mDrmSessionManager->IsKeyIdProcessed(unknownKeyId, status); + + EXPECT_FALSE(found); +} + /** * @brief Test hex string key ID vs binary key ID comparison */ From 0df1f3efcafeb937bc12615df0c4b1ad98145d20 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Wed, 5 Aug 2026 11:35:40 +0000 Subject: [PATCH 08/23] Revert "Fix race in IsKeyIdProcessed() causing false-positive DRM session status" This reverts commit 14edcaa1bc14605f46d3b5610156292b0095aa95. --- drm/DrmSessionManager.cpp | 8 +-- drm/DrmSessionManager.h | 4 -- .../tests/ClosedCaptionsTests/CMakeLists.txt | 1 - .../DrmSessionManagerTestCases.cpp | 72 ------------------- 4 files changed, 1 insertion(+), 84 deletions(-) diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 5b8f0dc8..383e523a 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -352,12 +352,6 @@ string _extractSubstring(string parentStr, string startStr, string endStr) bool DrmSessionManager::IsKeyIdProcessed(std::vector keyIdArray, bool &status) { bool ret = false; - // createDrmSession() holds mDrmSessionLock for its whole duration (slot - // claim through license acquisition). Taking it here first blocks until - // any in-flight createDrmSession() call for this keyId has fully - // resolved, so the cachedKeyIDs state read below is a final outcome - // rather than a slot that has merely been claimed by another thread. - std::lock_guard sessionGuard(mDrmSessionLock); std::lock_guard guard(cachedKeyMutex); for (int sessionSlot = 0; sessionSlot < mMaxDRMSessions; sessionSlot++) { @@ -371,7 +365,7 @@ bool DrmSessionManager::IsKeyIdProcessed(std::vector keyIdArray, bool & if (it != keyIDSlot.end()) { std::string debugStr = PlayerLogManager::getHexDebugStr(keyIdArray); - status = !it->isFailedKeyId && !cachedKeyIDs[sessionSlot].isFailedKeyEntries; + status = !it->isFailedKeyId; MW_LOG_INFO("Session created/in progress with same keyID %s at slot %d, status=%d", debugStr.c_str(), sessionSlot, status); ret = true; break; diff --git a/drm/DrmSessionManager.h b/drm/DrmSessionManager.h index 09ab52e9..7b4dd58c 100644 --- a/drm/DrmSessionManager.h +++ b/drm/DrmSessionManager.h @@ -323,10 +323,6 @@ class DrmSessionManager * @param[out] status - processed status of the key id success/fail * @return bool - true if keyId is already marked as failed or processed, * false if key is not cached - * @note Blocks until any concurrent createDrmSession() call has - * completed (shares mDrmSessionLock with createDrmSession), - * so callers get a final, non-racy outcome rather than a - * snapshot of a slot that has merely been claimed. */ bool IsKeyIdProcessed(std::vector keyIdArray, bool &status); /** diff --git a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt index 3184fac5..296294f6 100644 --- a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt +++ b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt @@ -29,7 +29,6 @@ include_directories(${PLAYER_ROOT}/subtec/subtecparser) include_directories(${PLAYER_ROOT}/playerJsonObject) include_directories(${PLAYER_ROOT}/playerLogManager) include_directories(${PLAYER_ROOT}/mp4demux) -include_directories(${PLAYER_ROOT}/closedcaptions/direct-rialto) include_directories(${PLAYER_ROOT}/closedcaptions/rialto) include_directories(${GTEST_INCLUDE_DIRS}) include_directories(${GMOCK_INCLUDE_DIRS}) diff --git a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp index 7cec2429..acac0af8 100644 --- a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp +++ b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp @@ -492,78 +492,6 @@ TEST_F(DrmSessionManagerComplexTests, KeyIdEntries_PrimaryKeyFails_MarksEntireFa EXPECT_TRUE(entries.isFailedKeyEntries); } -/** - * @brief IsKeyIdProcessed should report success when a cached keyId's own - * entry and slot are both unmarked as failed. - */ -TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_CachedKeyNotFailed_ReportsSuccess) -{ - std::vector keyId = {0x01, 0x02, 0x03, 0x04}; - - KeyIdEntries entries; - KeyIdEntry entry; - entry.keyId = keyId; - entry.isFailedKeyId = false; - entries.data.push_back(entry); - entries.isFailedKeyEntries = false; - mDrmSessionManager->AddKeyIdEntryToCache(0, entries); - - bool status = false; - bool found = mDrmSessionManager->IsKeyIdProcessed(keyId, status); - - EXPECT_TRUE(found); - EXPECT_TRUE(status); -} - -/** - * @brief Regression test: a keyId whose per-entry isFailedKeyId flag is - * false must still be reported as failed when the whole slot has been - * marked isFailedKeyEntries (set on session/license failure paths that - * only set the slot-level flag). Prior to the fix, IsKeyIdProcessed only - * checked the per-entry flag and could incorrectly report a failed - * session as processed/successful. - */ -TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_SlotMarkedFailed_ReportsFailureEvenIfEntryFlagClear) -{ - std::vector keyId = {0x05, 0x06, 0x07, 0x08}; - - KeyIdEntries entries; - KeyIdEntry entry; - entry.keyId = keyId; - entry.isFailedKeyId = false; // per-entry flag never set on this path - entries.data.push_back(entry); - entries.isFailedKeyEntries = true; // whole slot failed (e.g. license/session creation failure) - mDrmSessionManager->AddKeyIdEntryToCache(0, entries); - - bool status = true; // pre-set to ensure it's actually written to false - bool found = mDrmSessionManager->IsKeyIdProcessed(keyId, status); - - EXPECT_TRUE(found); - EXPECT_FALSE(status); -} - -/** - * @brief IsKeyIdProcessed should report "not found" for a keyId that has - * never been cached in any slot. - */ -TEST_F(DrmSessionManagerComplexTests, IsKeyIdProcessed_UnknownKeyId_ReportsNotFound) -{ - std::vector cachedKeyId = {0x01, 0x02, 0x03, 0x04}; - std::vector unknownKeyId = {0xAA, 0xBB, 0xCC, 0xDD}; - - KeyIdEntries entries; - KeyIdEntry entry; - entry.keyId = cachedKeyId; - entry.isFailedKeyId = false; - entries.data.push_back(entry); - mDrmSessionManager->AddKeyIdEntryToCache(0, entries); - - bool status = false; - bool found = mDrmSessionManager->IsKeyIdProcessed(unknownKeyId, status); - - EXPECT_FALSE(found); -} - /** * @brief Test hex string key ID vs binary key ID comparison */ From 19f7eb03d876fd487ca288d458cae8753625648b Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Wed, 5 Aug 2026 22:19:41 +0000 Subject: [PATCH 09/23] Added mNaluLengthPrefixed to MediaCodecInfo True when NAL units are length-prefixed (AVCC/HVCC, e.g. avcC/hvcC sample entries per ISO/IEC 14496-15); false for Annex-B (start-code delimited) bitstreams such as HLS-TS ES output. Meaningless for non-NAL-unit codecs (audio/subtitle), where it stays false. --- DemuxDataTypes.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/DemuxDataTypes.h b/DemuxDataTypes.h index bba1f12c..76d73d48 100644 --- a/DemuxDataTypes.h +++ b/DemuxDataTypes.h @@ -94,6 +94,11 @@ struct MediaCodecInfo GstStreamOutputFormat mCodecFormat; // GST_FORMAT_VIDEO_ES_H264, etc std::vector mCodecData; // codec private data, e.g. avcC box bool mIsEncrypted; + // True when NAL units are length-prefixed (AVCC/HVCC, e.g. avcC/hvcC + // sample entries per ISO/IEC 14496-15); false for Annex-B (start-code + // delimited) bitstreams such as HLS-TS ES output. Meaningless for + // non-NAL-unit codecs (audio/subtitle), where it stays false. + bool mNaluLengthPrefixed; union { struct @@ -116,7 +121,7 @@ struct MediaCodecInfo * Uniform initialization is preferred for C++ types as it's type-safe, clearer in intent, * and works correctly with all C++ types including those with constructors. */ - MediaCodecInfo() : mCodecFormat(GST_FORMAT_INVALID), mIsEncrypted(false), mCodecData(), mInfo{0} + MediaCodecInfo() : mCodecFormat(GST_FORMAT_INVALID), mIsEncrypted(false), mNaluLengthPrefixed(false), mCodecData(), mInfo{0} { } @@ -128,7 +133,7 @@ struct MediaCodecInfo * Uniform initialization is preferred for C++ types as it's type-safe, clearer in intent, * and works correctly with all C++ types including those with constructors. */ - MediaCodecInfo(GstStreamOutputFormat format) : mCodecFormat(format), mIsEncrypted(false), mCodecData(), mInfo{0} + MediaCodecInfo(GstStreamOutputFormat format) : mCodecFormat(format), mIsEncrypted(false), mNaluLengthPrefixed(false), mCodecData(), mInfo{0} { } @@ -144,6 +149,7 @@ struct MediaCodecInfo : mCodecFormat(exchange(other.mCodecFormat, GST_FORMAT_INVALID)) , mCodecData(exchange(other.mCodecData, {})) , mIsEncrypted(exchange(other.mIsEncrypted, false)) + , mNaluLengthPrefixed(exchange(other.mNaluLengthPrefixed, false)) , mInfo(exchange(other.mInfo, {})) // POD union - exchange with zero-initialized union { } @@ -160,6 +166,7 @@ struct MediaCodecInfo mCodecFormat = exchange(other.mCodecFormat, GST_FORMAT_INVALID); mCodecData = exchange(other.mCodecData, {}); mIsEncrypted = exchange(other.mIsEncrypted, false); + mNaluLengthPrefixed = exchange(other.mNaluLengthPrefixed, false); mInfo = exchange(other.mInfo, {}); // POD union - exchange with zero-initialized union } return *this; From 660d877a2e92514baa2c142c7bf93d9ab1cac449 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:45:06 +0100 Subject: [PATCH 10/23] Add InvalidateHandle() to decouple CC handle lifetime from refcount PlayerCCManagerBase::GetId()/Release() implements a usage-counter pattern that is correct for PlayerSubtecCCManager's shared, out-of-process connection, but was reused as-is for the Rialto and direct-Rialto backends, where the "handle" is actually a per-pipeline object owned by a single playback session. In multi-pipeline mode this can leave a dangling handle in the singleton after its owning session is destroyed, since the refcount only tracks session count, not the validity of a specific handle. - Add PlayerCCManagerBase::InvalidateHandle(void*), a no-op by default, that a handle owner can call to clear its handle from the singleton ahead of Release(), independent of the GetId()/Release() count. - Add PlayerCCManager::HasInstance() so callers can check for an existing singleton without creating one. - Override InvalidateHandle() in PlayerDirectRialtoCCManager and PlayerRialtoCCManager to clear m_control/mSubtitleControlHandle only when it matches the given handle. This is a minimal mitigation (Option A); wiring InvalidateHandle() into PlayerRialtoCCManager's GStreamer-side handle owner, and a larger structural fix (shared preferences singleton + per-session state), are tracked separately as follow-up work. --- closedcaptions/PlayerCCManager.cpp | 8 +++++ closedcaptions/PlayerCCManager.h | 20 ++++++++++++ .../PlayerDirectRialtoCCManager.cpp | 31 ++++++++++++++----- .../PlayerDirectRialtoCCManager.h | 10 ++++-- .../rialto/PlayerRialtoCCManager.cpp | 31 ++++++++++++++----- closedcaptions/rialto/PlayerRialtoCCManager.h | 8 ++++- .../tests/ClosedCaptionsTests/CMakeLists.txt | 1 + 7 files changed, 91 insertions(+), 18 deletions(-) diff --git a/closedcaptions/PlayerCCManager.cpp b/closedcaptions/PlayerCCManager.cpp index bfd0f689..ef09609c 100644 --- a/closedcaptions/PlayerCCManager.cpp +++ b/closedcaptions/PlayerCCManager.cpp @@ -860,6 +860,14 @@ PlayerCCManagerBase *PlayerCCManager::GetInstance() return mInstance; } +/** + * @brief Check whether the singleton has already been created + */ +bool PlayerCCManager::HasInstance() +{ + return mInstance != NULL; +} + /** * @brief Reset the state. */ diff --git a/closedcaptions/PlayerCCManager.h b/closedcaptions/PlayerCCManager.h index af4683cb..0adfa466 100644 --- a/closedcaptions/PlayerCCManager.h +++ b/closedcaptions/PlayerCCManager.h @@ -71,6 +71,17 @@ class PlayerCCManagerBase */ virtual void Release(int iID) = 0; + /** + * @brief Clear the stored control handle if it currently equals handle. + * Called by the handle owner's destructor so a handle can never be + * used after the object it points to is freed, independent of + * whether the GetId()/Release() usage count has reached zero (it + * may not have, if another session is still registered - see + * multi-pipeline mode). + * @param[in] handle - the handle being invalidated + */ + virtual void InvalidateHandle(void *) {} + /** * @fn SetStatus * @@ -274,6 +285,15 @@ class PlayerCCManager */ static PlayerCCManagerBase * GetInstance(); + /** + * @fn HasInstance + * @brief Check whether GetInstance() has already created the singleton, + * without creating it as a side effect. + * + * @return bool - true if an instance exists + */ + static bool HasInstance(); + /** * @fn SetRialto * @brief Configure which CC manager subclass GetInstance() will create. diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 35e73268..03ade0e9 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -65,10 +65,10 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) MW_LOG_INFO("ENTRY handle=%p", handle); auto *newControl = static_cast(handle); - const bool changedHandle = (newControl != m_control); + const bool changedHandle = (newControl != m_control.load()); m_control = newControl; - if (m_control == nullptr) + if (newControl == nullptr) { MW_LOG_WARN("Initialize called with null handle"); MW_LOG_INFO("EXIT"); @@ -118,6 +118,18 @@ void PlayerDirectRialtoCCManager::Release(int id) } } +void PlayerDirectRialtoCCManager::InvalidateHandle(void *handle) +{ + // m_control is atomic, so this can safely race with Initialize() / + // SetTrack() / StartRendering() / StopRendering() without m_idLock. + auto *expected = static_cast(handle); + if (expected != nullptr && m_control.compare_exchange_strong(expected, nullptr)) + { + MW_LOG_WARN("handle=%p invalidated ahead of Release()", handle); + } +} + + int PlayerDirectRialtoCCManager::SetTrack( const std::string &track, CCFormat format) { @@ -127,7 +139,8 @@ int PlayerDirectRialtoCCManager::SetTrack( MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); - if (m_control == nullptr) + IDirectRialtoCC *control = m_control.load(); + if (control == nullptr) { MW_LOG_INFO("No control handle — track cached"); return 0; @@ -135,31 +148,33 @@ int PlayerDirectRialtoCCManager::SetTrack( const std::string identifier = mapTrackIdentifier(track, format); MW_LOG_INFO("setTextTrackIdentifier=\"%s\"", identifier.c_str()); - m_control->setTextTrackIdentifier(identifier); + control->setTextTrackIdentifier(identifier); return 0; } void PlayerDirectRialtoCCManager::StartRendering() { MW_LOG_INFO("ENTRY — unmuting CC"); - if (m_control == nullptr) + IDirectRialtoCC *control = m_control.load(); + if (control == nullptr) { MW_LOG_WARN("No control handle — cannot unmute"); return; } - m_control->setCCMute(false); + control->setCCMute(false); MW_LOG_INFO("EXIT"); } void PlayerDirectRialtoCCManager::StopRendering() { MW_LOG_INFO("ENTRY — muting CC"); - if (m_control == nullptr) + IDirectRialtoCC *control = m_control.load(); + if (control == nullptr) { MW_LOG_WARN("No control handle — cannot mute"); return; } - m_control->setCCMute(true); + control->setCCMute(true); MW_LOG_INFO("EXIT"); } diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h index 936f1f50..de90a460 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -44,6 +44,7 @@ #include "PlayerCCManager.h" #include "IDirectRialtoCC.h" +#include #include #include @@ -63,6 +64,9 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase /// @copydoc PlayerCCManagerBase::Release void Release(int iID) override; + /// @copydoc PlayerCCManagerBase::InvalidateHandle + void InvalidateHandle(void *handle) override; + /// @copydoc PlayerCCManagerBase::SetTrack int SetTrack(const std::string &track, CCFormat format = eCLOSEDCAPTION_FORMAT_DEFAULT) override; @@ -98,8 +102,10 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase CCFormat format); /// Non-owning pointer to the AampRialtoPlayer CC control interface. - /// Null until Initialize() is called (first PLAYING state). - IDirectRialtoCC *m_control{nullptr}; + /// Null until Initialize() is called (first PLAYING state). Atomic since + /// InvalidateHandle() may run concurrently with the other accessors from + /// the handle owner's destructor. + std::atomic m_control{nullptr}; /// Guards mId / mIdSet. std::mutex m_idLock; diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.cpp b/closedcaptions/rialto/PlayerRialtoCCManager.cpp index db48d6d9..f8814355 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.cpp +++ b/closedcaptions/rialto/PlayerRialtoCCManager.cpp @@ -35,7 +35,7 @@ int PlayerRialtoCCManager::Initialize(void * handle) { MW_LOG_INFO("PlayerRialtoCCManager::Initialize(%p) called", handle); - bool changedHandle = (handle != mSubtitleControlHandle); + bool changedHandle = (handle != mSubtitleControlHandle.load()); mSubtitleControlHandle = handle; @@ -105,6 +105,20 @@ void PlayerRialtoCCManager::Release(int id) return; } +/** + * @brief Clear mSubtitleControlHandle if it currently equals handle + */ +void PlayerRialtoCCManager::InvalidateHandle(void *handle) +{ + // mSubtitleControlHandle is atomic, so this can safely race with + // Initialize() / SetTrack() / StartRendering() / StopRendering(). + void *expected = handle; + if (expected != nullptr && mSubtitleControlHandle.compare_exchange_strong(expected, nullptr)) + { + MW_LOG_WARN("PlayerRialtoCCManager::handle:%p invalidated ahead of Release()", handle); + } +} + /** * @brief Set CC track */ @@ -117,7 +131,8 @@ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat for MW_LOG_INFO("PlayerRialtoCCManager::set track \"%s\"", track.c_str()); - if (nullptr != mSubtitleControlHandle) + void *handle = mSubtitleControlHandle.load(); + if (nullptr != handle) { // We expect 'track' to have an alphabetic prefix. If it does not, // add one based on 'format'. @@ -137,7 +152,7 @@ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat for MW_LOG_INFO("PlayerRialtoCCManager::set track (modified) \"%s\"", textTrackIdentifier.c_str()); - g_object_set(mSubtitleControlHandle, "text-track-identifier", textTrackIdentifier.c_str(), NULL); + g_object_set(handle, "text-track-identifier", textTrackIdentifier.c_str(), NULL); } else { @@ -154,9 +169,10 @@ void PlayerRialtoCCManager::StartRendering() { MW_LOG_INFO("PlayerRialtoCCManager::unmuting"); - if (nullptr != mSubtitleControlHandle) + void *handle = mSubtitleControlHandle.load(); + if (nullptr != handle) { - g_object_set(mSubtitleControlHandle, "mute", FALSE, NULL); + g_object_set(handle, "mute", FALSE, NULL); } else { @@ -172,9 +188,10 @@ void PlayerRialtoCCManager::StopRendering() { MW_LOG_INFO("PlayerRialtoCCManager::muting"); - if (nullptr != mSubtitleControlHandle) + void *handle = mSubtitleControlHandle.load(); + if (nullptr != handle) { - g_object_set(mSubtitleControlHandle, "mute", TRUE, NULL); + g_object_set(handle, "mute", TRUE, NULL); } else { diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.h b/closedcaptions/rialto/PlayerRialtoCCManager.h index bbbd9882..71c8d9d7 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.h +++ b/closedcaptions/rialto/PlayerRialtoCCManager.h @@ -31,6 +31,7 @@ #include #include +#include #include /** @@ -48,6 +49,9 @@ class PlayerRialtoCCManager : public PlayerCCManagerBase */ void Release(int iID) override; + /// @copydoc PlayerCCManagerBase::InvalidateHandle + void InvalidateHandle(void *handle) override; + /** * @fn GetId * @return int - unique ID @@ -119,7 +123,9 @@ class PlayerRialtoCCManager : public PlayerCCManagerBase void ResetState() override; private: - void *mSubtitleControlHandle{nullptr}; + /// GstElement* decoder handle. Atomic since InvalidateHandle() may run + /// concurrently with the other accessors from the handle owner's destructor. + std::atomic mSubtitleControlHandle{nullptr}; std::mutex mIdLock{}; int mId{0}; diff --git a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt index 296294f6..e7b23601 100644 --- a/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt +++ b/test/utests/tests/ClosedCaptionsTests/CMakeLists.txt @@ -30,6 +30,7 @@ include_directories(${PLAYER_ROOT}/playerJsonObject) include_directories(${PLAYER_ROOT}/playerLogManager) include_directories(${PLAYER_ROOT}/mp4demux) include_directories(${PLAYER_ROOT}/closedcaptions/rialto) +include_directories(${PLAYER_ROOT}/closedcaptions/direct-rialto) include_directories(${GTEST_INCLUDE_DIRS}) include_directories(${GMOCK_INCLUDE_DIRS}) include_directories(${GLIB_INCLUDE_DIRS}) From 086994fa1916ae36323fb13eb71dc62f964bee6d Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Mon, 21 Sep 2026 15:47:38 +0000 Subject: [PATCH 11/23] VPLAY-12250: Fix use-after-free race in CC manager handle invalidation InvalidateHandle() relied on an atomic compare_exchange to clear the control/handle pointer, but the atomic only protects the pointer value, not the lifetime of the object it points to. A concurrent SetTrack() / StartRendering() / StopRendering() call could load the pointer before the CAS and dereference it after the owner destroyed the object. - PlayerDirectRialtoCCManager: replace std::atomic with a mutex (m_controlMutex) shared by InvalidateHandle() and all accessors, so invalidation blocks until any in-flight call on the old handle completes. - PlayerRialtoCCManager: apply the same mutex-guard fix for mSubtitleControlHandle. - PlayerDirectRialtoCCManager::SetTrack(): return the actual result of setTextTrackIdentifier() (0/-1) instead of always returning 0, per the base class contract. - Add SetTrack_ControlRejectsIdentifier_ReturnsFailure L1 test covering the failure path. Testing: PlayerDirectRialtoCCManagerTests (10/10) pass. --- .../PlayerDirectRialtoCCManager.cpp | 40 +++++++++++-------- .../PlayerDirectRialtoCCManager.h | 15 ++++--- .../rialto/PlayerRialtoCCManager.cpp | 39 ++++++++++-------- closedcaptions/rialto/PlayerRialtoCCManager.h | 13 ++++-- .../PlayerDirectRialtoCCManagerTestCases.cpp | 18 +++++++++ 5 files changed, 84 insertions(+), 41 deletions(-) diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 03ade0e9..27d564eb 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -65,8 +65,12 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) MW_LOG_INFO("ENTRY handle=%p", handle); auto *newControl = static_cast(handle); - const bool changedHandle = (newControl != m_control.load()); - m_control = newControl; + bool changedHandle; + { + std::lock_guard lock(m_controlMutex); + changedHandle = (newControl != m_control); + m_control = newControl; + } if (newControl == nullptr) { @@ -120,11 +124,14 @@ void PlayerDirectRialtoCCManager::Release(int id) void PlayerDirectRialtoCCManager::InvalidateHandle(void *handle) { - // m_control is atomic, so this can safely race with Initialize() / - // SetTrack() / StartRendering() / StopRendering() without m_idLock. - auto *expected = static_cast(handle); - if (expected != nullptr && m_control.compare_exchange_strong(expected, nullptr)) + // Holding m_controlMutex blocks until any SetTrack() / StartRendering() / + // StopRendering() call already in progress on the old handle has + // returned, so the caller can safely destroy the handle owner once this + // call returns. + std::lock_guard lock(m_controlMutex); + if (handle != nullptr && m_control == handle) { + m_control = nullptr; MW_LOG_WARN("handle=%p invalidated ahead of Release()", handle); } } @@ -139,8 +146,8 @@ int PlayerDirectRialtoCCManager::SetTrack( MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); - IDirectRialtoCC *control = m_control.load(); - if (control == nullptr) + std::lock_guard lock(m_controlMutex); + if (m_control == nullptr) { MW_LOG_INFO("No control handle — track cached"); return 0; @@ -148,33 +155,33 @@ int PlayerDirectRialtoCCManager::SetTrack( const std::string identifier = mapTrackIdentifier(track, format); MW_LOG_INFO("setTextTrackIdentifier=\"%s\"", identifier.c_str()); - control->setTextTrackIdentifier(identifier); - return 0; + const bool ok = m_control->setTextTrackIdentifier(identifier); + return ok ? 0 : -1; } void PlayerDirectRialtoCCManager::StartRendering() { MW_LOG_INFO("ENTRY — unmuting CC"); - IDirectRialtoCC *control = m_control.load(); - if (control == nullptr) + std::lock_guard lock(m_controlMutex); + if (m_control == nullptr) { MW_LOG_WARN("No control handle — cannot unmute"); return; } - control->setCCMute(false); + m_control->setCCMute(false); MW_LOG_INFO("EXIT"); } void PlayerDirectRialtoCCManager::StopRendering() { MW_LOG_INFO("ENTRY — muting CC"); - IDirectRialtoCC *control = m_control.load(); - if (control == nullptr) + std::lock_guard lock(m_controlMutex); + if (m_control == nullptr) { MW_LOG_WARN("No control handle — cannot mute"); return; } - control->setCCMute(true); + m_control->setCCMute(true); MW_LOG_INFO("EXIT"); } @@ -182,6 +189,7 @@ void PlayerDirectRialtoCCManager::ResetState() { MW_LOG_INFO("ENTRY"); PlayerCCManagerBase::ResetState(); + std::lock_guard lock(m_controlMutex); m_control = nullptr; MW_LOG_INFO("EXIT"); } diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h index de90a460..a5be1a12 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -44,7 +44,6 @@ #include "PlayerCCManager.h" #include "IDirectRialtoCC.h" -#include #include #include @@ -101,11 +100,17 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase static std::string mapTrackIdentifier(const std::string &track, CCFormat format); + /// Guards m_control. InvalidateHandle() also takes this lock, so any + /// SetTrack() / StartRendering() / StopRendering() call already in + /// progress on the old handle completes before InvalidateHandle() clears + /// the pointer and returns - preventing use-after-free when the handle + /// owner is destroyed concurrently. + mutable std::mutex m_controlMutex; + /// Non-owning pointer to the AampRialtoPlayer CC control interface. - /// Null until Initialize() is called (first PLAYING state). Atomic since - /// InvalidateHandle() may run concurrently with the other accessors from - /// the handle owner's destructor. - std::atomic m_control{nullptr}; + /// Null until Initialize() is called (first PLAYING state). Guarded by + /// m_controlMutex. + IDirectRialtoCC *m_control{nullptr}; /// Guards mId / mIdSet. std::mutex m_idLock; diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.cpp b/closedcaptions/rialto/PlayerRialtoCCManager.cpp index f8814355..5b60d72f 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.cpp +++ b/closedcaptions/rialto/PlayerRialtoCCManager.cpp @@ -35,9 +35,12 @@ int PlayerRialtoCCManager::Initialize(void * handle) { MW_LOG_INFO("PlayerRialtoCCManager::Initialize(%p) called", handle); - bool changedHandle = (handle != mSubtitleControlHandle.load()); - - mSubtitleControlHandle = handle; + bool changedHandle; + { + std::lock_guard lock(mControlMutex); + changedHandle = (handle != mSubtitleControlHandle); + mSubtitleControlHandle = handle; + } if (GetTrack().empty()) { @@ -74,6 +77,7 @@ void PlayerRialtoCCManager::ResetState() { MW_LOG_INFO("PlayerRialtoCCManager::Resetting"); PlayerCCManagerBase::ResetState(); + std::lock_guard lock(mControlMutex); mSubtitleControlHandle = nullptr; } @@ -110,11 +114,14 @@ void PlayerRialtoCCManager::Release(int id) */ void PlayerRialtoCCManager::InvalidateHandle(void *handle) { - // mSubtitleControlHandle is atomic, so this can safely race with - // Initialize() / SetTrack() / StartRendering() / StopRendering(). - void *expected = handle; - if (expected != nullptr && mSubtitleControlHandle.compare_exchange_strong(expected, nullptr)) + // Holding mControlMutex blocks until any SetTrack() / StartRendering() / + // StopRendering() call already in progress on the old handle has + // returned, so the caller can safely destroy the handle owner once this + // call returns. + std::lock_guard lock(mControlMutex); + if (handle != nullptr && mSubtitleControlHandle == handle) { + mSubtitleControlHandle = nullptr; MW_LOG_WARN("PlayerRialtoCCManager::handle:%p invalidated ahead of Release()", handle); } } @@ -131,8 +138,8 @@ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat for MW_LOG_INFO("PlayerRialtoCCManager::set track \"%s\"", track.c_str()); - void *handle = mSubtitleControlHandle.load(); - if (nullptr != handle) + std::lock_guard lock(mControlMutex); + if (nullptr != mSubtitleControlHandle) { // We expect 'track' to have an alphabetic prefix. If it does not, // add one based on 'format'. @@ -152,7 +159,7 @@ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat for MW_LOG_INFO("PlayerRialtoCCManager::set track (modified) \"%s\"", textTrackIdentifier.c_str()); - g_object_set(handle, "text-track-identifier", textTrackIdentifier.c_str(), NULL); + g_object_set(mSubtitleControlHandle, "text-track-identifier", textTrackIdentifier.c_str(), NULL); } else { @@ -169,10 +176,10 @@ void PlayerRialtoCCManager::StartRendering() { MW_LOG_INFO("PlayerRialtoCCManager::unmuting"); - void *handle = mSubtitleControlHandle.load(); - if (nullptr != handle) + std::lock_guard lock(mControlMutex); + if (nullptr != mSubtitleControlHandle) { - g_object_set(handle, "mute", FALSE, NULL); + g_object_set(mSubtitleControlHandle, "mute", FALSE, NULL); } else { @@ -188,10 +195,10 @@ void PlayerRialtoCCManager::StopRendering() { MW_LOG_INFO("PlayerRialtoCCManager::muting"); - void *handle = mSubtitleControlHandle.load(); - if (nullptr != handle) + std::lock_guard lock(mControlMutex); + if (nullptr != mSubtitleControlHandle) { - g_object_set(handle, "mute", TRUE, NULL); + g_object_set(mSubtitleControlHandle, "mute", TRUE, NULL); } else { diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.h b/closedcaptions/rialto/PlayerRialtoCCManager.h index 71c8d9d7..9acb6b80 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.h +++ b/closedcaptions/rialto/PlayerRialtoCCManager.h @@ -31,7 +31,6 @@ #include #include -#include #include /** @@ -123,9 +122,15 @@ class PlayerRialtoCCManager : public PlayerCCManagerBase void ResetState() override; private: - /// GstElement* decoder handle. Atomic since InvalidateHandle() may run - /// concurrently with the other accessors from the handle owner's destructor. - std::atomic mSubtitleControlHandle{nullptr}; + /// Guards mSubtitleControlHandle. InvalidateHandle() also takes this + /// lock, so any SetTrack() / StartRendering() / StopRendering() call + /// already in progress on the old handle completes before + /// InvalidateHandle() clears the pointer and returns - preventing + /// use-after-free when the handle owner is destroyed concurrently. + mutable std::mutex mControlMutex; + + /// GstElement* decoder handle. Guarded by mControlMutex. + void *mSubtitleControlHandle{nullptr}; std::mutex mIdLock{}; int mId{0}; diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp index 9a87fbf4..662315dc 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -29,6 +29,7 @@ #include #include +#include #include #include "PlayerDirectRialtoCCManager.h" @@ -121,6 +122,23 @@ TEST_F(PlayerDirectRialtoCCManagerTest, m_mgr.Initialize(m_mock.get()); } +/** + * @test SetTrack_ControlRejectsIdentifier_ReturnsFailure + * @brief When setTextTrackIdentifier() reports failure, SetTrack() must + * return -1, per the PlayerCCManagerBase::SetTrack() 0/-1 contract. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + SetTrack_ControlRejectsIdentifier_ReturnsFailure) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC2")) + .Times(1) + .WillOnce(Return(false)); + + EXPECT_EQ(m_mgr.SetTrack("2", eCLOSEDCAPTION_FORMAT_608), -1); +} + /** * @test SetTrack_NumericWith608Format_PrependsCCPrefix * @brief A numeric track string with 608 format should become "CC". From f10d56535aa778464050a7fe5295c40c97265a2a Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:18:42 +0000 Subject: [PATCH 12/23] VPLAY-12250: Propagate setKeyId() and SetTrack() failures for non-OpenCDM sessions DrmSessionManager::getDrmSession() called setKeyId() only inside already made unconditional. Non-OpenCDM backends (e.g. Direct-Rialto) never received the key ID. Move the call inside the existing drmSession != NULL check and drop the guard; setKeyId() defaults to a no-op, so calling it unconditionally is safe for every backend. Remove the matching stale #ifdef around the setKeyId mock in MockDrmSession.h so it stays in sync with the base class. PlayerDirectRialtoCCManager::Initialize() discarded the return value of SetTrack() for both the default "CC1" track and the cached-track re-apply path, always returning 0 even when the control rejected the identifier. Propagate the failure as -1 in both cases so PlayerCCManagerBase::Init() correctly reports failure. Add Initialize_ControlRejectsDefaultTrack_ReturnsFailure and Initialize_ControlRejectsCachedTrack_ReturnsFailure to cover the new failure paths. --- .../PlayerDirectRialtoCCManager.cpp | 12 +++++-- drm/DrmSessionManager.cpp | 7 ++-- test/utests/mocks/MockDrmSession.h | 2 -- .../PlayerDirectRialtoCCManagerTestCases.cpp | 34 +++++++++++++++++++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 27d564eb..77f6041c 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -84,12 +84,20 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) // Apps expect CC1 as the default; apply it so the first frame // renders without an explicit SetTextTrack() call. MW_LOG_INFO("Setting default track CC1"); - (void) SetTrack("CC1"); + if (SetTrack("CC1") != 0) + { + MW_LOG_WARN("Failed to set default track CC1"); + return -1; + } } else if (changedHandle) { // Re-apply the cached track on the new handle (e.g. re-tune). - (void) SetTrack(GetTrack(), mTrackFormat); + if (SetTrack(GetTrack(), mTrackFormat) != 0) + { + MW_LOG_WARN("Failed to reapply cached track on new handle"); + return -1; + } } MW_LOG_INFO("EXIT"); diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 383e523a..7661ad81 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -933,6 +933,9 @@ KeyState DrmSessionManager::getDrmSession(int &err, std::shared_ptr d drmSessionContexts[sessionSlot].drmSession->setOutputProtection(true); drmHelper->setOutputProtectionFlag(true); } + // setKeyId() is a no-op default on DrmSession, so it's safe to call + // for every backend (OpenCDM, Direct-Rialto, etc.), not just OpenCDM. + drmSessionContexts[sessionSlot].drmSession->setKeyId(keyIdArray); } else { @@ -940,10 +943,6 @@ KeyState DrmSessionManager::getDrmSession(int &err, std::shared_ptr d err = MW_DRM_INIT_FAILED ; } -#if defined(USE_OPENCDM_ADAPTER) - drmSessionContexts[sessionSlot].drmSession->setKeyId(keyIdArray); -#endif - return code; } diff --git a/test/utests/mocks/MockDrmSession.h b/test/utests/mocks/MockDrmSession.h index f10c7e4c..fb384c50 100644 --- a/test/utests/mocks/MockDrmSession.h +++ b/test/utests/mocks/MockDrmSession.h @@ -43,9 +43,7 @@ class MockDrmSession : public DrmSession MOCK_METHOD(int, decrypt, (const uint8_t *f_pbIV, uint32_t f_cbIV, const uint8_t *payloadData, uint32_t payloadDataSize, uint8_t **ppOpaqueData), (override)); MOCK_METHOD(KeyState, getState, (), (override)); MOCK_METHOD(void, clearDecryptContext, (), (override)); -#if defined(USE_OPENCDM_ADAPTER) MOCK_METHOD(void, setKeyId, (const std::vector& keyId), (override)); -#endif }; extern MockDrmSession *g_mockDrmSession; diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp index 662315dc..af7f8414 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -105,6 +105,23 @@ TEST_F(PlayerDirectRialtoCCManagerTest, m_mgr.Initialize(m_mock.get()); } +/** + * @test Initialize_ControlRejectsDefaultTrack_ReturnsFailure + * @brief When the control rejects the default "CC1" identifier, + * Initialize() must propagate the failure so + * PlayerCCManagerBase::Init() reports it, instead of silently + * discarding the SetTrack() result and returning 0. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_ControlRejectsDefaultTrack_ReturnsFailure) +{ + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .Times(1) + .WillOnce(Return(false)); + + EXPECT_EQ(m_mgr.Initialize(m_mock.get()), -1); +} + /** * @test Initialize_WithCachedTrack_ReappliesCachedTrack * @brief When a track has already been cached via SetTrack(), Initialize() @@ -122,6 +139,23 @@ TEST_F(PlayerDirectRialtoCCManagerTest, m_mgr.Initialize(m_mock.get()); } +/** + * @test Initialize_ControlRejectsCachedTrack_ReturnsFailure + * @brief When the control rejects the cached identifier on re-apply, + * Initialize() must propagate the failure instead of returning 0. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_ControlRejectsCachedTrack_ReturnsFailure) +{ + m_mgr.SetTrack("CC3"); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC3")) + .Times(1) + .WillOnce(Return(false)); + + EXPECT_EQ(m_mgr.Initialize(m_mock.get()), -1); +} + /** * @test SetTrack_ControlRejectsIdentifier_ReturnsFailure * @brief When setTextTrackIdentifier() reports failure, SetTrack() must From 492abad8014e8d52cbe41634b3cf0152f4406d42 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 09:55:08 +0000 Subject: [PATCH 13/23] VPLAY-12250: Fix ABI breaks and CC manager retry/cache race drm/DrmSessionManager.h and closedcaptions/PlayerCCManager.h added a default argument to an existing constructor/static method (DrmSessionManager's 4th param, SetRialto's 2nd param). Since both are installed public headers, this silently changed the mangled symbol, breaking binaries linked against the previous release. Restore the old symbols by keeping a distinct overload that delegates to the new one: - DrmSessionManager(int, void*, callback) delegates to DrmSessionManager(int, void*, callback, creator). - PlayerCCManager::SetRialto(bool) delegates to SetRialto(bool, bool). PlayerDirectRialtoCCManager::Initialize() left m_control pointing at the new handle even when SetTrack() failed to apply the track. A later Initialize() retry with the same handle then saw changedHandle == false and returned success without ever configuring the control. Clear the control via a new clearControlOnFailure() helper whenever SetTrack() fails during Initialize(), so a retry is treated as a handle change and reattempts SetTrack(). mTrack/mTrackFormat were written in SetTrack() outside m_controlMutex and read/cleared in Initialize()/ResetState() without any lock. Move all three accesses under m_controlMutex; Initialize() takes a local snapshot before releasing the lock to avoid recursive-lock deadlock when it subsequently calls SetTrack(). Add Initialize_RetryAfterRejectedDefaultTrack_ReappliesTrack to cover the retry fix. --- closedcaptions/PlayerCCManager.cpp | 8 ++++ closedcaptions/PlayerCCManager.h | 11 +++++- .../PlayerDirectRialtoCCManager.cpp | 37 +++++++++++++++---- .../PlayerDirectRialtoCCManager.h | 16 +++++--- drm/DrmSessionManager.cpp | 8 ++++ drm/DrmSessionManager.h | 10 ++++- .../PlayerDirectRialtoCCManagerTestCases.cpp | 23 ++++++++++++ 7 files changed, 98 insertions(+), 15 deletions(-) diff --git a/closedcaptions/PlayerCCManager.cpp b/closedcaptions/PlayerCCManager.cpp index ef09609c..9af0dd58 100644 --- a/closedcaptions/PlayerCCManager.cpp +++ b/closedcaptions/PlayerCCManager.cpp @@ -913,6 +913,14 @@ void PlayerCCManager::SetRialto(bool bIsRialto, bool bIsDirectRialto) } } +/** + * @brief Set the CC manager variant (no direct-Rialto option). + */ +void PlayerCCManager::SetRialto(bool bIsRialto) +{ + SetRialto(bIsRialto, false); +} + /** * @brief Destroy instance */ diff --git a/closedcaptions/PlayerCCManager.h b/closedcaptions/PlayerCCManager.h index 0adfa466..f38c473f 100644 --- a/closedcaptions/PlayerCCManager.h +++ b/closedcaptions/PlayerCCManager.h @@ -304,7 +304,16 @@ class PlayerCCManager * (PlayerDirectRialtoCCManager). * @return void */ - static void SetRialto(bool bIsRialto, bool bIsDirectRialto = false); + static void SetRialto(bool bIsRialto, bool bIsDirectRialto); + + /** + * @fn SetRialto + * @brief Preserves the pre-existing one-argument mangled symbol for + * binaries linked against older releases of this installed + * header; delegates to the two-argument overload. + * @return void + */ + static void SetRialto(bool bIsRialto); /** * @fn DestroyInstance diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 77f6041c..aa6240bc 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -66,10 +66,14 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) auto *newControl = static_cast(handle); bool changedHandle; + std::string cachedTrack; + CCFormat cachedFormat; { std::lock_guard lock(m_controlMutex); changedHandle = (newControl != m_control); - m_control = newControl; + m_control = newControl; + cachedTrack = mTrack; + cachedFormat = mTrackFormat; } if (newControl == nullptr) @@ -79,7 +83,7 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) return 0; } - if (GetTrack().empty()) + if (cachedTrack.empty()) { // Apps expect CC1 as the default; apply it so the first frame // renders without an explicit SetTextTrack() call. @@ -87,15 +91,17 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) if (SetTrack("CC1") != 0) { MW_LOG_WARN("Failed to set default track CC1"); + clearControlOnFailure(newControl); return -1; } } else if (changedHandle) { // Re-apply the cached track on the new handle (e.g. re-tune). - if (SetTrack(GetTrack(), mTrackFormat) != 0) + if (SetTrack(cachedTrack, cachedFormat) != 0) { MW_LOG_WARN("Failed to reapply cached track on new handle"); + clearControlOnFailure(newControl); return -1; } } @@ -148,13 +154,14 @@ void PlayerDirectRialtoCCManager::InvalidateHandle(void *handle) int PlayerDirectRialtoCCManager::SetTrack( const std::string &track, CCFormat format) { - // Cache for re-application after Initialize(). - mTrack = track; - mTrackFormat = format; - MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); + // mTrack/mTrackFormat are shared with Initialize() and ResetState(), so + // they must be updated under the same lock as m_control. std::lock_guard lock(m_controlMutex); + mTrack = track; + mTrackFormat = format; + if (m_control == nullptr) { MW_LOG_INFO("No control handle — track cached"); @@ -196,8 +203,22 @@ void PlayerDirectRialtoCCManager::StopRendering() void PlayerDirectRialtoCCManager::ResetState() { MW_LOG_INFO("ENTRY"); - PlayerCCManagerBase::ResetState(); + // Base ResetState() clears mTrack/mTrackFormat, so it must run under the + // same lock that guards them elsewhere in this class. std::lock_guard lock(m_controlMutex); + PlayerCCManagerBase::ResetState(); m_control = nullptr; MW_LOG_INFO("EXIT"); } + +void PlayerDirectRialtoCCManager::clearControlOnFailure(IDirectRialtoCC *handle) +{ + // A failed track application leaves the control unconfigured; clear it + // so a later Initialize() call with the same handle is treated as a + // handle change and retries SetTrack() instead of reporting success. + std::lock_guard lock(m_controlMutex); + if (m_control == handle) + { + m_control = nullptr; + } +} diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h index a5be1a12..a57b61da 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -100,11 +100,17 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase static std::string mapTrackIdentifier(const std::string &track, CCFormat format); - /// Guards m_control. InvalidateHandle() also takes this lock, so any - /// SetTrack() / StartRendering() / StopRendering() call already in - /// progress on the old handle completes before InvalidateHandle() clears - /// the pointer and returns - preventing use-after-free when the handle - /// owner is destroyed concurrently. + /// Clear m_control if it still equals handle, forcing a later + /// Initialize() call with the same handle to retry SetTrack() instead + /// of treating it as an unchanged, already-configured handle. + void clearControlOnFailure(IDirectRialtoCC *handle); + + /// Guards m_control and the inherited mTrack / mTrackFormat cache. + /// InvalidateHandle() also takes this lock, so any SetTrack() / + /// StartRendering() / StopRendering() call already in progress on the + /// old handle completes before InvalidateHandle() clears the pointer + /// and returns - preventing use-after-free when the handle owner is + /// destroyed concurrently. mutable std::mutex m_controlMutex; /// Non-owning pointer to the AampRialtoPlayer CC control interface. diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 7661ad81..a676136f 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -73,6 +73,14 @@ DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::func MW_LOG_INFO("DrmSessionManager MaxSession:%d",mMaxDRMSessions); } +/** + * @brief DrmSessionManager constructor (no injected creator). + */ +DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback) + : DrmSessionManager(maxDrmSessions, player, std::move(watermarkSessionUpdateCallback), nullptr) +{ +} + /** * @brief DrmSessionManager Destructor. */ diff --git a/drm/DrmSessionManager.h b/drm/DrmSessionManager.h index 7b4dd58c..8e6b1539 100644 --- a/drm/DrmSessionManager.h +++ b/drm/DrmSessionManager.h @@ -233,7 +233,15 @@ class DrmSessionManager /** * @fn DrmSessionManager */ - DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator = nullptr); + DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator); + + /** + * @fn DrmSessionManager + * @brief Preserves the pre-existing three-argument mangled symbol for + * binaries linked against older releases of this installed + * header; delegates to the creator-injecting constructor. + */ + DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback); /** * @fn watermarkSessionHandlerWrapper * @brief Wrapper function to handle session watermark. diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp index af7f8414..58d3464b 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -156,6 +156,29 @@ TEST_F(PlayerDirectRialtoCCManagerTest, EXPECT_EQ(m_mgr.Initialize(m_mock.get()), -1); } +/** + * @test Initialize_RetryAfterRejectedDefaultTrack_ReappliesTrack + * @brief After a failed default-track application, Initialize() must clear + * the unconfigured control so a retry with the same handle is + * treated as a handle change and re-attempts SetTrack(), instead of + * seeing an unchanged handle and returning success without + * configuring the control. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + Initialize_RetryAfterRejectedDefaultTrack_ReappliesTrack) +{ + { + ::testing::InSequence seq; + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .WillOnce(Return(false)); + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC1")) + .WillOnce(Return(true)); + } + + EXPECT_EQ(m_mgr.Initialize(m_mock.get()), -1); + EXPECT_EQ(m_mgr.Initialize(m_mock.get()), 0); +} + /** * @test SetTrack_ControlRejectsIdentifier_ReturnsFailure * @brief When setTextTrackIdentifier() reports failure, SetTrack() must From 17ad4bb7638b032ec4d7d612b896d314f8af9aa2 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 10:40:49 +0000 Subject: [PATCH 14/23] Fix ResetState deadlock and drop unsafe DrmSessionManager overload PlayerDirectRialtoCCManager: - Fix deadlock in ResetState(): PlayerCCManagerBase::ResetState() calls Stop() -> StopRendering(), which re-enters m_controlMutex. Run the base call before acquiring the lock instead of wrapping the whole function, matching the original (pre-mutex) call order. - Update m_controlMutex's doc comment to reflect that ResetState() clears mTrack/mTrackFormat via the base class before taking the lock. - Strengthen the test double: PlayerCCManagerBase::ResetState() in PlayerDirectRialtoCCManagerBaseStubs.cpp now calls Stop(), mirroring the real base class, so ResetState_ClearsControlHandle actually exercises the mutex re-entrancy this fix avoids instead of masking it. DrmSessionManager: - Revert the 3-arg constructor overload added to preserve the old mangled symbol. m_sessionCreator already changed this class's size, so a prebuilt binary compiled before that member existed would still link successfully against the restored 3-arg symbol, then heap- corrupt: operator new sizes the allocation from the caller's old (smaller) layout, while the constructor initializes an object sized for the new (larger) one. Collapsing back to a single constructor with creator defaulted to nullptr forces such callers to fail at compile time instead of silently corrupting memory at runtime. Verified: PlayerDirectRialtoCCManagerTests (13/13), DrmSessionManagerTests (29/29), DrmAes (33/34, 1 pre-existing skip), DrmHelperTests (160/160), DrmOcdmTests (96/96). --- .../direct-rialto/PlayerDirectRialtoCCManager.cpp | 7 ++++--- .../direct-rialto/PlayerDirectRialtoCCManager.h | 14 ++++++++------ drm/DrmSessionManager.cpp | 8 -------- drm/DrmSessionManager.h | 15 ++++++--------- .../PlayerDirectRialtoCCManagerBaseStubs.cpp | 10 ++++++++++ .../PlayerDirectRialtoCCManagerTestCases.cpp | 8 +++++++- 6 files changed, 35 insertions(+), 27 deletions(-) diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index aa6240bc..2cf5c5da 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -203,10 +203,11 @@ void PlayerDirectRialtoCCManager::StopRendering() void PlayerDirectRialtoCCManager::ResetState() { MW_LOG_INFO("ENTRY"); - // Base ResetState() clears mTrack/mTrackFormat, so it must run under the - // same lock that guards them elsewhere in this class. - std::lock_guard lock(m_controlMutex); + // PlayerCCManagerBase::ResetState() calls Stop() -> StopRendering(), + // which re-enters m_controlMutex; run it before taking the lock here to + // avoid deadlocking on this non-recursive mutex. PlayerCCManagerBase::ResetState(); + std::lock_guard lock(m_controlMutex); m_control = nullptr; MW_LOG_INFO("EXIT"); } diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h index a57b61da..0b272925 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -105,12 +105,14 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase /// of treating it as an unchanged, already-configured handle. void clearControlOnFailure(IDirectRialtoCC *handle); - /// Guards m_control and the inherited mTrack / mTrackFormat cache. - /// InvalidateHandle() also takes this lock, so any SetTrack() / - /// StartRendering() / StopRendering() call already in progress on the - /// old handle completes before InvalidateHandle() clears the pointer - /// and returns - preventing use-after-free when the handle owner is - /// destroyed concurrently. + /// Guards m_control and SetTrack()'s writes / Initialize()'s reads of + /// the inherited mTrack / mTrackFormat cache. InvalidateHandle() also + /// takes this lock, so any SetTrack() / StartRendering() / + /// StopRendering() call already in progress on the old handle completes + /// before InvalidateHandle() clears the pointer and returns - + /// preventing use-after-free when the handle owner is destroyed + /// concurrently. ResetState() clears mTrack / mTrackFormat via the base + /// class before taking this lock (see ResetState() for why). mutable std::mutex m_controlMutex; /// Non-owning pointer to the AampRialtoPlayer CC control interface. diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index a676136f..7661ad81 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -73,14 +73,6 @@ DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::func MW_LOG_INFO("DrmSessionManager MaxSession:%d",mMaxDRMSessions); } -/** - * @brief DrmSessionManager constructor (no injected creator). - */ -DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback) - : DrmSessionManager(maxDrmSessions, player, std::move(watermarkSessionUpdateCallback), nullptr) -{ -} - /** * @brief DrmSessionManager Destructor. */ diff --git a/drm/DrmSessionManager.h b/drm/DrmSessionManager.h index 8e6b1539..7ff90da7 100644 --- a/drm/DrmSessionManager.h +++ b/drm/DrmSessionManager.h @@ -232,16 +232,13 @@ class DrmSessionManager /** * @fn DrmSessionManager + * @brief creator defaults to null (no injection) rather than being a + * separate overload: a distinct 3-arg symbol would let a + * prebuilt binary compiled before m_sessionCreator existed link + * successfully against this header's larger layout and silently + * heap-corrupt on construction, instead of failing to compile. */ - DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator); - - /** - * @fn DrmSessionManager - * @brief Preserves the pre-existing three-argument mangled symbol for - * binaries linked against older releases of this installed - * header; delegates to the creator-injecting constructor. - */ - DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback); + DrmSessionManager(int maxDrmSessions, void *player, std::function watermarkSessionUpdateCallback, DrmSessionCreator creator = nullptr); /** * @fn watermarkSessionHandlerWrapper * @brief Wrapper function to handle session watermark. diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp index 1b30baeb..7934db4a 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp @@ -68,8 +68,18 @@ void PlayerCCManagerBase::SetParentalControlStatus(bool locked) mParentalCtrlLocked = locked; } +void PlayerCCManagerBase::Stop() +{ + StopRendering(); +} + void PlayerCCManagerBase::ResetState() { + // Mirrors the real PlayerCCManagerBase::ResetState(), which calls Stop() + // (-> StopRendering()) before clearing state; this exercises the same + // mutex re-entrancy that PlayerDirectRialtoCCManager::ResetState() must + // avoid deadlocking on. + Stop(); mOptions.clear(); mTrack.clear(); mTrackFormat = eCLOSEDCAPTION_FORMAT_DEFAULT; diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp index 58d3464b..7c4c8c3f 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -279,13 +279,19 @@ TEST_F(PlayerDirectRialtoCCManagerTest, /** * @test ResetState_ClearsControlHandle - * @brief After ResetState(), StartRendering() must not call the old mock. + * @brief ResetState() must call Stop() (-> StopRendering()) on the still- + * valid control without deadlocking on m_controlMutex, then clear + * the control so a subsequent StartRendering() is a no-op. */ TEST_F(PlayerDirectRialtoCCManagerTest, ResetState_ClearsControlHandle) { InitWithDefaultTrack(); + EXPECT_CALL(*m_mock, setCCMute(true)) + .Times(1) + .WillOnce(Return(true)); + m_mgr.ResetState(); EXPECT_NO_FATAL_FAILURE(m_mgr.StartRendering()); From 2d60866bbc3e4cda263b4cea8674e25fa6f6e1dc Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 11:28:40 +0000 Subject: [PATCH 15/23] Fix CC manager vtable/cache/race bugs; add DRM creator test - PlayerCCManager.h: move InvalidateHandle() to the end of PlayerCCManagerBase's virtual interface. It was previously declared between Release() and SetStatus(), which shifts every subsequent vtable slot and silently mis-dispatches calls from binaries built against an older header. - PlayerCCManager.h/.cpp: split ResetState() into Stop() + a new ResetTrackState() helper that only resets the cached track/rendering fields, without acquiring any lock itself. PlayerDirectRialtoCCManager and PlayerRialtoCCManager's ResetState() now call Stop() unlocked (still required to avoid deadlocking through StopRendering()'s re-entrant lock), then call ResetTrackState() and clear their handle while holding their own control mutex, instead of invoking PlayerCCManagerBase::ResetState() directly - removing a race between ResetState() and concurrent SetTrack()/Initialize() calls on those fields. Updated PlayerDirectRialtoCCManagerBaseStubs.cpp with a matching ResetTrackState() stub. - PlayerRialtoCCManager.cpp/.h: move mTrack/mTrackFormat writes inside mControlMutex in SetTrack(), and snapshot them under the same lock as mSubtitleControlHandle in Initialize(), removing a data race with concurrent SetTrack()/Initialize() calls. Mirrors the fix already applied to PlayerDirectRialtoCCManager. Updated the mControlMutex doc comment to reflect the additional state it now guards. - PlayerDirectRialtoCCManager.cpp: only update the cached mTrack/ mTrackFormat once setTextTrackIdentifier() confirms success. A rejected track previously still updated the cache, so a later Initialize() with the same handle would see changedHandle == false and skip reapplying the last-accepted track. Added SetTrack_ControlRejectsIdentifier_CacheKeepsPreviousTrack to cover it. - DrmSessionManagerTestCases.cpp: add coverage for the DrmSessionCreator injection path in getDrmSession(), which previously had no test exercising it. Extend TestableDrmSessionManager with a constructor overload forwarding a DrmSessionCreator, and add tests verifying (a) the creator is invoked and its returned session's ownership is transferred into the selected slot, and (b) a null creator result is reported as MW_DRM_INIT_FAILED. --- closedcaptions/PlayerCCManager.cpp | 7 ++ closedcaptions/PlayerCCManager.h | 39 ++++++--- .../PlayerDirectRialtoCCManager.cpp | 26 ++++-- .../rialto/PlayerRialtoCCManager.cpp | 26 ++++-- closedcaptions/rialto/PlayerRialtoCCManager.h | 9 ++- .../DrmSessionManagerTestCases.cpp | 80 +++++++++++++++++++ .../PlayerDirectRialtoCCManagerBaseStubs.cpp | 5 ++ .../PlayerDirectRialtoCCManagerTestCases.cpp | 20 +++++ 8 files changed, 183 insertions(+), 29 deletions(-) diff --git a/closedcaptions/PlayerCCManager.cpp b/closedcaptions/PlayerCCManager.cpp index 9af0dd58..09fc7eaf 100644 --- a/closedcaptions/PlayerCCManager.cpp +++ b/closedcaptions/PlayerCCManager.cpp @@ -875,7 +875,14 @@ void PlayerCCManagerBase::ResetState() { MW_LOG_INFO("PlayerCCManagerBase::Resetting"); Stop(); + ResetTrackState(); +} +/** + * @brief Reset the cached track/rendering fields without stopping. + */ +void PlayerCCManagerBase::ResetTrackState() +{ mOptions = ""; mTrack = ""; mTrackFormat = eCLOSEDCAPTION_FORMAT_DEFAULT; diff --git a/closedcaptions/PlayerCCManager.h b/closedcaptions/PlayerCCManager.h index f38c473f..c9d69b1c 100644 --- a/closedcaptions/PlayerCCManager.h +++ b/closedcaptions/PlayerCCManager.h @@ -71,17 +71,6 @@ class PlayerCCManagerBase */ virtual void Release(int iID) = 0; - /** - * @brief Clear the stored control handle if it currently equals handle. - * Called by the handle owner's destructor so a handle can never be - * used after the object it points to is freed, independent of - * whether the GetId()/Release() usage count has reached zero (it - * may not have, if another session is still registered - see - * multi-pipeline mode). - * @param[in] handle - the handle being invalidated - */ - virtual void InvalidateHandle(void *) {} - /** * @fn SetStatus * @@ -258,6 +247,17 @@ class PlayerCCManagerBase */ virtual void ResetState(); + /** + * @brief Reset mOptions/mTrack/mTrackFormat/mLastTextTracks/mEnabled/ + * mTrickplayStarted/mParentalCtrlLocked to their initial values. + * Does NOT call Stop(). Subclasses whose SetTrack()/Initialize() + * access these fields under a control mutex must call this while + * holding that same mutex (after calling Stop() separately, + * outside the lock) instead of calling ResetState() directly, to + * avoid racing with a concurrent SetTrack()/Initialize() call. + */ + void ResetTrackState(); + /* NOTE WELL: The ResetState() method resets these member variables back to ** their initial state. It should be updated if any of the following change ** or are added to. */ @@ -268,6 +268,23 @@ class PlayerCCManagerBase bool mEnabled{false}; /**< true if CC rendering enabled, false otherwise */ bool mTrickplayStarted{false}; /**< If a trickplay is going on or not */ bool mParentalCtrlLocked{false}; /**< If Parental Control lock enabled on not */ + +public: + /** + * @brief Clear the stored control handle if it currently equals handle. + * Called by the handle owner's destructor so a handle can never be + * used after the object it points to is freed, independent of + * whether the GetId()/Release() usage count has reached zero (it + * may not have, if another session is still registered - see + * multi-pipeline mode). + * Declared last (appended after the pre-existing virtual + * interface, not inserted between Release() and SetStatus()) so it + * only extends the vtable instead of shifting every later slot, + * which would silently mis-dispatch calls made through a binary + * built against an older header. + * @param[in] handle - the handle being invalidated + */ + virtual void InvalidateHandle(void *) {} }; /** diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 2cf5c5da..383ec86e 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -159,11 +159,13 @@ int PlayerDirectRialtoCCManager::SetTrack( // mTrack/mTrackFormat are shared with Initialize() and ResetState(), so // they must be updated under the same lock as m_control. std::lock_guard lock(m_controlMutex); - mTrack = track; - mTrackFormat = format; if (m_control == nullptr) { + // No backend to apply against yet; cache unconditionally so + // Initialize() can apply it once a control becomes available. + mTrack = track; + mTrackFormat = format; MW_LOG_INFO("No control handle — track cached"); return 0; } @@ -171,6 +173,15 @@ int PlayerDirectRialtoCCManager::SetTrack( const std::string identifier = mapTrackIdentifier(track, format); MW_LOG_INFO("setTextTrackIdentifier=\"%s\"", identifier.c_str()); const bool ok = m_control->setTextTrackIdentifier(identifier); + if (ok) + { + // Only cache once the backend confirms the change, so a rejected + // track doesn't leave mTrack out of sync with what the control is + // still actually rendering (which would make a later Initialize() + // with the same handle wrongly skip reapplying it). + mTrack = track; + mTrackFormat = format; + } return ok ? 0 : -1; } @@ -203,11 +214,14 @@ void PlayerDirectRialtoCCManager::StopRendering() void PlayerDirectRialtoCCManager::ResetState() { MW_LOG_INFO("ENTRY"); - // PlayerCCManagerBase::ResetState() calls Stop() -> StopRendering(), - // which re-enters m_controlMutex; run it before taking the lock here to - // avoid deadlocking on this non-recursive mutex. - PlayerCCManagerBase::ResetState(); + // Stop() -> StopRendering() re-enters m_controlMutex; call it before + // taking the lock to avoid deadlocking on this non-recursive mutex. + Stop(); std::lock_guard lock(m_controlMutex); + // Reset the cached track/rendering fields under the same lock that + // SetTrack()/Initialize() use to access them, avoiding a race with a + // concurrent call to either. + ResetTrackState(); m_control = nullptr; MW_LOG_INFO("EXIT"); } diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.cpp b/closedcaptions/rialto/PlayerRialtoCCManager.cpp index 5b60d72f..05316ec5 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.cpp +++ b/closedcaptions/rialto/PlayerRialtoCCManager.cpp @@ -36,13 +36,17 @@ int PlayerRialtoCCManager::Initialize(void * handle) MW_LOG_INFO("PlayerRialtoCCManager::Initialize(%p) called", handle); bool changedHandle; + std::string cachedTrack; + CCFormat cachedFormat; { std::lock_guard lock(mControlMutex); changedHandle = (handle != mSubtitleControlHandle); mSubtitleControlHandle = handle; + cachedTrack = mTrack; + cachedFormat = mTrackFormat; } - if (GetTrack().empty()) + if (cachedTrack.empty()) { // Apps expect to render default CC as CC1, so set that here in case // they do not explicitly call SetTrack(). @@ -52,7 +56,7 @@ int PlayerRialtoCCManager::Initialize(void * handle) else if (changedHandle) { // Configure the new handle. - (void) SetTrack(GetTrack(), mTrackFormat); + (void) SetTrack(cachedTrack, cachedFormat); } return 0; @@ -76,8 +80,14 @@ int PlayerRialtoCCManager::GetId() void PlayerRialtoCCManager::ResetState() { MW_LOG_INFO("PlayerRialtoCCManager::Resetting"); - PlayerCCManagerBase::ResetState(); + // Stop() -> StopRendering() re-enters mControlMutex; call it before + // taking the lock to avoid deadlocking on this non-recursive mutex. + Stop(); std::lock_guard lock(mControlMutex); + // Reset the cached track/rendering fields under the same lock that + // SetTrack()/Initialize() use to access them, avoiding a race with a + // concurrent call to either. + ResetTrackState(); mSubtitleControlHandle = nullptr; } @@ -131,14 +141,14 @@ void PlayerRialtoCCManager::InvalidateHandle(void *handle) */ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat format) { - // Cache the original track string and the format so the prefix (if any) - // can be re-applied correctly from the cached values. - mTrack = track; // For PlayerCCManager::GetTrack() - mTrackFormat = format; - MW_LOG_INFO("PlayerRialtoCCManager::set track \"%s\"", track.c_str()); + // mTrack/mTrackFormat are shared with Initialize() and ResetState(), so + // they must be updated under the same lock as mSubtitleControlHandle. std::lock_guard lock(mControlMutex); + mTrack = track; // For PlayerCCManager::GetTrack() + mTrackFormat = format; + if (nullptr != mSubtitleControlHandle) { // We expect 'track' to have an alphabetic prefix. If it does not, diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.h b/closedcaptions/rialto/PlayerRialtoCCManager.h index 9acb6b80..f70d6aaf 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.h +++ b/closedcaptions/rialto/PlayerRialtoCCManager.h @@ -122,10 +122,11 @@ class PlayerRialtoCCManager : public PlayerCCManagerBase void ResetState() override; private: - /// Guards mSubtitleControlHandle. InvalidateHandle() also takes this - /// lock, so any SetTrack() / StartRendering() / StopRendering() call - /// already in progress on the old handle completes before - /// InvalidateHandle() clears the pointer and returns - preventing + /// Guards mSubtitleControlHandle and SetTrack()'s writes / Initialize()'s + /// reads of the inherited mTrack / mTrackFormat cache. InvalidateHandle() + /// also takes this lock, so any SetTrack() / StartRendering() / + /// StopRendering() call already in progress on the old handle completes + /// before InvalidateHandle() clears the pointer and returns - preventing /// use-after-free when the handle owner is destroyed concurrently. mutable std::mutex mControlMutex; diff --git a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp index acac0af8..f5ff98ec 100644 --- a/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp +++ b/test/utests/tests/DrmSessionManagerTests/DrmSessionManagerTestCases.cpp @@ -28,6 +28,7 @@ #include "MockOpenCdmSessionAdapter.h" #include "DrmCallbacks.h" #include "MockDrmSessionFactory.h" +#include "MockDrmSession.h" #include "DrmHelper.h" #include "WidevineDrmHelper.h" #include "DrmInfo.h" @@ -55,6 +56,11 @@ class TestableDrmSessionManager : public DrmSessionManager { } + TestableDrmSessionManager(int maxDrmSessions, void *player, std::function watermarkCallback, DrmSessionCreator creator) + : DrmSessionManager(maxDrmSessions, player, watermarkCallback, std::move(creator)), mMaxSessions(maxDrmSessions) + { + } + // Provide controlled access to cachedKeyIDs with mutex protection void AddKeyIdEntryToCache(int slot, const KeyIdEntries& entries) { @@ -1295,3 +1301,77 @@ TEST_F(DrmSessionManagerComplexTests, OpencdmConstructionFailureTest) EXPECT_EQ(err, MW_DRM_SESSION_CREATE_FAILED); } +/** + * @brief getDrmSession() must invoke an injected DrmSessionCreator (instead + * of DrmSessionFactory::GetDrmSession()) and transfer ownership of the + * returned unique_ptr into the selected session slot. + */ +TEST_F(DrmSessionManagerComplexTests, DrmSessionCreator_InvokedAndOwnershipTransferred) +{ + bool creatorInvoked = false; + DrmHelperPtr capturedHelper; + auto *mockSession = new NiceMock("com.widevine.alpha"); + ON_CALL(*mockSession, getState()).WillByDefault(Return(KEY_INIT)); + + DrmSessionCreator creator = + [&](DrmHelperPtr drmHelper, DrmCallbacks *instance) -> std::unique_ptr + { + creatorInvoked = true; + capturedHelper = drmHelper; + return std::unique_ptr(mockSession); + }; + + TestableDrmSessionManager manager(maxDrmSessions, nullptr, nullptr, creator); + // getDrmSession() unconditionally invokes ProfileUpdateCb; it must be + // registered first or the call throws std::bad_function_call. + manager.RegisterProfilingUpdateCb([]() {}); + + auto mockHelper = CreateMockDrmHelper("com.widevine.alpha"); + ON_CALL(*mockHelper, getKey(_)) + .WillByDefault(Invoke([](std::vector& keyID) { keyID = {0x01, 0x02, 0x03, 0x04}; })); + + int err = 0; + int selectedSlot = -1; + KeyState result = manager.getDrmSession(err, mockHelper, selectedSlot, nullptr); + + EXPECT_TRUE(creatorInvoked); + EXPECT_EQ(capturedHelper, mockHelper); + ASSERT_GE(selectedSlot, 0); + EXPECT_EQ(manager.drmSessionContexts[selectedSlot].drmSession, mockSession); + EXPECT_EQ(result, KEY_INIT); + EXPECT_EQ(err, 0); +} + +/** + * @brief getDrmSession() must report MW_DRM_INIT_FAILED and leave the slot's + * drmSession null when an injected DrmSessionCreator fails (returns nullptr). + */ +TEST_F(DrmSessionManagerComplexTests, DrmSessionCreator_ReturnsNull_ReportsInitFailure) +{ + bool creatorInvoked = false; + DrmSessionCreator creator = + [&](DrmHelperPtr, DrmCallbacks *) -> std::unique_ptr + { + creatorInvoked = true; + return nullptr; + }; + + TestableDrmSessionManager manager(maxDrmSessions, nullptr, nullptr, creator); + // getDrmSession() unconditionally invokes ProfileUpdateCb; it must be + // registered first or the call throws std::bad_function_call. + manager.RegisterProfilingUpdateCb([]() {}); + + auto mockHelper = CreateMockDrmHelper("com.widevine.alpha"); + ON_CALL(*mockHelper, getKey(_)) + .WillByDefault(Invoke([](std::vector& keyID) { keyID = {0x01, 0x02, 0x03, 0x04}; })); + + int err = 0; + int selectedSlot = -1; + manager.getDrmSession(err, mockHelper, selectedSlot, nullptr); + + EXPECT_TRUE(creatorInvoked); + ASSERT_GE(selectedSlot, 0); + EXPECT_EQ(manager.drmSessionContexts[selectedSlot].drmSession, nullptr); + EXPECT_EQ(err, MW_DRM_INIT_FAILED); +} + diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp index 7934db4a..bb3d1323 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerBaseStubs.cpp @@ -80,6 +80,11 @@ void PlayerCCManagerBase::ResetState() // mutex re-entrancy that PlayerDirectRialtoCCManager::ResetState() must // avoid deadlocking on. Stop(); + ResetTrackState(); +} + +void PlayerCCManagerBase::ResetTrackState() +{ mOptions.clear(); mTrack.clear(); mTrackFormat = eCLOSEDCAPTION_FORMAT_DEFAULT; diff --git a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp index 7c4c8c3f..39adb15d 100644 --- a/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp +++ b/test/utests/tests/PlayerDirectRialtoCCManagerTests/PlayerDirectRialtoCCManagerTestCases.cpp @@ -196,6 +196,26 @@ TEST_F(PlayerDirectRialtoCCManagerTest, EXPECT_EQ(m_mgr.SetTrack("2", eCLOSEDCAPTION_FORMAT_608), -1); } +/** + * @test SetTrack_ControlRejectsIdentifier_CacheKeepsPreviousTrack + * @brief A rejected SetTrack() must not update mTrack/mTrackFormat, so a + * later Initialize() with the same handle still sees a changed + * handle correctly re-apply the last-accepted track rather than + * skipping it because the cache already matched the rejected one. + */ +TEST_F(PlayerDirectRialtoCCManagerTest, + SetTrack_ControlRejectsIdentifier_CacheKeepsPreviousTrack) +{ + InitWithDefaultTrack(); + + EXPECT_CALL(*m_mock, setTextTrackIdentifier("CC2")) + .Times(1) + .WillOnce(Return(false)); + + EXPECT_EQ(m_mgr.SetTrack("2", eCLOSEDCAPTION_FORMAT_608), -1); + EXPECT_EQ(m_mgr.GetTrack(), "CC1"); +} + /** * @test SetTrack_NumericWith608Format_PrependsCCPrefix * @brief A numeric track string with 608 format should become "CC". From 516a5935232139cf93a5e2971a6df7d1bd151732 Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 11:38:35 +0000 Subject: [PATCH 16/23] Simplify SetRialto() to a default parameter, not an overload PlayerCCManager::SetRialto() had a two-argument version plus a one-argument overload kept only to preserve a "pre-existing mangled symbol for binaries linked against older releases." That reasoning doesn't hold: like DrmSessionManager's DrmSessionCreator parameter, this library is always rebuilt from source with its consumers, so there's no stable ABI symbol to preserve. Replace the two overloads with a single SetRialto(bool bIsRialto, bool bIsDirectRialto = false), matching the pattern already used for DrmSessionManager. Existing single-argument call sites continue to compile unchanged. --- closedcaptions/PlayerCCManager.cpp | 8 -------- closedcaptions/PlayerCCManager.h | 11 +---------- 2 files changed, 1 insertion(+), 18 deletions(-) diff --git a/closedcaptions/PlayerCCManager.cpp b/closedcaptions/PlayerCCManager.cpp index 09fc7eaf..f29c882b 100644 --- a/closedcaptions/PlayerCCManager.cpp +++ b/closedcaptions/PlayerCCManager.cpp @@ -920,14 +920,6 @@ void PlayerCCManager::SetRialto(bool bIsRialto, bool bIsDirectRialto) } } -/** - * @brief Set the CC manager variant (no direct-Rialto option). - */ -void PlayerCCManager::SetRialto(bool bIsRialto) -{ - SetRialto(bIsRialto, false); -} - /** * @brief Destroy instance */ diff --git a/closedcaptions/PlayerCCManager.h b/closedcaptions/PlayerCCManager.h index c9d69b1c..41642049 100644 --- a/closedcaptions/PlayerCCManager.h +++ b/closedcaptions/PlayerCCManager.h @@ -321,16 +321,7 @@ class PlayerCCManager * (PlayerDirectRialtoCCManager). * @return void */ - static void SetRialto(bool bIsRialto, bool bIsDirectRialto); - - /** - * @fn SetRialto - * @brief Preserves the pre-existing one-argument mangled symbol for - * binaries linked against older releases of this installed - * header; delegates to the two-argument overload. - * @return void - */ - static void SetRialto(bool bIsRialto); + static void SetRialto(bool bIsRialto, bool bIsDirectRialto = false); /** * @fn DestroyInstance From 7e5976e4a3b3f27cb3bcf5db56b5d0cf4c27a66f Mon Sep 17 00:00:00 2001 From: anshephe <115161257+anshephe@users.noreply.github.com> Date: Tue, 22 Sep 2026 12:03:30 +0000 Subject: [PATCH 17/23] Fix Initialize()/SetTrack() race in CC managers, propagate err on missing AcquireLicenseCb PlayerRialtoCCManager and PlayerDirectRialtoCCManager released their control mutex between assigning the new handle and applying the cached track via SetTrack(), letting a concurrent Initialize()/SetTrack() call interleave and configure the wrong handle. Add a private SetTrackLocked() in each and hold the mutex for Initialize()'s entire body so handle assignment and configuration happen atomically. This lets PlayerDirectRialtoCCManager drop clearControlOnFailure() in favor of an inline reset, since the lock is now held continuously from handle assignment through configuration. Also set err = MW_DRM_INIT_FAILED in DrmSessionManager::createDrmSession() when AcquireLicenseCb isn't registered, matching sibling failure paths; previously err was left unset and callers' `if (err != -1)` check silently swallowed the failure. --- .../PlayerDirectRialtoCCManager.cpp | 46 ++++++++----------- .../PlayerDirectRialtoCCManager.h | 8 ++-- .../rialto/PlayerRialtoCCManager.cpp | 31 +++++++------ closedcaptions/rialto/PlayerRialtoCCManager.h | 5 ++ drm/DrmSessionManager.cpp | 1 + 5 files changed, 44 insertions(+), 47 deletions(-) diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp index 383ec86e..86c5eeb4 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.cpp @@ -65,16 +65,12 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) MW_LOG_INFO("ENTRY handle=%p", handle); auto *newControl = static_cast(handle); - bool changedHandle; - std::string cachedTrack; - CCFormat cachedFormat; - { - std::lock_guard lock(m_controlMutex); - changedHandle = (newControl != m_control); - m_control = newControl; - cachedTrack = mTrack; - cachedFormat = mTrackFormat; - } + // Held for the whole handle-assignment + configuration sequence so a + // concurrent Initialize()/SetTrack() call can't interleave and configure + // the wrong handle (or leave this handle unconfigured). + std::lock_guard lock(m_controlMutex); + const bool changedHandle = (newControl != m_control); + m_control = newControl; if (newControl == nullptr) { @@ -83,25 +79,25 @@ int PlayerDirectRialtoCCManager::Initialize(void *handle) return 0; } - if (cachedTrack.empty()) + if (mTrack.empty()) { // Apps expect CC1 as the default; apply it so the first frame // renders without an explicit SetTextTrack() call. MW_LOG_INFO("Setting default track CC1"); - if (SetTrack("CC1") != 0) + if (SetTrackLocked("CC1", eCLOSEDCAPTION_FORMAT_DEFAULT) != 0) { MW_LOG_WARN("Failed to set default track CC1"); - clearControlOnFailure(newControl); + m_control = nullptr; return -1; } } else if (changedHandle) { // Re-apply the cached track on the new handle (e.g. re-tune). - if (SetTrack(cachedTrack, cachedFormat) != 0) + if (SetTrackLocked(mTrack, mTrackFormat) != 0) { MW_LOG_WARN("Failed to reapply cached track on new handle"); - clearControlOnFailure(newControl); + m_control = nullptr; return -1; } } @@ -154,11 +150,16 @@ void PlayerDirectRialtoCCManager::InvalidateHandle(void *handle) int PlayerDirectRialtoCCManager::SetTrack( const std::string &track, CCFormat format) { - MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); - // mTrack/mTrackFormat are shared with Initialize() and ResetState(), so // they must be updated under the same lock as m_control. std::lock_guard lock(m_controlMutex); + return SetTrackLocked(track, format); +} + +int PlayerDirectRialtoCCManager::SetTrackLocked( + const std::string &track, CCFormat format) +{ + MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast(format)); if (m_control == nullptr) { @@ -226,14 +227,3 @@ void PlayerDirectRialtoCCManager::ResetState() MW_LOG_INFO("EXIT"); } -void PlayerDirectRialtoCCManager::clearControlOnFailure(IDirectRialtoCC *handle) -{ - // A failed track application leaves the control unconfigured; clear it - // so a later Initialize() call with the same handle is treated as a - // handle change and retries SetTrack() instead of reporting success. - std::lock_guard lock(m_controlMutex); - if (m_control == handle) - { - m_control = nullptr; - } -} diff --git a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h index 0b272925..18c9c43a 100755 --- a/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h +++ b/closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h @@ -100,10 +100,10 @@ class PlayerDirectRialtoCCManager : public PlayerCCManagerBase static std::string mapTrackIdentifier(const std::string &track, CCFormat format); - /// Clear m_control if it still equals handle, forcing a later - /// Initialize() call with the same handle to retry SetTrack() instead - /// of treating it as an unchanged, already-configured handle. - void clearControlOnFailure(IDirectRialtoCC *handle); + /// SetTrack() body, assuming m_controlMutex is already held. Used by + /// SetTrack() itself and by Initialize(), so handle assignment and + /// configuration happen as one atomic critical section. + int SetTrackLocked(const std::string &track, CCFormat format); /// Guards m_control and SetTrack()'s writes / Initialize()'s reads of /// the inherited mTrack / mTrackFormat cache. InvalidateHandle() also diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.cpp b/closedcaptions/rialto/PlayerRialtoCCManager.cpp index 05316ec5..fd72e58f 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.cpp +++ b/closedcaptions/rialto/PlayerRialtoCCManager.cpp @@ -35,28 +35,24 @@ int PlayerRialtoCCManager::Initialize(void * handle) { MW_LOG_INFO("PlayerRialtoCCManager::Initialize(%p) called", handle); - bool changedHandle; - std::string cachedTrack; - CCFormat cachedFormat; - { - std::lock_guard lock(mControlMutex); - changedHandle = (handle != mSubtitleControlHandle); - mSubtitleControlHandle = handle; - cachedTrack = mTrack; - cachedFormat = mTrackFormat; - } + // Held for the whole handle-assignment + configuration sequence so a + // concurrent Initialize()/SetTrack() call can't interleave and configure + // the wrong handle (or leave this handle unconfigured). + std::lock_guard lock(mControlMutex); + const bool changedHandle = (handle != mSubtitleControlHandle); + mSubtitleControlHandle = handle; - if (cachedTrack.empty()) + if (mTrack.empty()) { // Apps expect to render default CC as CC1, so set that here in case // they do not explicitly call SetTrack(). MW_LOG_INFO("PlayerRialtoCCManager::Setting default to \"CC1\""); - (void) SetTrack("CC1"); + (void) SetTrackLocked("CC1", eCLOSEDCAPTION_FORMAT_DEFAULT); } else if (changedHandle) { // Configure the new handle. - (void) SetTrack(cachedTrack, cachedFormat); + (void) SetTrackLocked(mTrack, mTrackFormat); } return 0; @@ -141,11 +137,16 @@ void PlayerRialtoCCManager::InvalidateHandle(void *handle) */ int PlayerRialtoCCManager::SetTrack(const std::string &track, const CCFormat format) { - MW_LOG_INFO("PlayerRialtoCCManager::set track \"%s\"", track.c_str()); - // mTrack/mTrackFormat are shared with Initialize() and ResetState(), so // they must be updated under the same lock as mSubtitleControlHandle. std::lock_guard lock(mControlMutex); + return SetTrackLocked(track, format); +} + +int PlayerRialtoCCManager::SetTrackLocked(const std::string &track, const CCFormat format) +{ + MW_LOG_INFO("PlayerRialtoCCManager::set track \"%s\"", track.c_str()); + mTrack = track; // For PlayerCCManager::GetTrack() mTrackFormat = format; diff --git a/closedcaptions/rialto/PlayerRialtoCCManager.h b/closedcaptions/rialto/PlayerRialtoCCManager.h index f70d6aaf..a2ec6d5f 100644 --- a/closedcaptions/rialto/PlayerRialtoCCManager.h +++ b/closedcaptions/rialto/PlayerRialtoCCManager.h @@ -121,6 +121,11 @@ class PlayerRialtoCCManager : public PlayerCCManagerBase */ void ResetState() override; + /// SetTrack() body, assuming mControlMutex is already held. Used by + /// SetTrack() itself and by Initialize(), so handle assignment and + /// configuration happen as one atomic critical section. + int SetTrackLocked(const std::string &track, const CCFormat format); + private: /// Guards mSubtitleControlHandle and SetTrack()'s writes / Initialize()'s /// reads of the inherited mTrack / mTrackFormat cache. InvalidateHandle() diff --git a/drm/DrmSessionManager.cpp b/drm/DrmSessionManager.cpp index 7661ad81..29e62cfa 100755 --- a/drm/DrmSessionManager.cpp +++ b/drm/DrmSessionManager.cpp @@ -532,6 +532,7 @@ DrmSession* DrmSessionManager::createDrmSession(int &responseCode, int &err, std if (!AcquireLicenseCb) { MW_LOG_WARN("AcquireLicenseCb not registered - cannot acquire license"); + err = MW_DRM_INIT_FAILED; std::lock_guard guard(cachedKeyMutex); if (cachedKeyIDs) { From 1f593abd2f47c41a7ca16e934ff1ea503c35d006 Mon Sep 17 00:00:00 2001 From: varshnie Date: Tue, 25 Aug 2026 15:01:02 +0530 Subject: [PATCH 18/23] VPAAMP-959:[MP4DEMUX]Use codec info from the manifest rather than lazy caps setting in mp4demux Reason for change:Use codec info from the manifest rather than lazy caps setting in mp4demux Risks: p1 Signed-off-by: varshnie --- InterfacePlayerRDK.cpp | 84 +++++++++++++++---- InterfacePlayerRDK.h | 1 + .../InterfacePlayerFunctionTests.cpp | 47 +++++++++++ 3 files changed, 117 insertions(+), 15 deletions(-) diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index d6018c30..6bd89de8 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -86,6 +86,65 @@ const char * CipherTypeToString(CipherType type) return "unknown"; } +static void ApplyEncryptedCaps(GstCaps *caps, const char *drmSystem) +{ + if (caps == NULL) + { + return; + } + GstStructure *structure = gst_caps_get_structure(caps, 0); + if (structure) + { + gst_structure_set(structure, + "original-media-type", G_TYPE_STRING, gst_structure_get_name(structure), + NULL); + if (drmSystem != NULL) + { + gst_structure_set(structure, + GST_PROTECTION_SYSTEM_ID_CAPS_FIELD, G_TYPE_STRING, drmSystem, + NULL); + } + // Same for both cenc and cbcs + gst_structure_set_name(structure, "application/x-cenc"); + } +} + +static bool HasQueuedProtectionEvent(const GstPlayerPriv *gstPrivateContext, pthread_mutex_t& protectionLock) +{ + bool hasQueuedProtectionEvent = false; + pthread_mutex_lock(&protectionLock); + for (int i = 0; i < GST_TRACK_COUNT; i++) + { + if (gstPrivateContext->protectionEvent[i] != NULL) + { + hasQueuedProtectionEvent = true; + break; + } + } + pthread_mutex_unlock(&protectionLock); + return hasQueuedProtectionEvent; +} + +static bool IsElementaryAudioVideoFormat(GstStreamOutputFormat format) +{ + switch (format) + { + case GST_FORMAT_AUDIO_ES_MP3: + case GST_FORMAT_AUDIO_ES_AAC: + case GST_FORMAT_AUDIO_ES_AAC_RAW: + case GST_FORMAT_AUDIO_ES_AC3: + case GST_FORMAT_AUDIO_ES_EC3: + case GST_FORMAT_AUDIO_ES_ATMOS: + case GST_FORMAT_AUDIO_ES_AC4: + case GST_FORMAT_VIDEO_ES_H264: + case GST_FORMAT_VIDEO_ES_HEVC: + case GST_FORMAT_VIDEO_ES_MPEG2: + return true; + default: + return false; + } +} + /*InterfacePlayerRDK constructor*/ InterfacePlayerRDK::InterfacePlayerRDK(bool isRialto) : mProtectionLock(), mPauseInjector(false), mSourceSetupMutex(), stopCallback(NULL), tearDownCb(NULL), notifyFirstFrameCallback(NULL), @@ -1869,6 +1928,10 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void * GstCaps * caps = NULL; GstMediaType mediaType = static_cast(type); gst_media_stream *stream = &privatePlayer->gstPrivateContext->stream[mediaType]; + const bool hasQueuedProtectionEvent = HasQueuedProtectionEvent(privatePlayer->gstPrivateContext, _this->mProtectionLock); + const bool isElementaryAudioVideoFormat = IsElementaryAudioVideoFormat(stream->format); + const bool shouldApplyEncryptedCaps = m_gstConfigParam->enableEncryptedCaps && hasQueuedProtectionEvent && isElementaryAudioVideoFormat; + MW_LOG_MIL("InitializeSourceForPlayer entry type[%d] format[%d] hasQueuedProtectionEvent[%d] elementary[%d] encryptedCapsEnabled[%d]", mediaType, stream->format, hasQueuedProtectionEvent, isElementaryAudioVideoFormat, m_gstConfigParam->enableEncryptedCaps); privatePlayer->SignalConnect(source, "need-data", G_CALLBACK(gst_need_data), _this); privatePlayer->SignalConnect(source, "enough-data", G_CALLBACK(gst_enough_data), _this); /* Sets up the call back function for enough data event */ privatePlayer->SignalConnect(source, "seek-data", G_CALLBACK(gstappsrc_seek), _this); /* Sets up the call back function for seek data event */ @@ -1910,6 +1973,11 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void * if (caps != NULL) { + if (shouldApplyEncryptedCaps) + { + MW_LOG_MIL("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format); + ApplyEncryptedCaps(caps, mDrmSystem); + } gchar *capsStr = gst_caps_to_string(caps); MW_LOG_MIL("Setting caps for source[type:%d]: %s", mediaType, capsStr); g_free(capsStr); @@ -5561,21 +5629,7 @@ void InterfacePlayerRDK::SetStreamCaps(GstMediaType type, MediaCodecInfo&& codec } if (codecInfo.mIsEncrypted) { - GstStructure *s = gst_caps_get_structure (caps, 0); - if (s) - { - gst_structure_set (s, - "original-media-type", G_TYPE_STRING, gst_structure_get_name (s), - NULL); - if (mDrmSystem != NULL) - { - gst_structure_set (s, - GST_PROTECTION_SYSTEM_ID_CAPS_FIELD, G_TYPE_STRING, mDrmSystem, - NULL); - } - // Same for both cenc and cbcs - gst_structure_set_name (s, "application/x-cenc"); - } + ApplyEncryptedCaps(caps, mDrmSystem); } gchar* capsStr = gst_caps_to_string(caps); MW_LOG_MIL("Setting stream caps for type[%d] format[%d]: %s", type, codecInfo.mCodecFormat, capsStr); diff --git a/InterfacePlayerRDK.h b/InterfacePlayerRDK.h index ba5a0908..3c0f7d6c 100644 --- a/InterfacePlayerRDK.h +++ b/InterfacePlayerRDK.h @@ -137,6 +137,7 @@ struct Configs int monitorAvsyncThresholdPositiveMs; int monitorAvsyncThresholdNegativeMs; int monitorAvJumpThresholdMs; + bool enableEncryptedCaps; }; diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index 40a6fb29..dd46fdb2 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -797,6 +797,53 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_Video) delete g_mockGstUtils; } +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsEnabled) +{ + g_mockGstUtils = new StrictMock(); + + void* playerInstance = mInterfaceGstPlayer; + void* source = reinterpret_cast(0x1234); + GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; + GstCaps caps = {}; + GstStructure structure = {}; + gst_media_stream* stream = &mPlayerContext->stream[mediaType]; + stream->format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->protectionEvent[mediaType] = reinterpret_cast(0x1234); + mPlayerConfigParams->videoBufBytes = 500; + mPlayerConfigParams->enableEncryptedCaps = true; + + EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(&caps, 0)).WillOnce(Return(&structure)); + EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("original-media-type"))).Times(1); + + mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); + + delete g_mockGstUtils; + g_mockGstUtils = nullptr; +} + +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsDisabled) +{ + g_mockGstUtils = new StrictMock(); + + void* playerInstance = mInterfaceGstPlayer; + void* source = reinterpret_cast(0x1234); + GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; + GstCaps caps = {}; + gst_media_stream* stream = &mPlayerContext->stream[mediaType]; + stream->format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->protectionEvent[mediaType] = reinterpret_cast(0x1234); + mPlayerConfigParams->videoBufBytes = 500; + mPlayerConfigParams->enableEncryptedCaps = false; + + EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(_, _)).Times(0); + + mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); + + delete g_mockGstUtils; +} + TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_Audio_CapsNull) { From e4076228cd4c5546f15ee1010b66926e1e951fc9 Mon Sep 17 00:00:00 2001 From: varshnie Date: Wed, 9 Sep 2026 12:05:32 +0530 Subject: [PATCH 19/23] VPAAMP-959:[MP4DEMUX]Use codec info from the manifest rather than lazy caps setting in mp4demux Reason for change:Use codec info from the manifest rather than lazy caps setting in mp4demux without cfg Risks: p1 Signed-off-by: varshnie --- DemuxDataTypes.h | 11 +++ InterfacePlayerPriv.h | 3 +- InterfacePlayerRDK.cpp | 75 +++++++----------- InterfacePlayerRDK.h | 4 +- test/pi-cli/commandProcessing.cpp | 5 +- .../tests/GstPlayer/FunctionalTests.cpp | 5 +- .../tests/GstPlayer/PauseOnPlaybackTests.cpp | 11 +-- .../InterfacePlayerFunctionTests.cpp | 79 ++++++++++++++++--- 8 files changed, 123 insertions(+), 70 deletions(-) diff --git a/DemuxDataTypes.h b/DemuxDataTypes.h index 76d73d48..5a61c9f8 100644 --- a/DemuxDataTypes.h +++ b/DemuxDataTypes.h @@ -173,6 +173,17 @@ struct MediaCodecInfo } }; +/* + * @struct PipelineCodecInfo + * @brief Codec information for all tracks in a pipeline + */ +struct PipelineCodecInfo +{ + MediaCodecInfo video; + MediaCodecInfo audio; + MediaCodecInfo subtitle; +}; + /* * @struct MediaDrmMetadata * @brief DRM metadata for encrypted samples diff --git a/InterfacePlayerPriv.h b/InterfacePlayerPriv.h index 19270489..b2fc8f43 100755 --- a/InterfacePlayerPriv.h +++ b/InterfacePlayerPriv.h @@ -125,6 +125,7 @@ struct gst_media_stream GstElement *sinkbin; /**< Sink element to consume data */ GstElement *source; /**< to provide data to the pipeline */ GstStreamOutputFormat format; /**< Stream output format for this stream */ + MediaCodecInfo codecInfo; /**< Codec and encryption information for this stream */ bool pendingSeek; /**< Flag denotes if a seek event has to be sent to the source */ bool resetPosition; /**< To indicate that the position of the stream is reset */ bool bufferUnderrun; @@ -137,7 +138,7 @@ struct gst_media_stream GstPad *demuxPad; /**< Demux src pad >*/ gulong demuxProbeId; /**< Demux pad probe ID >*/ - gst_media_stream() : sinkbin(NULL), source(NULL), format(GST_FORMAT_INVALID), + gst_media_stream() : sinkbin(NULL), source(NULL), format(GST_FORMAT_INVALID), codecInfo(), pendingSeek(false), resetPosition(false), bufferUnderrun(false), eosReached(false), sourceConfigured(false), sourceLock(PTHREAD_MUTEX_INITIALIZER), timeScale(1), trackId(-1), firstBufferProcessed(false), demuxPad(NULL), demuxProbeId(0) { diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index 6bd89de8..1f62cd2d 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -86,12 +86,22 @@ const char * CipherTypeToString(CipherType type) return "unknown"; } -static void ApplyEncryptedCaps(GstCaps *caps, const char *drmSystem) +/** + * @brief Apply encrypted content signaling fields to stream caps. + * @param[in,out] caps Caps to update in-place. + * @param[in] drmSystem DRM system UUID string for protection metadata. + */ +static void TransformToEncryptedCaps(GstCaps *caps, const char *drmSystem) { if (caps == NULL) { + MW_LOG_WARN("Skipping encrypted caps conversion: caps is NULL"); return; } + if (drmSystem == NULL) + { + MW_LOG_WARN("DRM system ID is NULL; encrypted caps will omit protection-system"); + } GstStructure *structure = gst_caps_get_structure(caps, 0); if (structure) { @@ -107,41 +117,9 @@ static void ApplyEncryptedCaps(GstCaps *caps, const char *drmSystem) // Same for both cenc and cbcs gst_structure_set_name(structure, "application/x-cenc"); } -} - -static bool HasQueuedProtectionEvent(const GstPlayerPriv *gstPrivateContext, pthread_mutex_t& protectionLock) -{ - bool hasQueuedProtectionEvent = false; - pthread_mutex_lock(&protectionLock); - for (int i = 0; i < GST_TRACK_COUNT; i++) - { - if (gstPrivateContext->protectionEvent[i] != NULL) - { - hasQueuedProtectionEvent = true; - break; - } - } - pthread_mutex_unlock(&protectionLock); - return hasQueuedProtectionEvent; -} - -static bool IsElementaryAudioVideoFormat(GstStreamOutputFormat format) -{ - switch (format) + else { - case GST_FORMAT_AUDIO_ES_MP3: - case GST_FORMAT_AUDIO_ES_AAC: - case GST_FORMAT_AUDIO_ES_AAC_RAW: - case GST_FORMAT_AUDIO_ES_AC3: - case GST_FORMAT_AUDIO_ES_EC3: - case GST_FORMAT_AUDIO_ES_ATMOS: - case GST_FORMAT_AUDIO_ES_AC4: - case GST_FORMAT_VIDEO_ES_H264: - case GST_FORMAT_VIDEO_ES_HEVC: - case GST_FORMAT_VIDEO_ES_MPEG2: - return true; - default: - return false; + MW_LOG_WARN("Skipping encrypted caps conversion: failed to get caps structure"); } } @@ -330,9 +308,9 @@ static GstStateChangeReturn SetStateWithWarnings(GstElement *element, GstState t * @param[in] drmMetadata The DRM metadata */ static void DecorateGstBufferWithDrmMetadata(GstBuffer *buffer, const MediaDrmMetadata &drmMetadata); - /** * @brief Configures the GStreamer pipeline. + * @param codecInfo Pipeline codec information. * @param format Video format. * @param audioFormat Audio format. * @param subFormat Whether subtitle format is enabled. @@ -348,7 +326,7 @@ static void DecorateGstBufferWithDrmMetadata(GstBuffer *buffer, const MediaDrmMe * @param enableLiveLatency Whether to enable live-latency mode in the * RialtoSink streams-info context (passed as enable-live-latency). */ -void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subFormat, +void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int format, int audioFormat, int subFormat, bool bESChangeStatus, bool setReadyAfterPipelineCreation, bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string manifestUrl, bool enableLiveLatency) { @@ -356,6 +334,11 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF GstStreamOutputFormat gstFormat = static_cast(format); GstStreamOutputFormat gstAudioFormat = static_cast(audioFormat); GstStreamOutputFormat gstSubFormat = static_cast(subFormat); + MediaCodecInfo* codecInfoByTrack[GST_TRACK_COUNT] = { + &codecInfo.video, + &codecInfo.audio, + &codecInfo.subtitle + }; GstStreamOutputFormat newFormat[GST_TRACK_COUNT]; newFormat[eGST_MEDIATYPE_VIDEO] = gstFormat; @@ -443,10 +426,10 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF for (int i = 0; i < GST_TRACK_COUNT; i++) { gst_media_stream *stream = &interfacePlayerPriv->gstPrivateContext->stream[i]; - if(stream->format != newFormat[i]) + bool isValidNewFormat = (newFormat[i] != GST_FORMAT_INVALID && newFormat[i] != GST_FORMAT_UNKNOWN); + if(stream->format != newFormat[i] || (isValidNewFormat && stream->codecInfo.mIsEncrypted != codecInfoByTrack[i]->mIsEncrypted)) { bool isInitialSetup = (stream->format == GST_FORMAT_INVALID || stream->format == GST_FORMAT_UNKNOWN); - bool isValidNewFormat = (newFormat[i] != GST_FORMAT_INVALID && newFormat[i] != GST_FORMAT_UNKNOWN); if (isValidNewFormat || isInitialSetup) { MW_LOG_MIL("Closing stream %d old format = %d, new format = %d",i, stream->format, newFormat[i]); @@ -493,6 +476,7 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF trickTeardown = false; TearDownStream((int)i); stream->format = newFormat[i]; + stream->codecInfo = std::move(*codecInfoByTrack[i]); stream->trackId = trackId; /* Sets up the stream for the given MediaType */ @@ -1928,10 +1912,7 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void * GstCaps * caps = NULL; GstMediaType mediaType = static_cast(type); gst_media_stream *stream = &privatePlayer->gstPrivateContext->stream[mediaType]; - const bool hasQueuedProtectionEvent = HasQueuedProtectionEvent(privatePlayer->gstPrivateContext, _this->mProtectionLock); - const bool isElementaryAudioVideoFormat = IsElementaryAudioVideoFormat(stream->format); - const bool shouldApplyEncryptedCaps = m_gstConfigParam->enableEncryptedCaps && hasQueuedProtectionEvent && isElementaryAudioVideoFormat; - MW_LOG_MIL("InitializeSourceForPlayer entry type[%d] format[%d] hasQueuedProtectionEvent[%d] elementary[%d] encryptedCapsEnabled[%d]", mediaType, stream->format, hasQueuedProtectionEvent, isElementaryAudioVideoFormat, m_gstConfigParam->enableEncryptedCaps); + MW_LOG_MIL("Entry type[%d] format[%d] encrypted[%d]", mediaType, stream->format, stream->codecInfo.mIsEncrypted); privatePlayer->SignalConnect(source, "need-data", G_CALLBACK(gst_need_data), _this); privatePlayer->SignalConnect(source, "enough-data", G_CALLBACK(gst_enough_data), _this); /* Sets up the call back function for enough data event */ privatePlayer->SignalConnect(source, "seek-data", G_CALLBACK(gstappsrc_seek), _this); /* Sets up the call back function for seek data event */ @@ -1973,10 +1954,10 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void * if (caps != NULL) { - if (shouldApplyEncryptedCaps) + if (stream->codecInfo.mIsEncrypted) { - MW_LOG_MIL("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format); - ApplyEncryptedCaps(caps, mDrmSystem); + MW_LOG_DEBUG("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format); + TransformToEncryptedCaps(caps, mDrmSystem); } gchar *capsStr = gst_caps_to_string(caps); MW_LOG_MIL("Setting caps for source[type:%d]: %s", mediaType, capsStr); @@ -5629,7 +5610,7 @@ void InterfacePlayerRDK::SetStreamCaps(GstMediaType type, MediaCodecInfo&& codec } if (codecInfo.mIsEncrypted) { - ApplyEncryptedCaps(caps, mDrmSystem); + TransformToEncryptedCaps(caps, mDrmSystem); } gchar* capsStr = gst_caps_to_string(caps); MW_LOG_MIL("Setting stream caps for type[%d] format[%d]: %s", type, codecInfo.mCodecFormat, capsStr); diff --git a/InterfacePlayerRDK.h b/InterfacePlayerRDK.h index 3c0f7d6c..ba8067a7 100644 --- a/InterfacePlayerRDK.h +++ b/InterfacePlayerRDK.h @@ -137,7 +137,6 @@ struct Configs int monitorAvsyncThresholdPositiveMs; int monitorAvsyncThresholdNegativeMs; int monitorAvJumpThresholdMs; - bool enableEncryptedCaps; }; @@ -353,7 +352,8 @@ class InterfacePlayerRDK * @param enableLiveLatency Whether to enable live-latency mode in the * RialtoSink streams-info context. Defaults to false. */ - void ConfigurePipeline(int, int, int, bool, bool, bool, int32_t, gint, const char *, int, bool, std::string url, bool enableLiveLatency = false); + void ConfigurePipeline(PipelineCodecInfo&& codecInfo, int format, int audioFormat, int subFormat, bool bESChangeStatus, bool setReadyAfterPipelineCreation, bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string url, bool enableLiveLatency = false); + /** * @brief Enables or disables pausing on playback start. * @param enable True to enable pausing, false to disable. diff --git a/test/pi-cli/commandProcessing.cpp b/test/pi-cli/commandProcessing.cpp index b47cd9b3..8526a382 100644 --- a/test/pi-cli/commandProcessing.cpp +++ b/test/pi-cli/commandProcessing.cpp @@ -206,7 +206,10 @@ void configurePipelineCommand(InterfacePlayerRDK& player, const std::vector(format)), + MediaCodecInfo(static_cast(audioFormat)), + MediaCodecInfo(static_cast(subFormat))}, bESChangeStatus, setReadyAfterPipelineCreation, isSubEnable, trackId, rate, pipelineName, PipelinePriority, subBool, url ); diff --git a/test/utests/tests/GstPlayer/FunctionalTests.cpp b/test/utests/tests/GstPlayer/FunctionalTests.cpp index 538ad5c6..fa2c33ea 100644 --- a/test/utests/tests/GstPlayer/FunctionalTests.cpp +++ b/test/utests/tests/GstPlayer/FunctionalTests.cpp @@ -253,7 +253,10 @@ class GstPlayerTests : public ::testing::Test EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_element_pipeline, GST_STATE_PLAYING)) .WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_VIDEO_ES_H264, + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_AUDIO_ES_AAC, + GST_FORMAT_SUBTITLE_TTML}, + GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_TTML, setup->bESChangeStatus, diff --git a/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp b/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp index 48f01d9f..70b9a966 100644 --- a/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp +++ b/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp @@ -31,14 +31,11 @@ using ::testing::NiceMock; using ::testing::Return; using ::testing::StrEq; -using ::testing::Eq; using ::testing::_; -using ::testing::Address; using ::testing::DoAll; using ::testing::SetArgPointee; using ::testing::NotNull; using ::testing::SaveArgPointee; -using ::testing::SaveArg; using ::testing::Pointer; class PauseOnPlaybackTests : public ::testing::Test @@ -141,7 +138,7 @@ TEST_F(PauseOnPlaybackTests, EnteredPausedSteHandler_ConfigurePauseOnPlayback) // Enable PauseOnPlayback mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); } // Test configuration of pipeline with PauseOnPlayback not enabled @@ -176,7 +173,7 @@ TEST_F(PauseOnPlaybackTests, EnteredPausedSteHandler_ConfigureNormalPlayback) EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_element_pipeline, GST_STATE_PLAYING)) .WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); } // Test bus_message callback when PauseOnPlayback has been enabled, and sink @@ -233,7 +230,7 @@ TEST_F(PauseOnPlaybackTests, bus_messsage_FrameStepPropertyAvailable) mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); ASSERT_TRUE(bus_sync_func != nullptr); ASSERT_TRUE(bus_message_func != nullptr); @@ -333,7 +330,7 @@ TEST_F(PauseOnPlaybackTests, bus_message_FrameStepPropertyNotAvailable) mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); ASSERT_TRUE(bus_sync_func != nullptr); ASSERT_TRUE(bus_message_func != nullptr); diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index dd46fdb2..b8e0180b 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -129,7 +129,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithWesterosAndRealtoSink) mPlayerConfigParams->useRialtoSink = true; EXPECT_EQ(mPlayerContext->usingRialtoSink, false); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->using_westerossink, true); EXPECT_EQ(mPlayerContext->usingRialtoSink, true); @@ -138,18 +138,43 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithWesterosAndRealtoSink) TEST_F(InterfacePlayerTests, ConfigurePipeline_WithSubtitlesEnabled) { g_mockGStreamer = nullptr; - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, true, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, true, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_SUBTITLE].format, GST_FORMAT_INVALID); } +TEST_F(InterfacePlayerTests, ConfigurePipeline_WithEncryptedCodecInfo) +{ + g_mockGStreamer = nullptr; + MediaCodecInfo videoCodecInfo(GST_FORMAT_VIDEO_ES_H264); + videoCodecInfo.mIsEncrypted = true; + + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + + EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); +} + +// Validate that an encryption-only change does not configure or count a track when its format is invalid. +TEST_F(InterfacePlayerTests, ConfigurePipeline_IgnoresEncryptionChangeForInvalidFormat) +{ + g_mockGStreamer = nullptr; + mPlayerContext->NumberOfTracks = 0; + MediaCodecInfo videoCodecInfo(GST_FORMAT_INVALID); + videoCodecInfo.mIsEncrypted = true; + + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); +} + TEST_F(InterfacePlayerTests, ConfigurePipeline_WithBufferingEnabled) { g_mockGStreamer = nullptr; mPlayerContext->buffering_enabled = true; mPlayerContext->rate = GST_NORMAL_PLAY_RATE; - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->buffering_in_progress, true); EXPECT_EQ(mPlayerContext->buffering_target_state, GST_STATE_PLAYING); @@ -164,7 +189,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_StreamConfiguration) EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->NumberOfTracks, 2); EXPECT_EQ(cbResponse, 5); //callback was called @@ -179,7 +204,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_ESChange) EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - mInterfaceGstPlayer->ConfigurePipeline(GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, true, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, true, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); EXPECT_EQ(cbResponse, 5); @@ -797,7 +822,8 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_Video) delete g_mockGstUtils; } -TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsEnabled) +// Validate that encrypted elementary stream caps include the original media type and configured DRM system. +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCaps) { g_mockGstUtils = new StrictMock(); @@ -808,13 +834,15 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream GstStructure structure = {}; gst_media_stream* stream = &mPlayerContext->stream[mediaType]; stream->format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->protectionEvent[mediaType] = reinterpret_cast(0x1234); + stream->codecInfo = MediaCodecInfo(stream->format); + stream->codecInfo.mIsEncrypted = true; mPlayerConfigParams->videoBufBytes = 500; - mPlayerConfigParams->enableEncryptedCaps = true; + mInterfaceGstPlayer->SetPreferredDRM("urn:uuid:edef8ba9-79d6-4ace-a3c8-27dcd51d21ed"); EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(&caps, 0)).WillOnce(Return(&structure)); EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("original-media-type"))).Times(1); + EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("protection-system"))).Times(1); mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); @@ -822,7 +850,8 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream g_mockGstUtils = nullptr; } -TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsDisabled) +// Validate that encrypted elementary stream caps retain the original media type without adding a missing DRM system. +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsWithoutDrmSystem) { g_mockGstUtils = new StrictMock(); @@ -830,11 +859,37 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream void* source = reinterpret_cast(0x1234); GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; GstCaps caps = {}; + GstStructure structure = {}; gst_media_stream* stream = &mPlayerContext->stream[mediaType]; stream->format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->protectionEvent[mediaType] = reinterpret_cast(0x1234); + stream->codecInfo = MediaCodecInfo(stream->format); + stream->codecInfo.mIsEncrypted = true; + mPlayerConfigParams->videoBufBytes = 500; + + EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(&caps, 0)).WillOnce(Return(&structure)); + EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("original-media-type"))).Times(1); + EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("protection-system"))).Times(0); + + mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); + + delete g_mockGstUtils; + g_mockGstUtils = nullptr; +} + +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_ClearElementaryStreamCaps) +{ + g_mockGstUtils = new StrictMock(); + + void* playerInstance = mInterfaceGstPlayer; + void* source = reinterpret_cast(0x1234); + GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; + GstCaps caps = {}; + gst_media_stream* stream = &mPlayerContext->stream[mediaType]; + stream->format = GST_FORMAT_VIDEO_ES_H264; + stream->codecInfo = MediaCodecInfo(stream->format); + stream->codecInfo.mIsEncrypted = false; mPlayerConfigParams->videoBufBytes = 500; - mPlayerConfigParams->enableEncryptedCaps = false; EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(_, _)).Times(0); @@ -3159,6 +3214,7 @@ TEST_F(InterfacePlayerTests, SetStreamCaps_ValidVideoCodecFormat) TEST_F(InterfacePlayerTests, SetStreamCaps_EncryptedAudioCodecFormat) { g_mockGstUtils = new StrictMock(); + mInterfaceGstPlayer->SetPreferredDRM("urn:uuid:edef8ba9-79d6-4ace-a3c8-27dcd51d21ed"); MediaCodecInfo codecInfo; codecInfo.mCodecFormat = GST_FORMAT_AUDIO_ES_AAC_RAW; codecInfo.mCodecData = std::vector{0x00, 0x00, 0x00, 0x01}; @@ -3172,6 +3228,7 @@ TEST_F(InterfacePlayerTests, SetStreamCaps_EncryptedAudioCodecFormat) EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(&caps, 0)).WillRepeatedly(Return(&s)); EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&s, StrEq("codec_data"))).Times(1); EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&s, StrEq("original-media-type"))).Times(1); + EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&s, StrEq("protection-system"))).Times(1); EXPECT_CALL(*g_mockGStreamer, gst_caps_set_simple(&caps, StrEq("channels"))).Times(1); EXPECT_CALL(*g_mockGStreamer, gst_app_src_set_caps(_, &caps)).Times(1); From 86a62b1c12e65d03727441f5d5491e4e114c0a72 Mon Sep 17 00:00:00 2001 From: Vinish100 Date: Fri, 11 Sep 2026 15:21:38 +0530 Subject: [PATCH 20/23] Make ConfigurePipeline backward compatible, rename to StreamCodecInfo and minor cleanup --- DemuxDataTypes.h | 6 +- InterfacePlayerRDK.cpp | 89 +++++++++++++------ InterfacePlayerRDK.h | 21 ++++- test/pi-cli/commandProcessing.cpp | 2 +- .../tests/GstPlayer/FunctionalTests.cpp | 5 +- .../tests/GstPlayer/PauseOnPlaybackTests.cpp | 8 +- .../InterfacePlayerFunctionTests.cpp | 76 +++++++++++++--- 7 files changed, 153 insertions(+), 54 deletions(-) diff --git a/DemuxDataTypes.h b/DemuxDataTypes.h index 5a61c9f8..d0b613f3 100644 --- a/DemuxDataTypes.h +++ b/DemuxDataTypes.h @@ -174,10 +174,10 @@ struct MediaCodecInfo }; /* - * @struct PipelineCodecInfo - * @brief Codec information for all tracks in a pipeline + * @struct StreamCodecInfo + * @brief Codec information for all tracks in a stream */ -struct PipelineCodecInfo +struct StreamCodecInfo { MediaCodecInfo video; MediaCodecInfo audio; diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index 1f62cd2d..f81fdc16 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -86,6 +86,11 @@ const char * CipherTypeToString(CipherType type) return "unknown"; } +static bool IsElementaryStreamFormat(GstStreamOutputFormat format) +{ + return format >= GST_FORMAT_AUDIO_ES_MP3 && format <= GST_FORMAT_VIDEO_ES_MPEG2; +} + /** * @brief Apply encrypted content signaling fields to stream caps. * @param[in,out] caps Caps to update in-place. @@ -100,7 +105,8 @@ static void TransformToEncryptedCaps(GstCaps *caps, const char *drmSystem) } if (drmSystem == NULL) { - MW_LOG_WARN("DRM system ID is NULL; encrypted caps will omit protection-system"); + MW_LOG_WARN("Skipping encrypted caps conversion: DRM system ID is NULL"); + return; } GstStructure *structure = gst_caps_get_structure(caps, 0); if (structure) @@ -308,9 +314,9 @@ static GstStateChangeReturn SetStateWithWarnings(GstElement *element, GstState t * @param[in] drmMetadata The DRM metadata */ static void DecorateGstBufferWithDrmMetadata(GstBuffer *buffer, const MediaDrmMetadata &drmMetadata); + /** * @brief Configures the GStreamer pipeline. - * @param codecInfo Pipeline codec information. * @param format Video format. * @param audioFormat Audio format. * @param subFormat Whether subtitle format is enabled. @@ -326,14 +332,38 @@ static void DecorateGstBufferWithDrmMetadata(GstBuffer *buffer, const MediaDrmMe * @param enableLiveLatency Whether to enable live-latency mode in the * RialtoSink streams-info context (passed as enable-live-latency). */ -void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int format, int audioFormat, int subFormat, +void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subFormat, + bool bESChangeStatus, bool setReadyAfterPipelineCreation, + bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string manifestUrl, bool enableLiveLatency) +{ + StreamCodecInfo streamInfo; + streamInfo.video.mCodecFormat = static_cast(format); + streamInfo.audio.mCodecFormat = static_cast(audioFormat); + streamInfo.subtitle.mCodecFormat = static_cast(subFormat); + ConfigurePipeline(std::move(streamInfo), bESChangeStatus, setReadyAfterPipelineCreation, + isSubEnable, trackId, rate, pipelineName, PipelinePriority, FirstFrameFlag, std::move(manifestUrl), enableLiveLatency); +} + +/** + * @brief Configures the GStreamer pipeline. + * @param codecInfo Codec information for the stream. + * @param bESChangeStatus Whether ES change status is enabled. + * @param setReadyAfterPipelineCreation Whether to set the player as ready after pipeline creation. + * @param isSubEnable Whether subtitles are enabled. + * @param trackId Track ID. + * @param rate Bitrate. + * @param pipelineName Pipeline name. + * @param PipelinePriority Pipeline priority. + * @param FirstFrameFlag Whether the first-frame callback is required. + * @param manifestUrl URL of the manifest used to configure stream setup. + * @param enableLiveLatency Whether to enable live-latency mode in the + * RialtoSink streams-info context (passed as enable-live-latency). + */ +void InterfacePlayerRDK::ConfigurePipeline(StreamCodecInfo&& codecInfo, bool bESChangeStatus, bool setReadyAfterPipelineCreation, bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string manifestUrl, bool enableLiveLatency) { mFirstFrameRequired = FirstFrameFlag; - GstStreamOutputFormat gstFormat = static_cast(format); - GstStreamOutputFormat gstAudioFormat = static_cast(audioFormat); - GstStreamOutputFormat gstSubFormat = static_cast(subFormat); MediaCodecInfo* codecInfoByTrack[GST_TRACK_COUNT] = { &codecInfo.video, &codecInfo.audio, @@ -341,20 +371,20 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for }; GstStreamOutputFormat newFormat[GST_TRACK_COUNT]; - newFormat[eGST_MEDIATYPE_VIDEO] = gstFormat; - newFormat[eGST_MEDIATYPE_AUDIO] = gstAudioFormat; + newFormat[eGST_MEDIATYPE_VIDEO] = static_cast(codecInfo.video.mCodecFormat);; + newFormat[eGST_MEDIATYPE_AUDIO] = static_cast(codecInfo.audio.mCodecFormat); bool newClosedCaptionsControl = false; if(isSubEnable) { MW_LOG_MIL("Gstreamer subs enabled"); - newFormat[eGST_MEDIATYPE_SUBTITLE] = gstSubFormat; + newFormat[eGST_MEDIATYPE_SUBTITLE] = static_cast(codecInfo.subtitle.mCodecFormat); } else { MW_LOG_MIL("Gstreamer subs disabled"); - newFormat[eGST_MEDIATYPE_SUBTITLE]=GST_FORMAT_INVALID; + newFormat[eGST_MEDIATYPE_SUBTITLE] = GST_FORMAT_INVALID; } if(!(m_gstConfigParam->useWesterosSink)) @@ -362,7 +392,6 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for interfacePlayerPriv->gstPrivateContext->using_westerossink = false; interfacePlayerPriv->gstPrivateContext->firstTuneWithWesterosSinkOff = interfacePlayerPriv->socInterface->IsFirstTuneWithWesteros(); } - else { interfacePlayerPriv->gstPrivateContext->using_westerossink = true; @@ -379,7 +408,7 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for interfacePlayerPriv->gstPrivateContext->usingRialtoSink = true; // If no subtitles defined, then create a closed caption control stream - newClosedCaptionsControl = (gstSubFormat == GST_FORMAT_INVALID); + newClosedCaptionsControl = (static_cast(codecInfo.subtitle.mCodecFormat) == GST_FORMAT_INVALID); // To avoid out of band subtitles being removed during trickplay, // check if they were previously configured, and don't enable Closed Caption Control. @@ -426,20 +455,21 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for for (int i = 0; i < GST_TRACK_COUNT; i++) { gst_media_stream *stream = &interfacePlayerPriv->gstPrivateContext->stream[i]; + bool isInitialSetup = (stream->format == GST_FORMAT_INVALID || stream->format == GST_FORMAT_UNKNOWN); bool isValidNewFormat = (newFormat[i] != GST_FORMAT_INVALID && newFormat[i] != GST_FORMAT_UNKNOWN); - if(stream->format != newFormat[i] || (isValidNewFormat && stream->codecInfo.mIsEncrypted != codecInfoByTrack[i]->mIsEncrypted)) + bool isEncryptionChanged = (stream->codecInfo.mIsEncrypted != codecInfoByTrack[i]->mIsEncrypted); + bool isFormatChanged = (stream->format != newFormat[i] || isEncryptionChanged); + // Reconfigure pipeline if this is the first setup, or the encryption status or format has changed + bool shouldReconfigure = isValidNewFormat && (isInitialSetup || isFormatChanged); + if(shouldReconfigure) { - bool isInitialSetup = (stream->format == GST_FORMAT_INVALID || stream->format == GST_FORMAT_UNKNOWN); - if (isValidNewFormat || isInitialSetup) - { - MW_LOG_MIL("Closing stream %d old format = %d, new format = %d",i, stream->format, newFormat[i]); - configureStream[i] = true; - interfacePlayerPriv->gstPrivateContext->NumberOfTracks++; - } - else - { - MW_LOG_MIL("Skipping reconfiguration for stream %d - both format invalid/unknown",i); - } + MW_LOG_MIL("Closing stream %d old format = %d, new format = %d",i, stream->format, newFormat[i]); + configureStream[i] = true; + interfacePlayerPriv->gstPrivateContext->NumberOfTracks++; + } + else + { + MW_LOG_MIL("Skipping reconfiguration for stream %d for [%d]->[%d], isEncryptionChanged = %d",i, stream->format, newFormat[i], isEncryptionChanged); } if(interfacePlayerPriv->socInterface->ShouldTearDownForTrickplay()) { @@ -511,7 +541,7 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for g_object_get(interfacePlayerPriv->gstPrivateContext->stream[eGST_MEDIATYPE_VIDEO].sinkbin, "video-sink", &vidsink, NULL); if(vidsink) { - gboolean videoOnly = (audioFormat == GST_FORMAT_INVALID); + gboolean videoOnly = (newFormat[eGST_MEDIATYPE_AUDIO] == GST_FORMAT_INVALID); MW_LOG_INFO("Setting single-path-stream to %d", videoOnly); g_object_set(vidsink, "single-path-stream", videoOnly, NULL); // RDKEMW-18286: Reinforce show-video-window before pipeline state change @@ -558,7 +588,7 @@ void InterfacePlayerRDK::ConfigurePipeline(PipelineCodecInfo&& codecInfo,int for } } /* If buffering is enabled, set the pipeline in Paused state, once sufficient content has been buffered the pipeline will be set to GST_STATE_PLAYING */ - else if (interfacePlayerPriv->gstPrivateContext->buffering_enabled && format != GST_FORMAT_INVALID && GST_NORMAL_PLAY_RATE == interfacePlayerPriv->gstPrivateContext->rate) + else if (interfacePlayerPriv->gstPrivateContext->buffering_enabled && newFormat[eGST_MEDIATYPE_VIDEO] != GST_FORMAT_INVALID && GST_NORMAL_PLAY_RATE == interfacePlayerPriv->gstPrivateContext->rate) { MW_LOG_INFO("Setting state to GST_STATE_PAUSED, target state to GST_STATE_PLAYING"); interfacePlayerPriv->gstPrivateContext->buffering_target_state = GST_STATE_PLAYING; @@ -1425,6 +1455,7 @@ void InterfacePlayerRDK::TearDownStream(int type) interfacePlayerPriv->gstPrivateContext->decoderHandleNotified = false; } stream->format = GST_FORMAT_INVALID; + stream->codecInfo = MediaCodecInfo{}; if (mediaType == eGST_MEDIATYPE_SUBTITLE) { interfacePlayerPriv->gstPrivateContext->usingClosedCaptionsControl = false; @@ -1949,12 +1980,15 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void * } else { + // Initial source caps intentionally use only the manifest-derived stream format and encryption flag. + // Codec private data, dimensions, channels, and sample rate are parsed from the MP4 init segment and + // applied through SetStreamCaps() before elementary media samples are injected. caps = GetCaps(static_cast(stream->format)); } if (caps != NULL) { - if (stream->codecInfo.mIsEncrypted) + if (stream->codecInfo.mIsEncrypted && IsElementaryStreamFormat(static_cast(stream->format))) { MW_LOG_DEBUG("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format); TransformToEncryptedCaps(caps, mDrmSystem); @@ -5622,6 +5656,7 @@ void InterfacePlayerRDK::SetStreamCaps(GstMediaType type, MediaCodecInfo&& codec { MW_LOG_ERR("Failed to get caps for type[%d] format[%d]", type, codecInfo.mCodecFormat); } + stream->codecInfo = std::move(codecInfo); } /** diff --git a/InterfacePlayerRDK.h b/InterfacePlayerRDK.h index ba8067a7..2b125fa0 100644 --- a/InterfacePlayerRDK.h +++ b/InterfacePlayerRDK.h @@ -335,7 +335,7 @@ class InterfacePlayerRDK *@param[in] drmID preferred drm */ void SetPreferredDRM(const char *drmID); - /** + /** * @brief Configures the GStreamer pipeline. * @param format Video format. * @param audioFormat Audio format. @@ -352,7 +352,24 @@ class InterfacePlayerRDK * @param enableLiveLatency Whether to enable live-latency mode in the * RialtoSink streams-info context. Defaults to false. */ - void ConfigurePipeline(PipelineCodecInfo&& codecInfo, int format, int audioFormat, int subFormat, bool bESChangeStatus, bool setReadyAfterPipelineCreation, bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string url, bool enableLiveLatency = false); + void ConfigurePipeline(int, int, int, bool, bool, bool, int32_t, gint, const char *, int, bool, std::string url, bool enableLiveLatency = false); + + /** + * @brief Configures the GStreamer pipeline. + * @param codecInfo Codec information for the stream. + * @param bESChangeStatus Whether ES change status is enabled. + * @param setReadyAfterPipelineCreation Whether to set the player as ready after pipeline creation. + * @param isSubEnable Whether subtitles are enabled. + * @param trackId Track ID. + * @param rate Bitrate. + * @param pipelineName Pipeline name. + * @param PipelinePriority Pipeline priority. + * @param FirstFrameFlag Whether the first-frame callback is required. + * @param url URL of the manifest used to configure stream setup. + * @param enableLiveLatency Whether to enable live-latency mode in the + * RialtoSink streams-info context. Defaults to false. + */ + void ConfigurePipeline(StreamCodecInfo&& codecInfo, bool bESChangeStatus, bool setReadyAfterPipelineCreation, bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string url, bool enableLiveLatency = false); /** * @brief Enables or disables pausing on playback start. diff --git a/test/pi-cli/commandProcessing.cpp b/test/pi-cli/commandProcessing.cpp index 8526a382..32ee3344 100644 --- a/test/pi-cli/commandProcessing.cpp +++ b/test/pi-cli/commandProcessing.cpp @@ -206,7 +206,7 @@ void configurePipelineCommand(InterfacePlayerRDK& player, const std::vector(format)), MediaCodecInfo(static_cast(audioFormat)), MediaCodecInfo(static_cast(subFormat))}, diff --git a/test/utests/tests/GstPlayer/FunctionalTests.cpp b/test/utests/tests/GstPlayer/FunctionalTests.cpp index fa2c33ea..5e5a07b8 100644 --- a/test/utests/tests/GstPlayer/FunctionalTests.cpp +++ b/test/utests/tests/GstPlayer/FunctionalTests.cpp @@ -253,12 +253,9 @@ class GstPlayerTests : public ::testing::Test EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_element_pipeline, GST_STATE_PLAYING)) .WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_TTML}, - GST_FORMAT_VIDEO_ES_H264, - GST_FORMAT_AUDIO_ES_AAC, - GST_FORMAT_SUBTITLE_TTML, setup->bESChangeStatus, setup->setReadyAfterPipelineCreation, true, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); diff --git a/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp b/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp index 70b9a966..5848507e 100644 --- a/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp +++ b/test/utests/tests/GstPlayer/PauseOnPlaybackTests.cpp @@ -138,7 +138,7 @@ TEST_F(PauseOnPlaybackTests, EnteredPausedSteHandler_ConfigurePauseOnPlayback) // Enable PauseOnPlayback mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); } // Test configuration of pipeline with PauseOnPlayback not enabled @@ -173,7 +173,7 @@ TEST_F(PauseOnPlaybackTests, EnteredPausedSteHandler_ConfigureNormalPlayback) EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_element_pipeline, GST_STATE_PLAYING)) .WillOnce(Return(GST_STATE_CHANGE_SUCCESS)); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); } // Test bus_message callback when PauseOnPlayback has been enabled, and sink @@ -230,7 +230,7 @@ TEST_F(PauseOnPlaybackTests, bus_messsage_FrameStepPropertyAvailable) mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); ASSERT_TRUE(bus_sync_func != nullptr); ASSERT_TRUE(bus_message_func != nullptr); @@ -330,7 +330,7 @@ TEST_F(PauseOnPlaybackTests, bus_message_FrameStepPropertyNotAvailable) mInterfaceGstPlayer->SetPauseOnStartPlayback(true); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_SUBTITLE_WEBVTT}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); ASSERT_TRUE(bus_sync_func != nullptr); ASSERT_TRUE(bus_message_func != nullptr); diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index b8e0180b..c7d5ec9b 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -129,7 +129,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithWesterosAndRealtoSink) mPlayerConfigParams->useRialtoSink = true; EXPECT_EQ(mPlayerContext->usingRialtoSink, false); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->using_westerossink, true); EXPECT_EQ(mPlayerContext->usingRialtoSink, true); @@ -138,18 +138,19 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithWesterosAndRealtoSink) TEST_F(InterfacePlayerTests, ConfigurePipeline_WithSubtitlesEnabled) { g_mockGStreamer = nullptr; - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, true, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, false, false, true, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_SUBTITLE].format, GST_FORMAT_INVALID); } TEST_F(InterfacePlayerTests, ConfigurePipeline_WithEncryptedCodecInfo) { + delete g_mockGStreamer; g_mockGStreamer = nullptr; MediaCodecInfo videoCodecInfo(GST_FORMAT_VIDEO_ES_H264); videoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); } @@ -157,12 +158,13 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithEncryptedCodecInfo) // Validate that an encryption-only change does not configure or count a track when its format is invalid. TEST_F(InterfacePlayerTests, ConfigurePipeline_IgnoresEncryptionChangeForInvalidFormat) { + delete g_mockGStreamer; g_mockGStreamer = nullptr; mPlayerContext->NumberOfTracks = 0; MediaCodecInfo videoCodecInfo(GST_FORMAT_INVALID); videoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(videoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); @@ -174,7 +176,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithBufferingEnabled) mPlayerContext->buffering_enabled = true; mPlayerContext->rate = GST_NORMAL_PLAY_RATE; - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_MPEGTS, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->buffering_in_progress, true); EXPECT_EQ(mPlayerContext->buffering_target_state, GST_STATE_PLAYING); @@ -189,7 +191,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_StreamConfiguration) EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->NumberOfTracks, 2); EXPECT_EQ(cbResponse, 5); //callback was called @@ -204,7 +206,7 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_ESChange) EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - mInterfaceGstPlayer->ConfigurePipeline(PipelineCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4, true, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_ISO_BMFF, GST_FORMAT_AUDIO_ES_AC3, GST_FORMAT_SUBTITLE_MP4}, true, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); EXPECT_EQ(cbResponse, 5); @@ -341,6 +343,9 @@ TEST_F(InterfacePlayerTests, TearDownStreamTest_successvideo) gst_media_stream* stream = &mPlayerContext->stream[eGST_MEDIATYPE_VIDEO]; stream->format = GST_FORMAT_VIDEO_ES_H264; + stream->codecInfo.mCodecFormat = GST_FORMAT_VIDEO_ES_H264; + stream->codecInfo.mCodecData = {0x01, 0x02}; + stream->codecInfo.mIsEncrypted = true; stream->eosReached = true; stream->bufferUnderrun = true; @@ -357,6 +362,9 @@ TEST_F(InterfacePlayerTests, TearDownStreamTest_successvideo) mInterfaceGstPlayer->TearDownStream(eGST_MEDIATYPE_VIDEO); EXPECT_EQ(stream->format, GST_FORMAT_INVALID); + EXPECT_EQ(stream->codecInfo.mCodecFormat, GST_FORMAT_INVALID); + EXPECT_TRUE(stream->codecInfo.mCodecData.empty()); + EXPECT_FALSE(stream->codecInfo.mIsEncrypted); EXPECT_EQ(stream->bufferUnderrun, false); EXPECT_EQ(stream->eosReached, false); EXPECT_EQ(mPlayerContext->buffering_in_progress, false); @@ -394,7 +402,11 @@ TEST_F(InterfacePlayerTests, TearDownStreamTest_misc) mInterfaceGstPlayer->TearDownStream(eGST_MEDIATYPE_AUDIO); stream = &mPlayerContext->stream[eGST_MEDIATYPE_SUBTITLE]; stream->format = GST_FORMAT_VIDEO_ES_H264; //dummy value + stream->codecInfo.mCodecFormat = GST_FORMAT_VIDEO_ES_H264; + stream->codecInfo.mIsEncrypted = true; mInterfaceGstPlayer->TearDownStream(eGST_MEDIATYPE_SUBTITLE); + EXPECT_EQ(stream->codecInfo.mCodecFormat, GST_FORMAT_INVALID); + EXPECT_FALSE(stream->codecInfo.mIsEncrypted); } TEST_F(InterfacePlayerTests, GstStopTestTrue) @@ -850,8 +862,32 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream g_mockGstUtils = nullptr; } -// Validate that encrypted elementary stream caps retain the original media type without adding a missing DRM system. -TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsWithoutDrmSystem) +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedContainerCapsRemainUnchanged) +{ + g_mockGstUtils = new StrictMock(); + + void* playerInstance = mInterfaceGstPlayer; + void* source = reinterpret_cast(0x1234); + GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; + GstCaps caps = {}; + gst_media_stream* stream = &mPlayerContext->stream[mediaType]; + stream->format = GST_FORMAT_MPEGTS; + stream->codecInfo = MediaCodecInfo(stream->format); + stream->codecInfo.mIsEncrypted = true; + mPlayerConfigParams->videoBufBytes = 500; + mInterfaceGstPlayer->SetPreferredDRM("urn:uuid:edef8ba9-79d6-4ace-a3c8-27dcd51d21ed"); + + EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_MPEGTS)).WillOnce(Return(&caps)); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(_, _)).Times(0); + + mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); + + delete g_mockGstUtils; + g_mockGstUtils = nullptr; +} + +// Validate that encrypted elementary stream caps remain unchanged when no DRM system is configured. +TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStreamCapsWithoutDrmSystemRemainUnchanged) { g_mockGstUtils = new StrictMock(); @@ -859,7 +895,6 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream void* source = reinterpret_cast(0x1234); GstMediaType mediaType = eGST_MEDIATYPE_VIDEO; GstCaps caps = {}; - GstStructure structure = {}; gst_media_stream* stream = &mPlayerContext->stream[mediaType]; stream->format = GST_FORMAT_VIDEO_ES_H264; stream->codecInfo = MediaCodecInfo(stream->format); @@ -867,9 +902,7 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_EncryptedElementaryStream mPlayerConfigParams->videoBufBytes = 500; EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_VIDEO_ES_H264)).WillOnce(Return(&caps)); - EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(&caps, 0)).WillOnce(Return(&structure)); - EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("original-media-type"))).Times(1); - EXPECT_CALL(*g_mockGStreamer, gst_structure_set(&structure, StrEq("protection-system"))).Times(0); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(_, _)).Times(0); mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); @@ -897,6 +930,7 @@ TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_ClearElementaryStreamCaps mInterfaceGstPlayer->InitializeSourceForPlayer(playerInstance, source, mediaType); delete g_mockGstUtils; + g_mockGstUtils = nullptr; } TEST_F(InterfacePlayerTests, InitializeSourceForPlayer_Audio_CapsNull) @@ -3208,6 +3242,22 @@ TEST_F(InterfacePlayerTests, SetStreamCaps_ValidVideoCodecFormat) delete g_mockGstUtils; } +TEST_F(InterfacePlayerTests, SetStreamCaps_EncryptedContainerCapsRemainUnchanged) +{ + g_mockGstUtils = new StrictMock(); + MediaCodecInfo codecInfo; + codecInfo.mCodecFormat = GST_FORMAT_ISO_BMFF; + codecInfo.mIsEncrypted = true; + GstCaps caps; + EXPECT_CALL(*g_mockGstUtils, GetCaps(GST_FORMAT_ISO_BMFF)).WillOnce(Return(&caps)); + EXPECT_CALL(*g_mockGStreamer, gst_caps_get_structure(_, _)).Times(0); + EXPECT_CALL(*g_mockGStreamer, gst_app_src_set_caps(_, &caps)).Times(1); + + mInterfaceGstPlayer->SetStreamCaps(eGST_MEDIATYPE_VIDEO, std::move(codecInfo)); + + delete g_mockGstUtils; +} + /** * SetStreamCaps Tests with encrypted audio codec format */ From b64fb7b566d49a13d10ec014af253170c7b4db43 Mon Sep 17 00:00:00 2001 From: Philip Stroffolino Date: Fri, 2 Oct 2026 20:26:58 -0400 Subject: [PATCH 21/23] Fix MP4 demux unknown-format stream setup --- InterfacePlayerRDK.cpp | 11 +- .../InterfacePlayerFunctionTests.cpp | 164 ++++++++++++++++++ 2 files changed, 171 insertions(+), 4 deletions(-) diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index f81fdc16..cb599507 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -455,12 +455,15 @@ void InterfacePlayerRDK::ConfigurePipeline(StreamCodecInfo&& codecInfo, for (int i = 0; i < GST_TRACK_COUNT; i++) { gst_media_stream *stream = &interfacePlayerPriv->gstPrivateContext->stream[i]; - bool isInitialSetup = (stream->format == GST_FORMAT_INVALID || stream->format == GST_FORMAT_UNKNOWN); + bool isInitialSetup = (stream->format == GST_FORMAT_INVALID && newFormat[i] == GST_FORMAT_UNKNOWN); bool isValidNewFormat = (newFormat[i] != GST_FORMAT_INVALID && newFormat[i] != GST_FORMAT_UNKNOWN); bool isEncryptionChanged = (stream->codecInfo.mIsEncrypted != codecInfoByTrack[i]->mIsEncrypted); - bool isFormatChanged = (stream->format != newFormat[i] || isEncryptionChanged); - // Reconfigure pipeline if this is the first setup, or the encryption status or format has changed - bool shouldReconfigure = isValidNewFormat && (isInitialSetup || isFormatChanged); + // The initial-setup case must be honoured even for an unknown format (e.g. muxed HLS/TS, or audio + // remapped to the video slot) because the real format is only discovered from the demuxer later - + // without a source element the injector never unblocks. An encryption-only change is only + // actionable once the format is usable. + bool shouldReconfigure = (stream->format != newFormat[i] && (isValidNewFormat || isInitialSetup)) + || (isEncryptionChanged && isValidNewFormat); if(shouldReconfigure) { MW_LOG_MIL("Closing stream %d old format = %d, new format = %d",i, stream->format, newFormat[i]); diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index c7d5ec9b..098a8cce 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -155,6 +155,155 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithEncryptedCodecInfo) EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); } +/* + * shouldReconfigure truth table (all 10 reachable rows are covered below) + * + * Formula: + * shouldReconfigure = (F && (V || I)) || (E && V) + * + * Symbols: + * F = format changed (stream->format != newFormat) + * I = initial UNKNOWN setup only (stream INVALID and newFormat UNKNOWN) + * V = valid new format (newFormat is neither INVALID nor UNKNOWN) + * E = encryption changed + * + * Reachable rows and expected result: + * +------+---+---+---+---+-------------------+ + * | Row | F | I | V | E | shouldReconfigure | + * +------+---+---+---+---+-------------------+ + * | R1 | 1 | 1 | 0 | 0 | 1 | + * | R2 | 1 | 1 | 0 | 1 | 1 | + * | R3 | 1 | 0 | 1 | 0 | 1 | + * | R4 | 1 | 0 | 1 | 1 | 1 | + * | R5 | 1 | 0 | 0 | 0 | 0 | + * | R6 | 1 | 0 | 0 | 1 | 0 | + * | R7 | 0 | 0 | 1 | 0 | 0 | + * | R8 | 0 | 0 | 1 | 1 | 1 | + * | R9 | 0 | 0 | 0 | 0 | 0 | + * | R10 | 0 | 0 | 0 | 1 | 0 | + * +------+---+---+---+---+-------------------+ + * + * Unreachable by definition: + * - I=1 with F=0 (INVALID cannot equal UNKNOWN) + * - I=1 with V=1 (UNKNOWN is not a valid format) + */ +TEST_F(InterfacePlayerTests, ConfigurePipeline_ShouldReconfigureDecisionMatrix) +{ + g_mockGStreamer = nullptr; + + // R1: F=1, I=1, V=0, E=0 -> shouldReconfigure=true. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_INVALID; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); + + // R2: F=1, I=1, V=0, E=1 -> shouldReconfigure=true. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_INVALID; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + MediaCodecInfo unknownEncryptedVideoCodecInfo(GST_FORMAT_UNKNOWN); + unknownEncryptedVideoCodecInfo.mIsEncrypted = true; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(unknownEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); + EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R3: F=1, I=0, V=1, E=0 -> shouldReconfigure=true. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_HEVC, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_HEVC); + + // R4: F=1, I=0, V=1, E=1 -> shouldReconfigure=true. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + MediaCodecInfo validChangedEncryptedVideoCodecInfo(GST_FORMAT_VIDEO_ES_HEVC); + validChangedEncryptedVideoCodecInfo.mIsEncrypted = true; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(validChangedEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_HEVC); + EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R5: F=1, I=0, V=0, E=0 -> shouldReconfigure=false. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_H264); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // Additional R5 transition check: UNKNOWN -> INVALID should also stay not reconfigured. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); + + // R6: F=1, I=0, V=0, E=1 -> shouldReconfigure=false. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + MediaCodecInfo invalidEncryptedVideoCodecInfo(GST_FORMAT_INVALID); + invalidEncryptedVideoCodecInfo.mIsEncrypted = true; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(invalidEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R7: F=0, I=0, V=1, E=0 -> shouldReconfigure=false. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R8: F=0, I=0, V=1, E=1 -> shouldReconfigure=true. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + MediaCodecInfo encryptedVideoCodecInfo(GST_FORMAT_VIDEO_ES_H264); + encryptedVideoCodecInfo.mIsEncrypted = true; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(encryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); + EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R9: F=0, I=0, V=0, E=0 -> shouldReconfigure=false. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); + + // R10: F=0, I=0, V=0, E=1 -> shouldReconfigure=false. + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; + MediaCodecInfo unknownEncryptedNoFormatChangeCodecInfo(GST_FORMAT_UNKNOWN); + unknownEncryptedNoFormatChangeCodecInfo.mIsEncrypted = true; + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(unknownEncryptedNoFormatChangeCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); + EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); +} + // Validate that an encryption-only change does not configure or count a track when its format is invalid. TEST_F(InterfacePlayerTests, ConfigurePipeline_IgnoresEncryptionChangeForInvalidFormat) { @@ -170,6 +319,21 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_IgnoresEncryptionChangeForInvalid EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); } +// A manifest may not reveal the video format (muxed HLS/TS, or audio remapped to the video slot). +// The track must still be set up on first configuration so the source exists and injection can start; +// the real caps arrive later from the demuxer via SetStreamCaps(). +TEST_F(InterfacePlayerTests, ConfigurePipeline_ConfiguresTrackWithUnknownFormatOnInitialSetup) +{ + g_mockGStreamer = nullptr; + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->rate = 1.0; + + mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_AUDIO_ES_AAC, GST_FORMAT_INVALID}, false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + + EXPECT_EQ(mPlayerContext->NumberOfTracks, 2); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); +} + TEST_F(InterfacePlayerTests, ConfigurePipeline_WithBufferingEnabled) { g_mockGStreamer = nullptr; From 3b531f2a97fb75567ac6913def628472de42cb8f Mon Sep 17 00:00:00 2001 From: Vinish100 Date: Thu, 8 Oct 2026 22:35:49 +0530 Subject: [PATCH 22/23] Optimize L1 for MP4 demux unknown-format stream setup --- .../InterfacePlayerFunctionTests.cpp | 354 ++++++++++-------- 1 file changed, 205 insertions(+), 149 deletions(-) diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index 098a8cce..db362abc 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -155,155 +155,6 @@ TEST_F(InterfacePlayerTests, ConfigurePipeline_WithEncryptedCodecInfo) EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); } -/* - * shouldReconfigure truth table (all 10 reachable rows are covered below) - * - * Formula: - * shouldReconfigure = (F && (V || I)) || (E && V) - * - * Symbols: - * F = format changed (stream->format != newFormat) - * I = initial UNKNOWN setup only (stream INVALID and newFormat UNKNOWN) - * V = valid new format (newFormat is neither INVALID nor UNKNOWN) - * E = encryption changed - * - * Reachable rows and expected result: - * +------+---+---+---+---+-------------------+ - * | Row | F | I | V | E | shouldReconfigure | - * +------+---+---+---+---+-------------------+ - * | R1 | 1 | 1 | 0 | 0 | 1 | - * | R2 | 1 | 1 | 0 | 1 | 1 | - * | R3 | 1 | 0 | 1 | 0 | 1 | - * | R4 | 1 | 0 | 1 | 1 | 1 | - * | R5 | 1 | 0 | 0 | 0 | 0 | - * | R6 | 1 | 0 | 0 | 1 | 0 | - * | R7 | 0 | 0 | 1 | 0 | 0 | - * | R8 | 0 | 0 | 1 | 1 | 1 | - * | R9 | 0 | 0 | 0 | 0 | 0 | - * | R10 | 0 | 0 | 0 | 1 | 0 | - * +------+---+---+---+---+-------------------+ - * - * Unreachable by definition: - * - I=1 with F=0 (INVALID cannot equal UNKNOWN) - * - I=1 with V=1 (UNKNOWN is not a valid format) - */ -TEST_F(InterfacePlayerTests, ConfigurePipeline_ShouldReconfigureDecisionMatrix) -{ - g_mockGStreamer = nullptr; - - // R1: F=1, I=1, V=0, E=0 -> shouldReconfigure=true. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_INVALID; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); - - // R2: F=1, I=1, V=0, E=1 -> shouldReconfigure=true. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_INVALID; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - MediaCodecInfo unknownEncryptedVideoCodecInfo(GST_FORMAT_UNKNOWN); - unknownEncryptedVideoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(unknownEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); - EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R3: F=1, I=0, V=1, E=0 -> shouldReconfigure=true. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_HEVC, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_HEVC); - - // R4: F=1, I=0, V=1, E=1 -> shouldReconfigure=true. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - MediaCodecInfo validChangedEncryptedVideoCodecInfo(GST_FORMAT_VIDEO_ES_HEVC); - validChangedEncryptedVideoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(validChangedEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_HEVC); - EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R5: F=1, I=0, V=0, E=0 -> shouldReconfigure=false. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_VIDEO_ES_H264); - EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // Additional R5 transition check: UNKNOWN -> INVALID should also stay not reconfigured. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_INVALID, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); - - // R6: F=1, I=0, V=0, E=1 -> shouldReconfigure=false. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - MediaCodecInfo invalidEncryptedVideoCodecInfo(GST_FORMAT_INVALID); - invalidEncryptedVideoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(invalidEncryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R7: F=0, I=0, V=1, E=0 -> shouldReconfigure=false. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_VIDEO_ES_H264, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R8: F=0, I=0, V=1, E=1 -> shouldReconfigure=true. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_VIDEO_ES_H264; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - MediaCodecInfo encryptedVideoCodecInfo(GST_FORMAT_VIDEO_ES_H264); - encryptedVideoCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(encryptedVideoCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 1); - EXPECT_TRUE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R9: F=0, I=0, V=0, E=0 -> shouldReconfigure=false. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{GST_FORMAT_UNKNOWN, GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, GST_FORMAT_UNKNOWN); - EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); - - // R10: F=0, I=0, V=0, E=1 -> shouldReconfigure=false. - mPlayerContext->NumberOfTracks = 0; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = GST_FORMAT_UNKNOWN; - mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = false; - MediaCodecInfo unknownEncryptedNoFormatChangeCodecInfo(GST_FORMAT_UNKNOWN); - unknownEncryptedNoFormatChangeCodecInfo.mIsEncrypted = true; - mInterfaceGstPlayer->ConfigurePipeline(StreamCodecInfo{std::move(unknownEncryptedNoFormatChangeCodecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, - false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); - EXPECT_EQ(mPlayerContext->NumberOfTracks, 0); - EXPECT_FALSE(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted); -} - // Validate that an encryption-only change does not configure or count a track when its format is invalid. TEST_F(InterfacePlayerTests, ConfigurePipeline_IgnoresEncryptionChangeForInvalidFormat) { @@ -3451,3 +3302,208 @@ TEST_F(InterfacePlayerTests, SetStreamCaps_EncryptedAudioCodecFormat) delete g_mockGstUtils; } +/* + * shouldReconfigure truth table (all 10 reachable rows are covered below) + * + * Formula: + * shouldReconfigure = (F && (V || I)) || (E && V) + * + * Symbols: + * F = format changed (stream->format != newFormat) + * I = initial UNKNOWN setup only (stream INVALID and newFormat UNKNOWN) + * V = valid new format (newFormat is neither INVALID nor UNKNOWN) + * E = encryption changed + * + * Reachable rows and expected result: + * +------+---+---+---+---+-------------------+ + * | Row | F | I | V | E | shouldReconfigure | + * +------+---+---+---+---+-------------------+ + * | R1 | 1 | 1 | 0 | 0 | 1 | + * | R2 | 1 | 1 | 0 | 1 | 1 | + * | R3 | 1 | 0 | 1 | 0 | 1 | + * | R4 | 1 | 0 | 1 | 1 | 1 | + * | R5 | 1 | 0 | 0 | 0 | 0 | + * | R6 | 1 | 0 | 0 | 1 | 0 | + * | R7 | 0 | 0 | 1 | 0 | 0 | + * | R8 | 0 | 0 | 1 | 1 | 1 | + * | R9 | 0 | 0 | 0 | 0 | 0 | + * | R10 | 0 | 0 | 0 | 1 | 0 | + * +------+---+---+---+---+-------------------+ + * + * Unreachable by definition: + * - I=1 with F=0 (INVALID cannot equal UNKNOWN) + * - I=1 with V=1 (UNKNOWN is not a valid format) + */ +// Define test parameters for the decision matrix +struct ShouldReconfigureTestParams { + std::string testName; // e.g., "R1", "R2", etc. + GstStreamOutputFormat initialFormat; + GstStreamOutputFormat newFormat; + bool initiallyEncrypted; + bool newEncrypted; + int expectedTrackCount; + GstStreamOutputFormat expectedFinalFormat; + bool expectedEncrypted; +}; + +class ConfigurePipelineShouldReconfigureTest : public InterfacePlayerTests, + public ::testing::WithParamInterface { +}; + +INSTANTIATE_TEST_SUITE_P( + DecisionMatrix, + ConfigurePipelineShouldReconfigureTest, + ::testing::Values( + // R1: F=1, I=1, V=0, E=0 -> shouldReconfigure=true + ShouldReconfigureTestParams{ + "R1_FormatChangeInvalidToUnknown_NoEncryption", + GST_FORMAT_INVALID, + GST_FORMAT_UNKNOWN, + false, + false, + 1, + GST_FORMAT_UNKNOWN, + false + }, + // R2: F=1, I=1, V=0, E=1 -> shouldReconfigure=true + ShouldReconfigureTestParams{ + "R2_FormatChangeInvalidToUnknown_WithEncryption", + GST_FORMAT_INVALID, + GST_FORMAT_UNKNOWN, + false, + true, + 1, + GST_FORMAT_UNKNOWN, + true + }, + // R3: F=1, I=0, V=1, E=0 -> shouldReconfigure=true + ShouldReconfigureTestParams{ + "R3_ValidFormatChange_NoEncryption", + GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_VIDEO_ES_HEVC, + false, + false, + 1, + GST_FORMAT_VIDEO_ES_HEVC, + false + }, + // R4: F=1, I=0, V=1, E=1 -> shouldReconfigure=true + ShouldReconfigureTestParams{ + "R4_ValidFormatChange_WithEncryption", + GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_VIDEO_ES_HEVC, + false, + true, + 1, + GST_FORMAT_VIDEO_ES_HEVC, + true + }, + // R5: F=1, I=0, V=0, E=0 -> shouldReconfigure=false + ShouldReconfigureTestParams{ + "R5_NoValidFormatChange_NoEncryption", + GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_UNKNOWN, + false, + false, + 0, + GST_FORMAT_VIDEO_ES_H264, + false + }, + // R5 additional: UNKNOWN -> INVALID transition + ShouldReconfigureTestParams{ + "R5_UnknownToInvalid_NoEncryption", + GST_FORMAT_UNKNOWN, + GST_FORMAT_INVALID, + false, + false, + 0, + GST_FORMAT_UNKNOWN, + false + }, + // R6: F=1, I=0, V=0, E=1 -> shouldReconfigure=false + ShouldReconfigureTestParams{ + "R6_NoValidFormatChange_WithEncryption", + GST_FORMAT_UNKNOWN, + GST_FORMAT_INVALID, + false, + true, + 0, + GST_FORMAT_UNKNOWN, + false + }, + // R7: F=0, I=0, V=1, E=0 -> shouldReconfigure=false + ShouldReconfigureTestParams{ + "R7_NoFormatChange_SameFormat_NoEncryption", + GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_VIDEO_ES_H264, + false, + false, + 0, + GST_FORMAT_VIDEO_ES_H264, + false + }, + // R8: F=0, I=0, V=1, E=1 -> shouldReconfigure=true + ShouldReconfigureTestParams{ + "R8_NoFormatChange_EncryptionChange", + GST_FORMAT_VIDEO_ES_H264, + GST_FORMAT_VIDEO_ES_H264, + false, + true, + 1, + GST_FORMAT_VIDEO_ES_H264, + true + }, + // R9: F=0, I=0, V=0, E=0 -> shouldReconfigure=false + ShouldReconfigureTestParams{ + "R9_NoFormatChange_UnknownFormat_NoEncryption", + GST_FORMAT_UNKNOWN, + GST_FORMAT_UNKNOWN, + false, + false, + 0, + GST_FORMAT_UNKNOWN, + false + }, + // R10: F=0, I=0, V=0, E=1 -> shouldReconfigure=false + ShouldReconfigureTestParams{ + "R10_NoFormatChange_UnknownFormat_EncryptionChange", + GST_FORMAT_UNKNOWN, + GST_FORMAT_UNKNOWN, + false, + true, + 0, + GST_FORMAT_UNKNOWN, + false + } + ), + [](const ::testing::TestParamInfo& info) { + return info.param.testName; + } +); + +TEST_P(ConfigurePipelineShouldReconfigureTest, ConfigurePipeline_ShouldReconfigureDecisionMatrix) +{ + auto param = GetParam(); + g_mockGStreamer = nullptr; + + mPlayerContext->NumberOfTracks = 0; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format = param.initialFormat; + mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted = param.initiallyEncrypted; + + // Build the codec info based on parameters + MediaCodecInfo codecInfo; + codecInfo.mCodecFormat = param.newFormat; + codecInfo.mIsEncrypted = param.newEncrypted; + + mInterfaceGstPlayer->ConfigurePipeline( + StreamCodecInfo{std::move(codecInfo), GST_FORMAT_INVALID, GST_FORMAT_INVALID}, + false, false, false, 0, GST_NORMAL_PLAY_RATE, "testPipeline", 0, false, "testManifest", false); + + EXPECT_EQ(mPlayerContext->NumberOfTracks, param.expectedTrackCount); + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].format, param.expectedFinalFormat); + if (param.expectedTrackCount > 0) { + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted, param.expectedEncrypted); + } else { + EXPECT_EQ(mPlayerContext->stream[eGST_MEDIATYPE_VIDEO].codecInfo.mIsEncrypted, param.initiallyEncrypted); + } +} From 661984f0d002f56b9c6ed71079ac9d0d18e1429f Mon Sep 17 00:00:00 2001 From: rekhap2kandhavelan Date: Wed, 21 Oct 2026 14:59:51 +0530 Subject: [PATCH 23/23] RDKEMW-27111 : [PLI][Develop] [RTK Rogers IUI V2] Audio Loss (#301) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reason for change: EOS only fires when both audio and video have resumed data flow for a normal discontinuity — never for just one side Test Procedure: Refere ticket Risks: low Priority: P1 Co-authored-by: rekha kandhavelan --- InterfacePlayerRDK.cpp | 44 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index cb599507..919a8073 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -3786,10 +3786,46 @@ bool InterfacePlayerRDK::CheckDiscontinuity(int mediaType, int streamFormat , bo { MW_LOG_WARN("PTS-RESTAMP ENABLED, but we have codec change, so Signal EOS (%s).",gstGetMediaTypeName(type)); } - GstPlayer_SignalEOS(stream); - // We are in buffering, but we received discontinuity, un-pause pipeline - shouldHaltBuffering = true; - ret = true; + + // For a muxed audio/video discontinuity, only trigger EOS once BOTH tracks have + // resumed data flow (firstBufferProcessed) after the reset. Signalling EOS for + // only one side leaves its sibling's appsrc out of sync - causing audio loss / + // video freeze. Codec-change always proceeds immediately since the decoder swap + // can't wait on the sibling track's buffer flow. + bool proceedWithEos = true; + GstMediaType siblingType = (type == eGST_MEDIATYPE_AUDIO) ? eGST_MEDIATYPE_VIDEO : + (type == eGST_MEDIATYPE_VIDEO) ? eGST_MEDIATYPE_AUDIO : eGST_MEDIATYPE_DEFAULT; + if (!codecChange && siblingType != eGST_MEDIATYPE_DEFAULT) + { + gst_media_stream *siblingStream = &interfacePlayerPriv->gstPrivateContext->stream[siblingType]; + if (siblingStream->format != GST_FORMAT_INVALID && !siblingStream->firstBufferProcessed) + { + proceedWithEos = false; + } + } + + if (proceedWithEos) + { + GstPlayer_SignalEOS(stream); + // We are in buffering, but we received discontinuity, un-pause pipeline + shouldHaltBuffering = true; + ret = true; + + //If we have an audio discontinuity, signal subtec as well + if ((type == eGST_MEDIATYPE_AUDIO) && (interfacePlayerPriv->gstPrivateContext->stream[eGST_MEDIATYPE_SUBTITLE].source)) + { + GstPlayer_SignalEOS(interfacePlayerPriv->gstPrivateContext->stream[eGST_MEDIATYPE_SUBTITLE]); + } + else + { + MW_LOG_WARN("Skipped EOS %s", gstGetMediaTypeName(type)); + } + } + else + { + MW_LOG_WARN("Skipping appsrc EOS for %s discontinuity - sibling track not yet ready (firstBufferProcessed=0)", gstGetMediaTypeName(type)); + } + //If we have an audio discontinuity, signal subtec as well if ((type == eGST_MEDIATYPE_AUDIO) && (interfacePlayerPriv->gstPrivateContext->stream[eGST_MEDIATYPE_SUBTITLE].source))