Feature/vplay 12250 vpaamp 966 a

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.
  */

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.

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<IDirectRialtoCC *>(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<IDirectRialtoCC *>(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,39 +139,42 @@ int PlayerDirectRialtoCCManager::SetTrack(
 
 	MW_LOG_INFO("track=\"%s\" format=%d", track.c_str(), static_cast<int>(format));
 
-	if (m_control == nullptr)
+	IDirectRialtoCC *control = m_control.load();
+	if (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);
+	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");
 }
 

closedcaptions/direct-rialto/PlayerDirectRialtoCCManager.h

@@ -44,6 +44,7 @@
 #include "PlayerCCManager.h"
 #include "IDirectRialtoCC.h"
 
+#include <atomic>
 #include <mutex>
 #include <set>
 
@@ -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<IDirectRialtoCC *> m_control{nullptr};
 
 	/// Guards mId / mIdSet.
 	std::mutex m_idLock;

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
 	{

closedcaptions/rialto/PlayerRialtoCCManager.h

@@ -31,6 +31,7 @@
 
 #include <string>
 #include <set>
+#include <atomic>
 #include <mutex>
 
 /**
@@ -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<void *> mSubtitleControlHandle{nullptr};
 
 	std::mutex mIdLock{};
 	int mId{0};

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})