VPLAY-11795 memset on objects is potentially harmful

VPLAY-11795 memset on objects is potentially harmful

Reason for Change: Eliminates undefined behavior from memset on non-POD types and apply RAII principles to POD type initialization.

Non-POD fixes (prevents corruption/crashes):
- CachedFragment: Use Clear() method instead of memset
- SpeedCache: Use assignment operator for reset
- MediaStreamContext: Initialize in member initializer list
- AampLLDashServiceData: Refactor clear() to use assignment (DRY)
- RecordingComponent: Use assignment in reset methods and constructor
- Remove aesCtrAttrDataList memset (already initialized)

POD improvements (RAII compliance):
- Local arrays: Use = {} initialization (work, dataDescTags, tuneStrPrefix,
  description, moneytracebuf, buf)
- Member arrays: Add m_SPS, m_PPS to member initializer list
- TileInfo: Remove redundant memset

Code cleanup:
- Remove unused SetLLDashSpeedCache() and refactor test

Author: DomSyna

AampLLDASHData.h

@@ -29,51 +29,23 @@
  * @brief To store Low Latency Service configurations
  */
 struct AampLLDashServiceData {
-	bool lowLatencyMode;        	/**< LL Playback mode enabled */
-	bool strictSpecConformance; 	/**< Check for Strict LL Dash spec conformance*/
-	double availabilityTimeOffset;  	/**< LL Availability Time Offset */
-	bool availabilityTimeComplete;  	/**< LL Availability Time Complete */
-	int targetLatency;          	/**< Target Latency of playback */
-	int minLatency;             	/**< Minimum Latency of playback */
-	int maxLatency;             	/**< Maximum Latency of playback */
-	int latencyThreshold;       	/**< Latency when play rate correction kicks-in */
-	double minPlaybackRate;     	/**< Minimum playback rate for playback */
-	double maxPlaybackRate;     	/**< Maximum playback rate for playback */
-	bool isSegTimeLineBased;		/**< Indicates is stream is segmenttimeline based */
-	double fragmentDuration;		/**< Maximum Fragment Duration */
-	UtcTiming utcTiming;		/**< Server UTC timings */
+	bool lowLatencyMode = false;				/**< LL Playback mode enabled */
+	bool strictSpecConformance = false;			/**< Check for Strict LL Dash spec conformance*/
+	double availabilityTimeOffset = 0.0;		/**< LL Availability Time Offset */
+	bool availabilityTimeComplete = false;		/**< LL Availability Time Complete */
+	int targetLatency = 0;						/**< Target Latency of playback */
+	int minLatency = 0;							/**< Minimum Latency of playback */
+	int maxLatency = 0;							/**< Maximum Latency of playback */
+	int latencyThreshold = 0;					/**< Latency when play rate correction kicks-in */
+	double minPlaybackRate = 0.0;				/**< Minimum playback rate for playback */
+	double maxPlaybackRate = 0.0;				/**< Maximum playback rate for playback */
+	bool isSegTimeLineBased = false;			/**< Indicates is stream is segmenttimeline based */
+	double fragmentDuration = 0.0;				/**< Maximum Fragment Duration */
+	UtcTiming utcTiming = eUTC_HTTP_INVALID;	/**< Server UTC timings */
 
-	AampLLDashServiceData() : lowLatencyMode(false),
-			strictSpecConformance(false),
-			availabilityTimeOffset(0.0),
-			availabilityTimeComplete(false),
-			targetLatency(0),
-			minLatency(0),
-			maxLatency(0),
-			latencyThreshold(0),
-			minPlaybackRate(0.0),
-			maxPlaybackRate(0.0),
-			isSegTimeLineBased(false),
-			fragmentDuration(0.0),
-			utcTiming(eUTC_HTTP_INVALID)
-			{
-			}
-	/**< API to reset the Data*/
-	void clear(void)
+	void clear()
 	{
-		lowLatencyMode =false;
-		strictSpecConformance = false;
-		availabilityTimeOffset = 0.0;
-		availabilityTimeComplete = false;
-		targetLatency = 0;
-		minLatency = 0;
-		maxLatency = 0;
-		latencyThreshold = 0;
-		minPlaybackRate = 0.0;
-		maxPlaybackRate = 0.0;
-		isSegTimeLineBased = false;
-		fragmentDuration = 0.0; 
-		utcTiming = eUTC_HTTP_INVALID;
+		*this = {};
 	}
 };
 

