From 7da2bda30239a411681d6591a7e780fed4251f01 Mon Sep 17 00:00:00 2001 From: Jose Fagundez Date: Mon, 3 Aug 2026 13:48:12 +0000 Subject: [PATCH 1/8] 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 2/8] 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 3/8] 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 4/8] 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 5/8] 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 6/8] 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 7/8] 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 8/8] 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})