RDKEMW- 26814:Code improvement for Player interface library

InterfacePlayerPriv.h

@@ -203,6 +203,8 @@ struct GstPlayerPriv
 	bool usingRialtoSink;                                                            /**< true if rialto sink is used for video and audio sinks */
 	bool usingClosedCaptionsControl;                                                 /**< true if subtitle sink being used for CC control */
 	char videoRectangle[VIDEO_COORDINATES_SIZE];
+	int videoRectangleWidth;                                                             /**< Cached width parsed from videoRectangle; 0 when unset/invalid */
+	int videoRectangleHeight;                                                            /**< Cached height parsed from videoRectangle; 0 when unset/invalid */
 	bool pauseOnStartPlayback;                                                               /**< true if should start playback paused */
 	std::atomic<bool> eosSignalled;                                                  /**< Indicates if EOS has signaled */
 	gboolean buffering_enabled;                                                              /**< enable buffering based on multiqueue */

InterfacePlayerRDK.cpp

@@ -195,7 +195,7 @@ static void TransformToEncryptedCaps(GstCaps *caps, const char *drmSystem)
 /*InterfacePlayerRDK constructor*/
 InterfacePlayerRDK::InterfacePlayerRDK(bool isRialto) :
 mProtectionLock(), mPauseInjector(false), mSourceSetupMutex(), stopCallback(NULL), tearDownCb(NULL), notifyFirstFrameCallback(NULL),