fragmentcollector_mpd.cpp

@@ -159,14 +159,14 @@ StreamAbstractionAAMP_MPD::StreamAbstractionAAMP_MPD(class PrivateInstanceAAMP *
 	,mIsSegmentTimelineEnabled(false)
 	,mSeekedInPeriod(false)
 	,mIsFinalFirstPTS(false)
+	,mMediaStreamContext{}
 {
 	this->aamp = aamp;
 	if (aamp->mDRMLicenseManager)
 	{
 		AampDRMLicenseManager *licenseManager = aamp->mDRMLicenseManager;
 		licenseManager->SetLicenseFetcher(this);
 	}
-	memset(&mMediaStreamContext, 0, sizeof(mMediaStreamContext));
 	GetABRManager().clearProfiles();
 	mLastPlaylistDownloadTimeMs = aamp_GetCurrentTimeMS();
 
@@ -10199,7 +10199,7 @@ StreamAbstractionAAMP_MPD::~StreamAbstractionAAMP_MPD()
 	}
 
 	aamp->CurlTerm(eCURLINSTANCE_VIDEO, DEFAULT_CURL_INSTANCE_COUNT);
-	memset(aamp->GetLLDashServiceData(),0x00,sizeof(AampLLDashServiceData));
+	aamp->GetLLDashServiceData()->clear();
 	aamp->SetLowLatencyServiceConfigured(false);
 	aamp->SyncEnd();
 	mManifestDnldRespPtr = nullptr;
@@ -10983,7 +10983,6 @@ static void indexThumbnails(dash::mpd::IMPD *mpd, int thumbIndexValue, std::vect
 												{
 													std::string tmedia = media;
 													TileInfo tileInfo;
-													memset( &tileInfo,0,sizeof(tileInfo) );
 													tileInfo.startTime = startTime + ( adDuration / timeScale) ;
 													AAMPLOG_TRACE("timeLineIndex[%d] size [%zu] updated durationMs[%" PRIu64 "] startTime:%f adDuration:%f repeatCount:%d",  timeLineIndex, timelines.size(), durationMs, startTime, adDuration, repeatCount);
 
@@ -11031,7 +11030,6 @@ static void indexThumbnails(dash::mpd::IMPD *mpd, int thumbIndexValue, std::vect
 											{
 												std::string tmedia = media;
 												TileInfo tileInfo;
-												memset( &tileInfo,0,sizeof(tileInfo));
 												tileInfo.startTime = tStartTime;
 												tStartTime += tDuration; //increment the nextStartTime by TileDuration
 												replace(tmedia,"RepresentationID",RepresentationID);
@@ -12595,10 +12593,9 @@ double StreamAbstractionAAMP_MPD::GetEncoderDisplayLatency()
 						{
 							AAMPLOG_TRACE("ProducerReferenceTime@wallClockTime [%s]", wallClockTime.c_str());
 
-							std::tm tmTime;
+							std::tm tmTime{};
 							const char* format = "%Y-%m-%dT%H:%M:%S.%f%Z";
 							char out_buffer[ 80 ];
-							memset(&tmTime, 0, sizeof(tmTime));
 							strptime(wallClockTime.c_str(), format, &tmTime);
 							wTime = mktime(&tmTime);
 

priv_aamp.cpp

@@ -979,7 +979,7 @@ int PrivateInstanceAAMP::HandleSSLProgressCallback ( void *clientp, double dltot
 				//Reset speedcache when Fragment download Starts
 				struct SpeedCache* speedcache = NULL;
 				speedcache = aamp->GetLLDashSpeedCache();
-				memset(speedcache, 0x00, sizeof(struct SpeedCache));
+				*speedcache = SpeedCache();
 			}
 
 			downloadbps = getCurrentContentDownloadSpeed(aamp, context->mediaType, context->dlStarted, (long)context->downloadStartTime, dlnow);
@@ -1375,7 +1375,6 @@ PrivateInstanceAAMP::PrivateInstanceAAMP(AampConfig *config) : mReportProgressPo
 	mCustomHeaders["Connection:"] = std::vector<std::string> { "Keep-Alive" };
 	preferredLanguagesList.push_back("en");
 
-	memset(&aesCtrAttrDataList, 0, sizeof(aesCtrAttrDataList));
 	mHarvestCountLimit = GETCONFIGVALUE_PRIV(eAAMPConfig_HarvestCountLimit);
 	mHarvestConfig = GETCONFIGVALUE_PRIV(eAAMPConfig_HarvestConfig);
 	mAsyncTuneEnabled = ISCONFIGSET_PRIV(eAAMPConfig_AsyncTune);
@@ -6171,9 +6170,8 @@ void PrivateInstanceAAMP::Tune(const char *mainManifestUrl,
 	mIsFirstRequestToFOG = (mFogTSBEnabled == true);
 
 	{
-		char tuneStrPrefix[64];
+		char tuneStrPrefix[64] = {};
 		mTsbSessionRequestUrl.clear();
-		memset(tuneStrPrefix, '\0', sizeof(tuneStrPrefix));
 		if (!mAppName.empty())
 		{
 			snprintf(tuneStrPrefix, sizeof(tuneStrPrefix), "%s PLAYER[%d] APP: %s",(mbPlayEnabled?STRFGPLAYER:STRBGPLAYER), mPlayerId, mAppName.c_str());
@@ -8990,8 +8988,7 @@ void PrivateInstanceAAMP::UpdateLiveOffset()
  */
 void PrivateInstanceAAMP::SendStalledErrorEvent()
 {
-	char description[MAX_ERROR_DESCRIPTION_LENGTH];
-	memset(description, '\0', MAX_ERROR_DESCRIPTION_LENGTH);
+	char description[MAX_ERROR_DESCRIPTION_LENGTH] = {};
 	int stalltimeout = GETCONFIGVALUE_PRIV(eAAMPConfig_StallTimeoutMS);
 	snprintf(description, (MAX_ERROR_DESCRIPTION_LENGTH - 1), "Playback has been stalled for more than %d ms due to lack of new fragments", stalltimeout);
 	SendErrorEvent(AAMP_TUNE_PLAYBACK_STALLED, description);
@@ -9128,8 +9125,7 @@ MediaFormat PrivateInstanceAAMP::GetMediaFormatTypeEnum() const
  */
 void PrivateInstanceAAMP::GetMoneyTraceString(std::string &customHeader) const
 {
-	char moneytracebuf[512];
-	memset(moneytracebuf, 0, sizeof(moneytracebuf));
+	char moneytracebuf[512] = {};
 
 	if (mCustomHeaders.size() > 0)
 	{
@@ -12806,8 +12802,7 @@ struct curl_slist* PrivateInstanceAAMP::GetCustomHeaders(AampMediaType mediaType
 				{
 					continue;
 				}
-				char buf[512];
-				memset(buf, '\0', 512);
+				char buf[512] = {};
 				if (it->second.size() >= 2)
 				{
 					snprintf(buf, 512, "trace-id=%s;parent-id=%s;span-id=%lld",
@@ -13109,14 +13104,6 @@ uint32_t  PrivateInstanceAAMP::GetSubTimeScale(void)
 	return subTimeScale;
 }
 
-/**
- * @brief Sets Speed Cache
- */
-void PrivateInstanceAAMP::SetLLDashSpeedCache(struct SpeedCache &speedCache)
-{
-	this->speedCache = speedCache;
-}
-
 /**
  * @brief Gets Speed Cache
  */

priv_aamp.h

@@ -3534,14 +3534,6 @@ class PrivateInstanceAAMP : public DrmCallbacks, public std::enable_shared_from_
 	 */
 	uint32_t GetSubTimeScale(void);
 
-	/**
-	 *   @fn SetLLDashSpeedCache
-	 *
-	 *   @param[in] speedCache - Speed Cache
-	 *   @return void
-	 */
-	void SetLLDashSpeedCache(struct SpeedCache &speedCache);
-
 	/**
 	 *   @fn GetLLDashSpeedCache
 	 *

streamabstraction.cpp

@@ -1994,8 +1994,7 @@ void MediaTrack::FlushFragments()
 	{
 		for (int i = 0; i < maxCachedFragmentsPerTrack; i++)
 		{
-			mCachedFragment[i].fragment.Free();
-			memset(&mCachedFragment[i], 0, sizeof(CachedFragment));
+			mCachedFragment[i].Clear();
 		}
 		fragmentIdxToInject = 0;
 		fragmentIdxToFetch = 0;

test/utests/tests/PrivAampTests/PrivAampTests.cpp

@@ -3791,12 +3791,35 @@ TEST_F(PrivAampTests,SetAudTimeScaleTest)
  	EXPECT_EQ(val,val1);
 }
 
-TEST_F(PrivAampTests,SetLLDashSpeedCacheTest)
+TEST_F(PrivAampTests,GetLLDashSpeedCacheTest)
 {
-	struct SpeedCache spCache;
-	p_aamp->SetLLDashSpeedCache(spCache);
-
-	struct SpeedCache *pCache1 = p_aamp->GetLLDashSpeedCache();
+	// Test that GetLLDashSpeedCache returns a valid pointer and can be modified
+	struct SpeedCache *pCache = p_aamp->GetLLDashSpeedCache();
+	
+	ASSERT_NE(pCache, nullptr);
+	
+	pCache->last_sample_time_val = 12345;
+	pCache->speed_now = 5000000; // 5 Mbps
+	pCache->totalDownloaded = 1024000;
+	pCache->bStart = true;
+	pCache->mChunkSpeedData.push_back(std::make_pair(1.5, 4000000));
+	
+	struct SpeedCache *pCache2 = p_aamp->GetLLDashSpeedCache();
+	EXPECT_EQ(pCache2->last_sample_time_val, 12345);
+	EXPECT_EQ(pCache2->speed_now, 5000000);
+	EXPECT_EQ(pCache2->totalDownloaded, 1024000);
+	EXPECT_TRUE(pCache2->bStart);
+	EXPECT_EQ(pCache2->mChunkSpeedData.size(), 1);
+	
+	// Test resetting the cache 
+	*pCache = SpeedCache();
+	
+	// Verify reset worked
+	EXPECT_EQ(pCache->last_sample_time_val, 0);
+	EXPECT_EQ(pCache->speed_now, 0);
+	EXPECT_EQ(pCache->totalDownloaded, 0);
+	EXPECT_FALSE(pCache->bStart);
+	EXPECT_TRUE(pCache->mChunkSpeedData.empty());
 }
 
 TEST_F(PrivAampTests,SetLiveOffsetAppRequestTest)

tsFragmentProcessor.cpp

@@ -623,14 +623,12 @@ void TSFragmentProcessor::DemuxFragment(const uint8_t * base_packet_ptr, size_t
 void TSFragmentProcessor::ProcessPMTSection(uint8_t * section, size_t sectionLength)
 {
 	unsigned char *programInfo, *programInfoEnd;
-	char work[32];
+	char work[32] = {};
 
 	int version = ((section[2] >> 1) & 0x1F);
 	int pcrPid = (((section[5] & 0x1F) << 8) + section[6]);
 	int infoLength = (((section[7] & 0x0F) << 8) + section[8]);
 
-	memset(work, 0, sizeof(work));
-
 	// Reset of old values
 	ResetAudioComponents();
 	ResetVideoComponents();
@@ -817,15 +815,13 @@ void TSFragmentProcessor::ProcessPMTSection(uint8_t * section, size_t sectionLen
 
 void TSFragmentProcessor::ResetAudioComponents()
 {
-	// Reset of old values
 	for (auto & comp : m_audioComponents)
 	{
 		if (comp.associatedLanguage)
 		{
 			free(comp.associatedLanguage);
 		}
-		comp.associatedLanguage = nullptr;
-		memset(&comp, 0, sizeof(RecordingComponent));
+		comp = RecordingComponent();
 	}
 
 	m_audioComponentCount = 0;	
@@ -835,7 +831,7 @@ void TSFragmentProcessor::ResetVideoComponents()
 {
 	for (auto & comp : m_videoComponents)
 	{
-		memset(&comp, 0, sizeof(RecordingComponent));
+		comp = RecordingComponent();
 	}
 	m_videoComponentCount = 0;
 }

tsprocessor.cpp

@@ -196,6 +196,8 @@ TSProcessor::TSProcessor(class PrivateInstanceAAMP *aamp,StreamOperation streamO
 	, m_auxiliaryAudio(false)
 	,m_audioGroupId()
 	,m_applyOffset(true)
+	,m_SPS{}
+	,m_PPS{}
 {
 	AAMPLOG_INFO(" constructor: %p", this);
 	bool optimizeMuxed = false;
@@ -205,8 +207,6 @@ TSProcessor::TSProcessor(class PrivateInstanceAAMP *aamp,StreamOperation streamO
 		optimizeMuxed = (m_streamOperation == eStreamOp_DEMUX_ALL);
 	}
 
-	memset(m_SPS, 0, 32 * sizeof(H264SPS));
-	memset(m_PPS, 0, 256 * sizeof(H264PPS));
 	m_versionPMT = 0;
 
 	if ((m_streamOperation == eStreamOp_DEMUX_ALL) || (m_streamOperation == eStreamOp_DEMUX_VIDEO) || (m_streamOperation == eStreamOp_DEMUX_VIDEO_AND_AUX))
@@ -237,10 +237,6 @@ TSProcessor::TSProcessor(class PrivateInstanceAAMP *aamp,StreamOperation streamO
 		}
 		m_demux = true;
 	}
-
-	int compBufLen = MAX_PIDS*sizeof(RecordingComponent);
-	memset(videoComponents, 0, compBufLen);
-	memset(audioComponents, 0, compBufLen);
 }
 
 /**
@@ -395,9 +391,9 @@ void TSProcessor::insertPCR(unsigned char *packet, int pid)
 void TSProcessor::processPMTSection(unsigned char* section, int sectionLength)
 {
 	unsigned char *programInfo, *programInfoEnd;
-	unsigned int dataDescTags[MAX_PIDS];
+	unsigned int dataDescTags[MAX_PIDS] = {};
 	int streamType = 0, pid = 0, len = 0;
-	char work[32];
+	char work[32] = {};
 	StreamOutputFormat videoFormat = FORMAT_INVALID;
 	StreamOutputFormat audioFormat = FORMAT_INVALID;
 	bool cueiDescriptorFound = false;
@@ -406,21 +402,20 @@ void TSProcessor::processPMTSection(unsigned char* section, int sectionLength)
 	int pcrPid = (((section[5] & 0x1F) << 8) + section[6]);
 	int infoLength = (((section[7] & 0x0F) << 8) + section[8]);
 
-	for (int i = 0; i < audioComponentCount; ++i)
+	for (auto & comp : videoComponents)
 	{
-		if (audioComponents[i].associatedLanguage)
+		comp = RecordingComponent();
+	}
+
+	for (auto & comp : audioComponents)
+	{
+		if (comp.associatedLanguage)
 		{
-			free(audioComponents[i].associatedLanguage);
+			free(comp.associatedLanguage);
 		}
+		comp = RecordingComponent();
 	}
 
-	memset(videoComponents, 0, sizeof(videoComponents));
-	memset(audioComponents, 0, sizeof(audioComponents));
-
-	memset(dataDescTags, 0, sizeof(dataDescTags));
-
-	memset(work, 0, sizeof(work));
-
 	videoComponentCount = audioComponentCount = 0;
 	m_dsmccComponentFound = false;