-mSourceSetupCV(), mScheduler(), callbackMap(), setupStreamCallbackMap(), mDrmSystem(NULL), mEncrypt(NULL), mDRMSessionManager(NULL),
+mSourceSetupCV(), mScheduler(), callbackMap(), setupStreamCallbackMap(), mDrmSystem(), mEncrypt(NULL), mDRMSessionManager(NULL),
 trickTeardown(false), mFirstFrameRequired(false), mResumeInjector(false), PipelineSetToReady(false), mSchedulerStarted(false)
 {
 	interfacePlayerPriv = new InterfacePlayerPriv(isRialto);
@@ -231,10 +231,6 @@ trickTeardown(false), mFirstFrameRequired(false), mResumeInjector(false), Pipeli
 InterfacePlayerRDK::~InterfacePlayerRDK()
 {
 	DestroyPipeline();
-	if (mDrmSystem)
-	{
-		delete[] mDrmSystem;
-	}
 	/* safe delete configuration parameter */
 	MW_SAFE_DELETE(m_gstConfigParam);
 	mScheduler.StopScheduler();
@@ -294,6 +290,8 @@ aSyncControl(), syncControl(), callbackControl(), seekPosition(0)
 	 * using the westerossink connected using westeros compositor.
 	 */
 	strcpy(videoRectangle, "");
+	videoRectangleWidth = 0;
+	videoRectangleHeight = 0;
 	for (int i = 0; i < GST_TRACK_COUNT; i++)
 	{
 		protectionEvent[i] = NULL;
@@ -1072,19 +1070,7 @@ void InterfacePlayerRDK::SetPreferredDRM(const char *drmID)
 {
 	if (drmID != NULL)
 	{
-		if (mDrmSystem != NULL)
-		{
-			delete[] mDrmSystem;
-		}
-		mDrmSystem = new char[strlen(drmID) + 1];
-		if (mDrmSystem != NULL)
-		{
-			strcpy(mDrmSystem, drmID);
-		}
-		else
-		{
-			MW_LOG_ERR("Memory allocation failed for mDrmSystem\n");
-		}
+		mDrmSystem = drmID;
 	}
 }
 
@@ -2208,7 +2194,7 @@ void InterfacePlayerRDK::InitializeSourceForPlayer(void *PlayerInstance, void *
 		if (stream->codecInfo.mIsEncrypted && IsElementaryStreamFormat(static_cast<GstStreamOutputFormat>(stream->format)))
 		{
 			MW_LOG_DEBUG("Applying encrypted caps during source configuration for type[%d] format[%d]", mediaType, stream->format);
-			TransformToEncryptedCaps(caps, mDrmSystem);
+			TransformToEncryptedCaps(caps, mDrmSystem.empty() ? NULL : mDrmSystem.c_str());
 		}
 		gchar *capsStr = gst_caps_to_string(caps);
 		MW_LOG_MIL("Setting caps for source[type:%d]: %s", mediaType, capsStr);
@@ -2607,7 +2593,10 @@ int InterfacePlayerRDK::SetupStream(int streamId,  void *playerInstance, std::st
 				auto subtitlebin = gst_bin_new("subtitlebin");
 				auto vipertransform = gst_element_factory_make("vipertransform", NULL);
 				gst_bin_add_many(GST_BIN(subtitlebin),vipertransform,textsink,NULL);
-				gst_element_link(vipertransform, textsink);
+				if (!gst_element_link(vipertransform, textsink))
+				{
+					MW_LOG_ERR("Failed to link vipertransform to rialtomsesubtitlesink; subtitles will not render");
+				}
 				gst_element_add_pad(subtitlebin, gst_ghost_pad_new("sink", gst_element_get_static_pad(vipertransform, "sink")));
 
 				g_object_set(stream->sinkbin, "text-sink", subtitlebin, NULL);
@@ -3160,14 +3149,13 @@ long InterfacePlayerRDK::GetDurationMilliseconds(void)
  */
 void InterfacePlayerRDK::GetVideoSize(int &width, int &height)
 {
-	int x;
-	int y;
-	int w = 0;
-	int h = 0;
-	if ((4 == sscanf(interfacePlayerPriv->gstPrivateContext->videoRectangle, "%d,%d,%d,%d", &x, &y, &w, &h)) && (w > 0) && (h > 0))
+	// Use the dimensions cached by SetVideoRectangle() instead of re-parsing
+	// the "x,y,w,h" string on every call.
+	if (interfacePlayerPriv->gstPrivateContext->videoRectangleWidth > 0 &&
+	    interfacePlayerPriv->gstPrivateContext->videoRectangleHeight > 0)
 	{
-		width = w;
-		height = h;
+		width  = interfacePlayerPriv->gstPrivateContext->videoRectangleWidth;
+		height = interfacePlayerPriv->gstPrivateContext->videoRectangleHeight;
 	}
 }
 
@@ -3206,6 +3194,8 @@ void InterfacePlayerRDK::SetVideoRectangle(int x, int y, int w, int h)
 		return;
 	}
 	snprintf(interfacePlayerPriv->gstPrivateContext->videoRectangle, sizeof(interfacePlayerPriv->gstPrivateContext->videoRectangle), "%d,%d,%d,%d", x,y,w,h);
+	interfacePlayerPriv->gstPrivateContext->videoRectangleWidth  = w;
+	interfacePlayerPriv->gstPrivateContext->videoRectangleHeight = h;
 	MW_LOG_MIL("Rect %s, video_sink =%p",
 			   interfacePlayerPriv->gstPrivateContext->videoRectangle, interfacePlayerPriv->gstPrivateContext->video_sink);
 	if (m_gstConfigParam->enableRectPropertyCfg)
@@ -5702,10 +5692,10 @@ static GstBusSyncReply bus_sync_handler(GstBus * bus, GstMessage * msg, Interfac
 			gst_message_parse_context_type(msg, &contextType);
 			if (!g_strcmp0(contextType, "drm-preferred-decryption-system-id"))
 			{
-				MW_LOG_MIL("Setting %s as preferred drm",pInterfacePlayerRDK->mDrmSystem);
+				MW_LOG_MIL("Setting %s as preferred drm", pInterfacePlayerRDK->mDrmSystem.c_str());
 				GstContext* context = gst_context_new("drm-preferred-decryption-system-id", FALSE);
 				GstStructure* contextStructure = gst_context_writable_structure(context);
-				gst_structure_set(contextStructure, "decryption-system-id", G_TYPE_STRING, pInterfacePlayerRDK->mDrmSystem,  NULL);
+				gst_structure_set(contextStructure, "decryption-system-id", G_TYPE_STRING, pInterfacePlayerRDK->mDrmSystem.empty() ? NULL : pInterfacePlayerRDK->mDrmSystem.c_str(),  NULL);
 				gst_element_set_context(GST_ELEMENT(GST_MESSAGE_SRC(msg)), context);
 			}
 
@@ -6192,7 +6182,7 @@ void InterfacePlayerRDK::SetStreamCaps(GstMediaType type, MediaCodecInfo&& codec
 		}
 		if (codecInfo.mIsEncrypted)
 		{
-			TransformToEncryptedCaps(caps, mDrmSystem);
+			TransformToEncryptedCaps(caps, mDrmSystem.empty() ? NULL : mDrmSystem.c_str());
 		}
 		gchar* capsStr = gst_caps_to_string(caps);
 		MW_LOG_MIL("Setting stream caps for type[%d] format[%d]: %s", type, codecInfo.mCodecFormat, capsStr);

InterfacePlayerRDK.h

@@ -22,6 +22,7 @@
 
 #include <map>
 #include <cstdint>
+#include <string>
 #include <string.h>
 #include <vector>
 #include <mutex>
@@ -166,7 +167,7 @@ class InterfacePlayerRDK
 
 	public:
 		Configs *m_gstConfigParam;
-		char *mDrmSystem;
+		std::string mDrmSystem;
 		void *mEncrypt;
 		void *mDRMSessionManager;
 		std::map<InterfaceCB, std::function<void()>> callbackMap;

drm/DrmSessionManager.cpp

@@ -74,8 +74,8 @@ KeyIdEntries::KeyIdEntries() : creationTime(0), isFailedKeyEntries(false), isPri
 /**
  *  @brief DrmSessionManager constructor.
  */
-DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function<void(uint32_t, uint32_t, const std::string&)> watermarkSessionUpdateCallback, DrmSessionCreator creator) : drmSessionContexts(NULL),
-		cachedKeyIDs(NULL), accessToken(NULL),
+DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::function<void(uint32_t, uint32_t, const std::string&)> watermarkSessionUpdateCallback, DrmSessionCreator creator) : drmSessionContexts(nullptr),
+		cachedKeyIDs(nullptr), accessToken(NULL),
 		accessTokenLen(0), sessionMgrState(SessionMgrState::eSESSIONMGR_ACTIVE), accessTokenMutex(),
 		cachedKeyMutex()
 		,mEnableAccessAttributes(true)
@@ -89,8 +89,8 @@ DrmSessionManager::DrmSessionManager(int maxDrmSessions, void *player, std::func
 		,mFirstFrameSeen(false)
 		,mPlayerSendWatermarkSessionUpdateEventCB(std::move(watermarkSessionUpdateCallback))
 {
-	drmSessionContexts	= new DrmSessionContext[mMaxDRMSessions];
-	cachedKeyIDs		= new KeyIdEntries[mMaxDRMSessions];
+	drmSessionContexts	= std::make_unique<DrmSessionContext[]>(mMaxDRMSessions);
+	cachedKeyIDs		= std::make_unique<KeyIdEntries[]>(mMaxDRMSessions);
 	m_drmConfigParam = new configs();
 	playerSecInstance = new PlayerSecInterface();
 
@@ -106,8 +106,6 @@ DrmSessionManager::~DrmSessionManager()
 {
 	clearAccessToken();
 	clearSessionData();
-	MW_SAFE_DELETE_ARRAY(drmSessionContexts);
-	MW_SAFE_DELETE_ARRAY(cachedKeyIDs);
 	MW_SAFE_DELETE(playerSecInstance);
 	MW_SAFE_DELETE(m_drmConfigParam);
 	ContentSecurityManager::setWatermarkSessionEvent_CB(nullptr);
@@ -1086,13 +1084,11 @@ void DrmSessionManager::UpdateMaxDRMSessions(int maxSessions)
 	{
 		// Clean up the current sessions
 		clearSessionData();
-		MW_SAFE_DELETE_ARRAY(drmSessionContexts);
-		MW_SAFE_DELETE_ARRAY(cachedKeyIDs);
 
 		//Update to new session count
 		mMaxDRMSessions = maxSessions;
-		drmSessionContexts      = new DrmSessionContext[mMaxDRMSessions];
-		cachedKeyIDs            = new KeyIdEntries[mMaxDRMSessions];
+		drmSessionContexts      = std::make_unique<DrmSessionContext[]>(mMaxDRMSessions);
+		cachedKeyIDs            = std::make_unique<KeyIdEntries[]>(mMaxDRMSessions);
 		MW_LOG_INFO("Updated DrmSessionManager MaxSession to:%d", mMaxDRMSessions);
 	}
 }

drm/DrmSessionManager.h

@@ -168,15 +168,15 @@ class DrmSessionManager
 {
 public:
 
-	DrmSessionContext *drmSessionContexts;
+	std::unique_ptr<DrmSessionContext[]> drmSessionContexts;
 	configs *m_drmConfigParam;
 	PlayerSecInterface *playerSecInstance;/** PlayerSecInterface instance **/
 	ContentSecurityManagerSession mContentSecurityManagerSession;
     std::atomic<bool> mFirstFrameSeen;
 	std::atomic<bool> mIsVideoOnMute;
 	std::atomic<int> mCurrentSpeed;
 protected:
-	KeyIdEntries *cachedKeyIDs;
+	std::unique_ptr<KeyIdEntries[]> cachedKeyIDs;
 	std::mutex cachedKeyMutex;
 private:
 	char* accessToken;

drm/helper/VerimatrixHelper.cpp

@@ -50,12 +50,8 @@ bool VerimatrixHelper::parsePssh(const uint8_t* initData, uint32_t initDataLen)
 
 	initData += VERIMATRIX_PSSH_DATA_POSITION;
 	initDataLen -= VERIMATRIX_PSSH_DATA_POSITION;
-	char *init = new char[initDataLen + 1];
-	memcpy(init, initData, initDataLen);
-	init[initDataLen] = 0;
 
-	std::string pssh(init);
-	delete []init;
+	std::string pssh(reinterpret_cast<const char*>(initData), initDataLen);
 
 	MW_LOG_WARN("pssh %s", pssh.c_str());
 

drm/ocdm/opencdmsessionadapter.cpp

@@ -42,6 +42,8 @@
 
 #include <sys/time.h>
 #include <set>
+#include <thread>
+#include <chrono>
 #define LICENSE_RENEWAL_MESSAGE_TYPE "1"
 
 /**
@@ -351,10 +353,16 @@ int OCDMSessionAdapter::processDRMKey(DrmData* key, uint32_t timeout)
 				}
 			} 
 
-			// wait for 5sec for all the logs to be flushed
-			sleep(5);
-			// Now kill self
-			processHandler.SelfKill();
+			// Defer the self-kill by 5sec on a worker thread so the killed
+			// OCDM process can restart and logs can flush, without blocking
+			// this decrypt/key path which would otherwise stall playback.
+			std::thread([]()
+			{
+				std::this_thread::sleep_for(std::chrono::seconds(5));
+				// Now kill self
+				ProcessHandler handler;
+				handler.SelfKill();
+			}).detach();
 		}
 		else {
 #ifdef USE_THUNDER_OCDM_API_0_2

drm/processProtectionHls.cpp

@@ -224,19 +224,15 @@ shared_ptr<DrmHelper> ProcessContentProtection( std::string attrName, bool propa
 		}
 		if (PlayerLogManager::isLogLevelAllowed(mLOGLEVEL_TRACE))
 		{
-			MW_LOG_TRACE("content metadata from manifest; length %zu", dataLength);
-			printf("*****************************************************************\n");
+			string dump;
+			dump.reserve(dataLength * 3 + 1);
 			for (int i = 0; i < dataLength; i++)
 			{
-				printf("%c", data[i]);
+				char hexByte[4];
+				snprintf(hexByte, sizeof(hexByte), "%02x ", data[i]);
+				dump += hexByte;
 			}
-			printf("\n*****************************************************************\n");
-			for (int i = 0; i < dataLength; i++)
-			{
-				printf("%02x ", data[i]);
-			}
-			printf("\n*****************************************************************\n");
-
+			MW_LOG_TRACE("content metadata from manifest; length %zu data %s", dataLength, dump.c_str());
 		}
 		if (!drmHelper->parsePssh(data, (uint32_t)dataLength))
 		{

externals/PlayerThunderAccessBase.h

@@ -110,7 +110,7 @@ struct PlayerAudioData{
     int pk;
     std::string mixType;
 
-    PlayerAudioData(std::string lang, std::string content_Type, std::string nm, std::string t, int pk_int, std::string mix_Type)
+    PlayerAudioData(const std::string& lang, const std::string& content_Type, const std::string& nm, const std::string& t, int pk_int, const std::string& mix_Type)
     : language(std::move(lang)),
     contentType(std::move(content_Type)),
     name(std::move(nm)),
@@ -146,7 +146,7 @@ struct PlayerTextData{
     std::string name;
     int pk;
 
-    PlayerTextData(std::string t, std::string lang, int ccSer, std::string cc_Type, std::string nm, int pk_int)
+    PlayerTextData(const std::string& t, const std::string& lang, int ccSer, const std::string& cc_Type, const std::string& nm, int pk_int)
     :type(std::move(t)),
     language(std::move(lang)),
     ccServiceNumber(ccSer),
@@ -204,7 +204,7 @@ class PlayerThunderAccessBase
      *   @retval true on success
      *   @retval false on failure
      */
-    virtual bool UnSubscribeEvent (std::string eventName) = 0;
+    virtual bool UnSubscribeEvent (const std::string& eventName) = 0;
 
     /**
      *   @fn SetVideoRectangle
@@ -218,7 +218,7 @@ class PlayerThunderAccessBase
      *   @retval true if success
      *   @retval false if failure
      */
-    virtual bool SetVideoRectangle(int x, int y, int w, int h, std::string videoInputType, PlayerThunderAccessShim shim) = 0;
+    virtual bool SetVideoRectangle(int x, int y, int w, int h, const std::string& videoInputType, PlayerThunderAccessShim shim) = 0;
 
     /**
      *   @fn SetPreferredAudioLanguages_OTA
@@ -244,12 +244,12 @@ class PlayerThunderAccessBase
      *   @param port port number
      *   @param  videoInputType string video input type
      */
-    virtual void StartHelperVideoin(int port, std::string videoInputType) = 0;
+    virtual void StartHelperVideoin(int port, const std::string& videoInputType) = 0;
     /**
      *   @fn StopHelper
      *   @param  videoInputType string video input type
      */
-    virtual void StopHelperVideoin(std::string videoInputType) = 0;
+    virtual void StopHelperVideoin(const std::string& videoInputType) = 0;
 
     /**
      *   @fn RegisterEventOnVideoStreamInfoUpdateHdmiin
@@ -276,7 +276,7 @@ class PlayerThunderAccessBase
      *   @param preferredRenditionString player's preferred rendition
      *   @param atsc_preferredRenditionString ATSC preferred rendition
      */
-    virtual void StartOta(std::string url, std::string waylandDisplay, std::string preferredLanguagesString, std::string atsc_preferredLanguagesString, std::string preferredRenditionString, std::string atsc_preferredRenditionString) = 0;
+    virtual void StartOta(const std::string& url, const std::string& waylandDisplay, const std::string& preferredLanguagesString, const std::string& atsc_preferredLanguagesString, const std::string& preferredRenditionString, const std::string& atsc_preferredRenditionString) = 0;
 
     /**
      *   @fn StopOta
@@ -288,7 +288,7 @@ class PlayerThunderAccessBase
      * @param audData vector to be filled with audio data
      * @return string of current track pk
      */
-    virtual std::string GetAudioTracksOta(std::vector<PlayerAudioData> audData) = 0;
+    virtual std::string GetAudioTracksOta(std::vector<PlayerAudioData>& audData) = 0;
 
     /**
      * @fn SetAudioTrackOta
@@ -303,7 +303,7 @@ class PlayerThunderAccessBase
      * @param txtData vector to be filled with text data
      * @return true if successful, false if failed
      */
-    virtual bool GetTextTracksOta(std::vector<PlayerTextData> txtData) = 0;
+    virtual bool GetTextTracksOta(std::vector<PlayerTextData>& txtData) = 0;
 
     /**
      * @fn DisableContentRestrictionsOta
@@ -333,7 +333,7 @@ class PlayerThunderAccessBase
      * @param onPlayerErrorHandlerCb player error handler callback
      * @return true if successful, false if failed
      */
-    virtual bool StartRmf(std::string url, std::function<void(std::string)> onPlayerStatusHandlerCb, std::function<void(std::string)> onPlayerErrorHandlerCb) = 0;
+    virtual bool StartRmf(const std::string& url, std::function<void(std::string)> onPlayerStatusHandlerCb, std::function<void(std::string)> onPlayerErrorHandlerCb) = 0;
 
     /**
      * @fn StopRmf
@@ -389,7 +389,7 @@ class PlayerThunderAccessBase
      * @param metaData metaData
      * @return true if successful, false if failed
      */
-    virtual bool PersistentStoreSaveWatermark(const char* base64Image, std::string metaData) = 0;
+    virtual bool PersistentStoreSaveWatermark(const char* base64Image, const std::string& metaData) = 0;
 
     /**
      * @fn PersistentStoreLoadWatermark