diff --git a/AAMP-UVE-API.md b/AAMP-UVE-API.md index 57de935b86..c1878696dd 100644 --- a/AAMP-UVE-API.md +++ b/AAMP-UVE-API.md @@ -2716,28 +2716,47 @@ XREReceiver.onEvent("onDecoderAvailable", { decoderHandle: null }); ## Inband (CEA608/708) Closed Caption Management (modern UVE/AAMP API) -Configure nativeCCRendering to true to signal use of subtec for caption rendering. -``` -player.initConfig( { nativeCCRendering: true } ); +Whether AAMP manages CC visibility and styles directly depends on the platform. +On **X1 devices**, CC is owned by **XREReceiver**, which runs independently of AAMP. +`nativeCCRendering` must remain `false` (the default) so AAMP does not interfere with +XREReceiver's trickplay muting, parental control gating, or style management. +On **platforms without XREReceiver**, the app must set `nativeCCRendering: true` to +hand AAMP ownership of the CC lifecycle via `PlayerCCManager`. Apps must refrain from +using AAMP CC APIs when `nativeCCRendering` is set to false. +### Platform Summary + +| Device Class | `nativeCCRendering` | `player.setTextStyleOptions` | +|---|---|---| +| X1 (XREReceiver present) | `false` (default — do not set to true) | Do **not** call AAMP CC APIs; XREReceiver owns CC visibility and styling and applies guide-configured styles automatically | +| Non-XRE but pre-ENT-OS | `true` (must be explicitly enabled) | Required to apply caption styling; guide settings are not automatically propagated | +| ENT-OS (with Text Track plugin) | `true` | Not required; Text Track plugin automatically maps guide-configured caption styling | + +### Configuration + +For non-XRE platforms, opt AAMP into managing CC before calling `load()`: +```js +player.initConfig( { nativeCCRendering: true } ); ``` -Toggle CC display on or off at runtime: -``` -player.setClosedCaptionStatus(true); // show captions (off by default) -player.setClosedCaptionStatus(false); // mute captions -``` -Get/Set CC track at runtime: -``` -player.getTextTrack(); // returns json object listing track attributes -player.setTextTrack(trackIdentifier); -Get/Set CC style options at runtime +### Runtime CC Control + +Toggle CC display on or off: +```js +player.setClosedCaptionStatus(true); // show captions (off by default) +player.setClosedCaptionStatus(false); // hide captions ``` -player.getTextStyleOptions(); // returns JSON object reflecting currently styling options -player.setTextStyleOptions(options); // TODO: include examples known to work with RDK CC Manager and/or subtec -On newer devices there is no need to call setTextStyleOptions, as the Text Track plugin will automatically map guide-configured caption styling. +Get/Set CC track: +```js +player.getTextTrack(); // returns JSON object listing track attributes +player.setTextTrack(trackIdentifier); +``` +Get/Set CC style options: +```js +player.getTextStyleOptions(); // returns JSON object reflecting current styling options +player.setTextStyleOptions(options); // set styling options (see setTextStyleOptions API for format) ``` --- diff --git a/AampConfig.h b/AampConfig.h index 5e6453f924..cad502861a 100644 --- a/AampConfig.h +++ b/AampConfig.h @@ -152,7 +152,18 @@ typedef enum eAAMPConfig_BulkTimedMetaReport, /**< Enabled Bulk event reporting for TimedMetadata*/ eAAMPConfig_BulkTimedMetaReportLive, /**< Enabled Bulk TimedMetadata event reporting for live stream */ eAAMPConfig_AvgBWForABR, /**< Enables usage of AverageBandwidth if available for ABR */ - eAAMPConfig_NativeCCRendering, /**< If native CC rendering to be supported */ + eAAMPConfig_NativeCCRendering, /**< Controls whether AAMP manages CC visibility/styles + directly via PlayerCCManager (true), or defers to an + external CC controller such as XREReceiver (false). + Default: false. + On X1 platforms XREReceiver owns CC; set to false so + AAMP does not interfere with trickplay muting, parental + control gating, or CC track selection. + On platforms without XREReceiver, set to true so AAMP + takes over the full CC lifecycle. + Note: Regardless of this flag, AAMP's CC APIs still + route through PlayerCCManager and apps must refrain from + using them when the flag is set to false.*/ eAAMPConfig_Subtec_subtitle, /**< Enable subtec-based subtitles */ eAAMPConfig_WebVTTNative, /**< Enable subtec-based subtitles */ eAAMPConfig_AsyncTune, /**< To enable Asynchronous tune */ diff --git a/AampMPDParseHelper.cpp b/AampMPDParseHelper.cpp index def5d31bd4..e6d84be997 100644 --- a/AampMPDParseHelper.cpp +++ b/AampMPDParseHelper.cpp @@ -30,7 +30,7 @@ * @fn AampMPDParseHelper * @brief Default Constructor */ -AampMPDParseHelper::AampMPDParseHelper() : mMPDInstance(NULL),mIsLiveManifest(false),mMinUpdateDurationMs(0), +AampMPDParseHelper::AampMPDParseHelper() : mMPDInstance(NULL),mIsLiveManifest(false),mMinUpdateDurationMs(0), mIsFogMPD(false), mAvailabilityStartTime(0.0),mPublishTime(0.0),mSegmentDurationSeconds(0),mTSBDepth(0.0), mPresentationOffsetDelay(0.0),mMediaPresentationDuration(0), @@ -654,7 +654,7 @@ double AampMPDParseHelper::GetPeriodEndTime(int periodIndex, uint64_t mLastPlayl } string startTimeStr = period->GetStart(); - periodDurationMs = GetPeriodDuration(periodIndex,mLastPlaylistDownloadTimeMs,checkIFrame,IsUninterruptedTSB); + periodDurationMs = GetPeriodDuration(periodIndex,mLastPlaylistDownloadTimeMs,checkIFrame,IsUninterruptedTSB); if((mMPDInstance->GetAvailabilityStarttime().empty()) && !(mMPDInstance->GetType() == "static")) { AAMPLOG_WARN("availabilityStartTime required to calculate period duration not present in MPD"); @@ -669,18 +669,18 @@ double AampMPDParseHelper::GetPeriodEndTime(int periodIndex, uint64_t mLastPlayl { AAMPLOG_INFO("Period startTime is not present in MPD, so calculating start time with previous period durations"); if(mIsLiveManifest) - { + { periodStartMs = GetPeriodStartTime(periodIndex,mLastPlaylistDownloadTimeMs) * 1000 - (mAvailabilityStartTime * 1000); } else { - periodStartMs = GetPeriodStartTime(periodIndex,mLastPlaylistDownloadTimeMs) * 1000; + periodStartMs = GetPeriodStartTime(periodIndex,mLastPlaylistDownloadTimeMs) * 1000; } } else { periodStartMs = ParseISO8601Duration(startTimeStr.c_str()) + (aamp_GetPeriodStartTimeDeltaRelativeToPTSOffset(period)* 1000); - } + } periodEndTime = ((double)(periodStartMs + periodDurationMs) /1000); if(mIsLiveManifest) { @@ -855,14 +855,14 @@ std::shared_ptr AampMPDParseHelper::GetSegmentTemplateForVideo double AampMPDParseHelper::GetPeriodDuration(int periodIndex,uint64_t mLastPlaylistDownloadTimeMs, bool checkIFrame, bool IsUninterruptedTSB) { auto it = std::find_if(mMPDPeriodDetails.begin(), mMPDPeriodDetails.end(), - [periodIndex](const PeriodInfo& period) { - return period.periodIndex == periodIndex; - }); - - if (it != mMPDPeriodDetails.end()) { - // Found a matching PeriodInfo object, return its Duration - return it->duration; - } + [periodIndex](const PeriodInfo& period) { + return period.periodIndex == periodIndex; + }); + + if (it != mMPDPeriodDetails.end()) { + // Found a matching PeriodInfo object, return its Duration + return it->duration; + } else { double periodDuration = 0; diff --git a/AampMPDUtils.cpp b/AampMPDUtils.cpp index 2d8398663f..7a1b242cdc 100644 --- a/AampMPDUtils.cpp +++ b/AampMPDUtils.cpp @@ -221,7 +221,7 @@ double ComputeFragmentDuration( uint32_t duration, uint32_t timeScale ) * @param[out] referenced_duration referenced duration * @retval true on success */ -bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset) +bool ParseSegmentIndexBox( const uint8_t *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset) { if (!start) { @@ -229,7 +229,7 @@ bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, uns return false; } - const char **f = &start; + const uint8_t **f = &start; unsigned int len = Read32(f); if (len != size) @@ -297,14 +297,14 @@ bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, uns * @param[in] n word size in bytes * @retval 32 bit value */ -uint64_t ReadWordHelper( const char **pptr, int n ) +uint64_t ReadWordHelper( const uint8_t **pptr, int n ) { - const char *ptr = *pptr; + const uint8_t *ptr = *pptr; uint64_t rc = 0; while( n-- ) { rc <<= 8; - rc |= (unsigned char)*ptr++; + rc |= *ptr++; } *pptr = ptr; return rc; @@ -315,7 +315,7 @@ uint64_t ReadWordHelper( const char **pptr, int n ) * @param pptr pointer to read from * @retval word value */ -unsigned int Read16( const char **pptr) +unsigned int Read16( const uint8_t **pptr) { return (unsigned int)ReadWordHelper(pptr,2); } @@ -325,7 +325,7 @@ unsigned int Read16( const char **pptr) * @param pptr pointer to read from * @retval word value */ -unsigned int Read32( const char **pptr) +unsigned int Read32( const uint8_t **pptr) { return (unsigned int)ReadWordHelper(pptr,4); } @@ -335,7 +335,7 @@ unsigned int Read32( const char **pptr) * @param pptr pointer to read from * @retval word value */ -uint64_t Read64( const char **pptr) +uint64_t Read64( const uint8_t **pptr) { return ReadWordHelper(pptr,8); } diff --git a/AampMPDUtils.h b/AampMPDUtils.h index 545fe46731..ee941cfef8 100644 --- a/AampMPDUtils.h +++ b/AampMPDUtils.h @@ -93,28 +93,28 @@ void ConstructFragmentURL( std::string& fragmentUrl, const FragmentDescriptor *f * @param[out] referenced_duration referenced duration * @retval true on success */ -bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset); +bool ParseSegmentIndexBox( const uint8_t *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset); /** * @brief Read 16 word helper function * @param pptr pointer to read from * @retval word value */ -unsigned int Read16( const char **pptr); +unsigned int Read16( const uint8_t **pptr); /** * @brief Read 32 word helper function * @param pptr pointer to read from * @retval word value */ -unsigned int Read32( const char **pptr); +unsigned int Read32( const uint8_t **pptr); /** * @brief Read 64 word helper function * @param pptr pointer to read from * @retval word value */ -uint64_t Read64( const char **pptr); +uint64_t Read64( const uint8_t **pptr); /** * @brief read unsigned multi-byte value and update buffer pointer @@ -122,7 +122,7 @@ uint64_t Read64( const char **pptr); * @param[in] n word size in bytes * @retval 32 bit value */ -uint64_t ReadWordHelper( const char **pptr, int n ); +uint64_t ReadWordHelper( const uint8_t **pptr, int n ); /** * @brief Replace matching token with given number diff --git a/AampTSBSessionManager.cpp b/AampTSBSessionManager.cpp index df103b2055..d0186da4ae 100644 --- a/AampTSBSessionManager.cpp +++ b/AampTSBSessionManager.cpp @@ -210,9 +210,8 @@ std::shared_ptr AampTSBSessionManager::Read(TsbInitDataPtr initf cachedFragment->fragment.resize(len); UnlockReadMutex(); - TSB::Status status = mTSBStore->Read(uniqueUrl, cachedFragment->fragment.GetPtr(), len); + TSB::Status status = mTSBStore->Read(uniqueUrl, cachedFragment->fragment.data(), len); LockReadMutex(); - if (status != TSB::Status::OK) { AAMPLOG_WARN("Failure in read from TSBLibrary"); @@ -279,7 +278,7 @@ std::shared_ptr AampTSBSessionManager::Read(TsbFragmentDataPtr f cachedFragment->fragment.resize(len); UnlockReadMutex(); - status = mTSBStore->Read(uniqueUrl, cachedFragment->fragment.GetPtr(), len); + status = mTSBStore->Read(uniqueUrl, cachedFragment->fragment.data(), len); LockReadMutex(); if (status == TSB::Status::OK) @@ -323,7 +322,7 @@ void AampTSBSessionManager::EnqueueWrite(std::string url, std::shared_ptrRecalculatePTS(static_cast(cachedFragment->type), cachedFragment->fragment.GetPtr(), cachedFragment->fragment.size()); + double pts = mAamp->RecalculatePTS(static_cast(cachedFragment->type), cachedFragment->fragment.data(), cachedFragment->fragment.size()); // Get or create the datamanager for the mediatype std::shared_ptr dataManager = GetTsbDataManager(mediaType); @@ -404,7 +403,7 @@ void AampTSBSessionManager::ProcessWriteQueue() std::string uniqueUrl = ToUniqueUrl(writeData.url, writeData.cachedFragment->absPosition); // Call TSBHandler Write operation - TSB::Status status = mTSBStore->Write(uniqueUrl, writeData.cachedFragment->fragment.GetPtr(), writeData.cachedFragment->fragment.size()); + TSB::Status status = mTSBStore->Write(uniqueUrl, writeData.cachedFragment->fragment.data(), writeData.cachedFragment->fragment.size()); if (status == TSB::Status::OK) { diff --git a/ElementaryProcessor.cpp b/ElementaryProcessor.cpp index eac4b666c6..771a6207ec 100644 --- a/ElementaryProcessor.cpp +++ b/ElementaryProcessor.cpp @@ -45,11 +45,11 @@ ElementaryProcessor::~ElementaryProcessor() * @brief Process and send Elementary fragment */ bool ElementaryProcessor::sendSegment(AampGrowableBuffer* pBuffer,double position,double duration, double fragmentPTSoffset, bool discontinuous, - bool isInit,process_fcn_t processor, bool &ptsError) + bool isInit,process_fcn_t processor, bool &ptsError) { ptsError = false; bool ret = true; - ret = setTuneTimePTS(pBuffer->GetPtr(), pBuffer->size(), position, duration, discontinuous, ptsError); + ret = setTuneTimePTS(pBuffer->data(), pBuffer->size(), position, duration, discontinuous, ptsError); if (ret) { AAMPLOG_INFO("IsoBmffProcessor:: eMEDIATYPE_SUBTITLE sending segment at pos:%f dur:%f", position, duration); @@ -76,7 +76,7 @@ void ElementaryProcessor::sendStream(AampGrowableBuffer *pBuffer,double position /** * @brief Process and set tune time PTS */ -bool ElementaryProcessor::setTuneTimePTS(char *segment, const size_t& size, double position, double duration, bool discontinuous, bool &ptsError) +bool ElementaryProcessor::setTuneTimePTS(const uint8_t *segment, size_t size, double position, double duration, bool discontinuous, bool &ptsError) { ptsError = false; bool ret = true; diff --git a/ElementaryProcessor.h b/ElementaryProcessor.h index 9b39373a85..d8c3bf1e12 100644 --- a/ElementaryProcessor.h +++ b/ElementaryProcessor.h @@ -159,7 +159,7 @@ class ElementaryProcessor : public MediaProcessor /** * @fn setTuneTimePTS * - * @param[in] segment - fragment buffer pointer + * @param[in] segment - fragment buffer pointer (binary data) * @param[in] size - fragment buffer size * @param[in] position - position of fragment * @param[in] duration - duration of fragment @@ -167,7 +167,7 @@ class ElementaryProcessor : public MediaProcessor * @param[out] ptsError - flag indicates if any PTS error occurred * @return false if base was set, true otherwise */ - bool setTuneTimePTS(char *segment, const size_t& size, double position, double duration, bool discontinuous, bool &ptsError); + bool setTuneTimePTS(const uint8_t *segment, size_t size, double position, double duration, bool discontinuous, bool &ptsError); /** * @fn setBasePTS diff --git a/MediaStreamContext.cpp b/MediaStreamContext.cpp index 6a3c2b55f3..d4c99d644c 100644 --- a/MediaStreamContext.cpp +++ b/MediaStreamContext.cpp @@ -205,10 +205,15 @@ bool MediaStreamContext::CacheFragment(std::string fragmentUrl, unsigned int cur /** * @brief Cache Fragment Chunk */ -bool MediaStreamContext::CacheFragmentChunk(AampMediaType actualType, const char *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks) +bool MediaStreamContext::CacheFragmentChunk(AampMediaType actualType, const uint8_t *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks) { AAMPLOG_DEBUG("[%s] Chunk Buffer Length %zu Remote URL %s", name, size, remoteUrl.c_str()); + if (ptr == NULL && size > 0) + { + AAMPLOG_WARN("[%s] Null fragment pointer with non-zero size %zu", name, size); + return false; + } bool ret = true; if (WaitForCachedFragmentChunkInjected()) { @@ -245,7 +250,7 @@ bool MediaStreamContext::CacheFragmentChunk(AampMediaType actualType, const char */ cachedFragment->PTSOffsetSec = GetContext()->mPTSOffset.inSeconds(); - AAMPLOG_TRACE("[%s] cachedFragment %p ptr %p", name, cachedFragment, cachedFragment->fragment.GetPtr()); + AAMPLOG_TRACE("[%s] cachedFragment %p ptr %p", name, cachedFragment, cachedFragment->fragment.data()); UpdateTSAfterChunkFetch(); } else @@ -638,7 +643,7 @@ void MediaStreamContext::OnFragmentDownloadSuccess(DownloadInfoPtr dlInfo) (IsLocalTSBInjection() || (isPipelinePaused && !aamp->GetBufUnderFlowStatus()))) { AAMPLOG_TRACE("[%s] cachedFragment %p ptr %p not injecting IsLocalTSBInjection %d, aamp->mSinkPaused %d, aamp->GetBufUnderFlowStatus() %d", - name, cachedFragment, cachedFragment->fragment.GetPtr(), IsLocalTSBInjection(), isPipelinePaused, aamp->GetBufUnderFlowStatus()); + name, cachedFragment, cachedFragment->fragment.data(), IsLocalTSBInjection(), isPipelinePaused, aamp->GetBufUnderFlowStatus()); cachedFragment->fragment.Free(); auto timeBasedBufferManager = GetTimeBasedBufferManager(); if(timeBasedBufferManager) @@ -870,7 +875,7 @@ bool MediaStreamContext::DownloadFragment(DownloadInfoPtr dlInfo) { unsigned int firstOffset; ParseSegmentIndexBox( - IDX.GetPtr(), + IDX.data(), IDX.size(), 0, NULL, @@ -887,7 +892,7 @@ bool MediaStreamContext::DownloadFragment(DownloadInfoPtr dlInfo) for (int i = 0; i < dlInfo->fragmentIndex; i++) { if (ParseSegmentIndexBox( - IDX.GetPtr(), + IDX.data(), IDX.size(), i, &referenced_size, @@ -901,7 +906,7 @@ bool MediaStreamContext::DownloadFragment(DownloadInfoPtr dlInfo) unsigned int referenced_size; float fragmentDuration; if (ParseSegmentIndexBox( - IDX.GetPtr(), + IDX.data(), IDX.size(), dlInfo->fragmentIndex, &referenced_size, diff --git a/MediaStreamContext.h b/MediaStreamContext.h index fc87c7f11e..9e4b027606 100644 --- a/MediaStreamContext.h +++ b/MediaStreamContext.h @@ -135,7 +135,7 @@ class MediaStreamContext : public MediaTrack * @param dnldStartTime of the download * @param durationInTicks duration of the chunk in ticks */ - bool CacheFragmentChunk(AampMediaType actualType, const char *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks); + bool CacheFragmentChunk(AampMediaType actualType, const uint8_t *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks); /** * @fn CacheFragmentData diff --git a/MetadataProcessor.cpp b/MetadataProcessor.cpp index d6ec08f3a8..0e30eea79b 100644 --- a/MetadataProcessor.cpp +++ b/MetadataProcessor.cpp @@ -49,9 +49,6 @@ void IsoBMFFMetadataProcessor::ProcessFragmentMetadata(const CachedFragment * ca AAMPLOG_INFO(" [metadata][%p] Processing metadata.", this); AAMPLOG_INFO(" [metadata][%p] - Starting processing fragment - uri: %s", this, uri.c_str()); - char * data_ptr = const_cast(cachedFragment->fragment.GetPtr()); - auto data_len = cachedFragment->fragment.size(); - if (discontinuity_pending && mPtsOffsetUpdate) { AAMPLOG_INFO(" [metadata][%p] - Processing discontinuity with current PTS: %" PRIu64 " | %f", this, mCurrentMaxPTS, mCurrentMaxPTS_s); @@ -71,7 +68,7 @@ void IsoBMFFMetadataProcessor::ProcessFragmentMetadata(const CachedFragment * ca AAMPLOG_INFO(" [metadata][%p] Has valid PTS, processing the fragment", this); - ProcessID3Metadata(type, data_ptr, data_len); + ProcessID3Metadata(type, cachedFragment->fragment.GetVector()); } bool IsoBMFFMetadataProcessor::SetTuneTimePTS() @@ -114,18 +111,18 @@ bool IsoBMFFMetadataProcessor::SetTuneTimePTS() return ret; } -void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const char * data_ptr, size_t data_len) +void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const std::vector& data) { namespace aih = aamp::id3_metadata::helpers; - if (data_ptr) + if (!data.empty()) { - uint8_t * seg_buffer = const_cast(reinterpret_cast(data_ptr)); + const size_t data_len = data.size(); IsoBmffBuffer buffer; - buffer.setBuffer(seg_buffer, data_len); + buffer.setBuffer(data.data(), data_len); buffer.parseBuffer(); - if(!buffer.isInitSegment()) + if (!buffer.isInitSegment()) { uint8_t* message = nullptr; uint32_t messageLen = 0; @@ -152,7 +149,7 @@ void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const char size_t curOffset = 0; while (curOffset < data_len) { - uint8_t * box_ptr = seg_buffer + curOffset; + uint8_t * box_ptr = const_cast(data.data() + curOffset); Box *box = Box::constructBox(box_ptr, (uint32_t)(data_len - curOffset), false, -1); box->setOffset((uint32_t)curOffset); @@ -160,7 +157,7 @@ void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const char if (IS_TYPE(box->getType(), Box::EMSG)) { - uint8_t * ptr = seg_buffer + curOffset; + uint8_t * ptr = const_cast(data.data() + curOffset); const uint8_t * src_box_ptr = ptr; std::stringstream ss; @@ -201,8 +198,6 @@ void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const char } } - - TSMetadataProcessor::TSMetadataProcessor(id3_callback_t id3_hdl, ptsoffset_update_t ptsoffset_callback, std::shared_ptr video_processor) diff --git a/MetadataProcessor.hpp b/MetadataProcessor.hpp index 8fad3f0ccd..f7bde309d4 100644 --- a/MetadataProcessor.hpp +++ b/MetadataProcessor.hpp @@ -178,10 +178,9 @@ class IsoBMFFMetadataProcessor : public MetadataProcessorIntf, MetadataProcessor * @brief Processes the IsoBMFF stream to extract ID3 metadata * * @param type AampMediaType - * @param data_ptr Pointer to the segment's data - * @param data_len Length of the segment + * @param data Reference to vector containing the segment's binary data */ - void ProcessID3Metadata(AampMediaType type, const char * data_ptr, size_t data_len); + void ProcessID3Metadata(AampMediaType type, const std::vector& data); /// Flag for tracking whether the current PTS is valid (Initialized) or not bool processPTSComplete; diff --git a/StreamAbstractionAAMP.h b/StreamAbstractionAAMP.h index 0bb769906e..ca94d4b7d8 100644 --- a/StreamAbstractionAAMP.h +++ b/StreamAbstractionAAMP.h @@ -162,11 +162,23 @@ class MediaTrack void AbortWaitForPlaylistDownload(); /** - * @fn AbortFragmentDownloaderWait + * @fn AbortWaitForManifestUpdate * * @return void */ - void AbortFragmentDownloaderWait(); + void AbortWaitForManifestUpdate(); + + /** + * @fn GetManifestUpdateCounter + * @brief Returns the current manifest update counter. + * Callers should snapshot this value BEFORE performing any + * check or download work that leads to the decision to wait, + * then pass it to WaitForManifestUpdate(snapshotCounter). + * This closes the race window where AbortWaitForManifestUpdate() + * fires between the caller's work and the wait call. + * @return Current counter value. + */ + uint32_t GetManifestUpdateCounter(); /** * @fn AbortWaitForCachedFragmentChunk @@ -182,6 +194,19 @@ class MediaTrack */ void WaitForManifestUpdate(); + /** + * @fn WaitForManifestUpdate + * @brief Overload that accepts a caller-supplied counter snapshot. + * Blocks until the live counter differs from snapshotCounter. + * If AbortWaitForManifestUpdate() already fired after the snapshot + * was taken, the predicate is immediately true and no blocking + * occurs — no lost-wakeup. + * @param[in] snapshotCounter Snapshot obtained from GetManifestUpdateCounter() + * before the caller began its check or download work. + * @return void + */ + void WaitForManifestUpdate(uint32_t snapshotCounter); + /** * @fn PlaylistDownloader * @@ -265,13 +290,6 @@ class MediaTrack */ AampMediaType GetPlaylistMediaTypeFromTrack(TrackType type, bool isIframe); - /** - * @fn NotifyFragmentCollectorWait - * - * @return void - */ - void NotifyFragmentCollectorWait() {fragmentCollectorWaitingForPlaylistUpdate = true;} - /** * @fn EnterTimedWaitForPlaylistRefresh * @@ -883,8 +901,8 @@ class MediaTrack bool abortPlaylistDownloader; /**< Flag used to abort playlist downloader*/ std::condition_variable plDownloadWait; /**< Conditional variable for signaling timed wait*/ std::mutex dwnldMutex; /**< Download mutex for conditional timed wait, used for playlist and fragment downloads*/ - bool fragmentCollectorWaitingForPlaylistUpdate; /**< Flag to indicate that the fragment collector is waiting for ongoing playlist download, used for profile changes*/ - std::condition_variable frDownloadWait; /**< Conditional variable for signaling timed wait*/ + uint32_t mManifestUpdateCounter; /**< Monotonically increasing counter incremented by AbortWaitForManifestUpdate. */ + std::condition_variable mManifestUpdateWait; /**< Conditional variable for signaling manifest update */ std::condition_variable audioFragmentCached; /**< Signal after a audio fragment cached after reconfigure */ double lastInjectedPosition; /**< Last injected position */ double lastInjectedDuration; /**< Last injected fragment end position */ diff --git a/admanager_mpd.cpp b/admanager_mpd.cpp index a604f1fe75..d11171cf13 100644 --- a/admanager_mpd.cpp +++ b/admanager_mpd.cpp @@ -934,7 +934,7 @@ MPD* PrivateCDAIObjectMPD::GetAdMPD(std::string &manifestUrl, bool &finalManifes { finalManifest = true; } - std::string manifestStr(manifest.GetPtr(), manifest.size()); + std::string manifestStr(reinterpret_cast(manifest.data()), manifest.size()); xmlTextReaderPtr reader = xmlReaderForMemory(manifestStr.c_str(), (int) manifestStr.size(), NULL, NULL, 0); if(tryFog && !mAamp->mConfig->IsConfigSet(eAAMPConfig_PlayAdFromCDN) && reader && mIsFogTSB) //Main content from FOG. Ad is expected from FOG. { @@ -968,8 +968,8 @@ MPD* PrivateCDAIObjectMPD::GetAdMPD(std::string &manifestUrl, bool &finalManifes { //FOG already has the manifest. Releasing the one from CDN and using FOG's xmlFreeTextReader(reader); - reader = xmlReaderForMemory(fogManifest.GetPtr(), (int) fogManifest.size(), NULL, NULL, 0); - manifestStr.assign(fogManifest.GetPtr(), fogManifest.size()); + reader = xmlReaderForMemory(reinterpret_cast(fogManifest.data()), (int) fogManifest.size(), NULL, NULL, 0); + manifestStr.assign(reinterpret_cast(fogManifest.data()), fogManifest.size()); manifest.Free(); manifest.Replace(&fogManifest); } @@ -1071,7 +1071,7 @@ MPD* PrivateCDAIObjectMPD::GetAdMPD(std::string &manifestUrl, bool &finalManifes if (AampLogManager::isLogLevelAllowed(eLOGLEVEL_TRACE)) { // use printf to avoid 2048 char syslog limitation - printf("***Ad manifest***:\n\n%.*s\n", (int)manifest.size(), manifest.GetPtr() ); + printf("***Ad manifest***:\n\n%.*s\n", (int)manifest.size(), reinterpret_cast(manifest.data()) ); } manifest.Free(); } diff --git a/drm/DrmInterface.cpp b/drm/DrmInterface.cpp index e2759e9a0c..3f6f7372b2 100644 --- a/drm/DrmInterface.cpp +++ b/drm/DrmInterface.cpp @@ -166,7 +166,7 @@ void DrmInterface::ProfileUpdateDrmDecrypt(bool type, int bucketType) void DrmInterface::GetAccessKey(std::string &keyURI, std::string& tempEffectiveUrl, int& http_error, double& downloadTime,unsigned int curlInstance, bool &keyAcquisitionStatus, int &failureReason, char** ptr) { bool fetched = mpAamp->GetFile(keyURI, (AampMediaType)eMEDIATYPE_LICENCE, &mAesKeyBuf, tempEffectiveUrl, &http_error, &downloadTime, NULL, curlInstance, true); - *ptr =mAesKeyBuf.GetPtr(); + *ptr = reinterpret_cast(mAesKeyBuf.data()); if (fetched) { diff --git a/fragmentcollector_hls.cpp b/fragmentcollector_hls.cpp index 166f55c372..e37ee3c2d7 100644 --- a/fragmentcollector_hls.cpp +++ b/fragmentcollector_hls.cpp @@ -506,7 +506,7 @@ AAMPStatusType StreamAbstractionAAMP_HLS::ParseMainManifest() streamInfoStore.clear(); mediaInfoStore.clear(); - lstring iter = lstring(mainManifest.GetPtr(),mainManifest.size()); + lstring iter = lstring(reinterpret_cast(mainManifest.data()),mainManifest.size()); while( !iter.empty() ) { lstring ptr = iter.mystrpbrk(); @@ -793,7 +793,7 @@ lstring TrackState::GetIframeFragmentUriFromIndex(bool &bSegmentRepeated) while (fragmentInfo.startswith('#')) { const char *fragmentPtr = fragmentInfo.getPtr(); - size_t offs = fragmentPtr - playlist.GetPtr(); + size_t offs = fragmentPtr - reinterpret_cast(playlist.data()); lstring iter( fragmentPtr, playlist.size() - offs ); fragmentInfo = iter.mystrpbrk(); // #EXTINF fragmentInfo = iter.mystrpbrk(); // #EXT-X-BYTERANGE (or url) @@ -861,7 +861,7 @@ lstring TrackState::GetNextFragmentUriFromPlaylist(bool& reloadUri, bool ignoreD auto p = fragmentURI.getPtr(); auto l = playlist.size(); - size_t offs = p - playlist.GetPtr(); + size_t offs = p - reinterpret_cast(playlist.data()); if( offs>=l ) return rc; lstring iter( p, l-offs ); @@ -1146,7 +1146,7 @@ lstring TrackState::GetNextFragmentUriFromPlaylist(bool& reloadUri, bool ignoreD */ lstring TrackState::FindMediaForSequenceNumber() { - lstring iter = lstring( playlist.GetPtr(), playlist.size() ); + lstring iter = lstring( reinterpret_cast(playlist.data()), playlist.size() ); long long mediaSequenceNumber = nextMediaSequenceNumber - 1; std::string key; lstring initFragment; @@ -1322,7 +1322,7 @@ bool TrackState::FetchFragmentHelper(int &http_error, bool &decryption_error, bo cachedFragment->discontinuityIndex = 0; if( ISCONFIGSET(eAAMPConfig_HlsTsEnablePTSReStamp) ) { // TODO: optimize me - lstring iter = lstring( playlist.GetPtr(), playlist.size() ); + lstring iter = lstring( reinterpret_cast(playlist.data()), playlist.size() ); while( !iter.empty() ) { lstring ptr = iter.mystrpbrk(); @@ -1928,7 +1928,7 @@ void TrackState::IndexPlaylist(bool IsRefresh, AampTime &culledSec) FlushIndex(); mIndexingInProgress = true; - lstring iter = lstring(playlist.GetPtr(),playlist.size()); + lstring iter = lstring(reinterpret_cast(playlist.data()),playlist.size()); if( !iter.empty() ){ lstring ptr = iter.mystrpbrk(); if( !ptr.equal("#EXTM3U") ) @@ -2365,7 +2365,7 @@ void TrackState::ProcessPlaylist(AampGrowableBuffer& newPlaylist, int http_error } else { - lstring iter( playlist.GetPtr(),playlist.size() ); + lstring iter( reinterpret_cast(playlist.data()),playlist.size() ); fragmentURI = iter.mystrpbrk(); playlistPosition = -1; } @@ -2609,7 +2609,7 @@ std::string StreamAbstractionAAMP_HLS::GetPlaylistURI(TrackType trackType ) StreamOutputFormat GetFormatFromFragmentExtension( const AampGrowableBuffer &playlist ) { StreamOutputFormat format = FORMAT_INVALID; - lstring iter(playlist.GetPtr(),playlist.size()); + lstring iter(reinterpret_cast(playlist.data()),playlist.size()); while( !iter.empty() ) { lstring ptr = iter.mystrpbrk(); @@ -3299,7 +3299,7 @@ AAMPStatusType StreamAbstractionAAMP_HLS::Init(TuneType tuneType) { if( AampLogManager::isLogLevelAllowed(eLOGLEVEL_TRACE) ) { // use printf to avoid 2048 char syslog limitation - printf("***Main Manifest***:\n\n%.*s\n************\n", (int)this->mainManifest.size(), this->mainManifest.GetPtr()); + printf("***Main Manifest***:\n\n%.*s\n************\n", (int)this->mainManifest.size(), reinterpret_cast(this->mainManifest.data())); } AampDRMLicenseManager *licenseManager = aamp->mDRMLicenseManager; @@ -3338,7 +3338,7 @@ AAMPStatusType StreamAbstractionAAMP_HLS::Init(TuneType tuneType) if(mainManifestResult == eAAMPSTATUS_MANIFEST_CONTENT_ERROR || mainManifestResult == eAAMPSTATUS_MANIFEST_PARSE_ERROR) { // use printf to avoid 2048 char syslog limitation // Dump the invalid manifest content before reporting error - printf("ERROR: Invalid Main Manifest : %.*s\n", (int)this->mainManifest.size(), this->mainManifest.GetPtr() ); + printf("ERROR: Invalid Main Manifest : %.*s\n", (int)this->mainManifest.size(), reinterpret_cast(this->mainManifest.data()) ); return mainManifestResult; } } @@ -3597,7 +3597,7 @@ AAMPStatusType StreamAbstractionAAMP_HLS::Init(TuneType tuneType) bool playContextConfigured = false; if( AampLogManager::isLogLevelAllowed(eLOGLEVEL_TRACE) ) { // use printf to avoid 2048 char syslog limitation - printf("***Initial Playlist:******\n\n%.*s\n*****************\n", (int)ts->playlist.size(), ts->playlist.GetPtr() ); + printf("***Initial Playlist:******\n\n%.*s\n*****************\n", (int)ts->playlist.size(), reinterpret_cast(ts->playlist.data()) ); } // Flag also denotes if first encrypted init fragment was pushed or not ts->mCheckForInitialFragEnc = true; //force encrypted header at the start @@ -3657,7 +3657,7 @@ AAMPStatusType StreamAbstractionAAMP_HLS::Init(TuneType tuneType) aamp->UpdateRefreshPlaylistInterval(maxIntervalBtwPlaylistUpdateMs/1000.0); } - lstring iter = lstring(ts->playlist.GetPtr(),ts->playlist.size()); + lstring iter = lstring(reinterpret_cast(ts->playlist.data()),ts->playlist.size()); ts->fragmentURI = iter.mystrpbrk(); StreamOutputFormat format = GetFormatFromFragmentExtension(ts->playlist); if (FORMAT_ISO_BMFF == format) @@ -4560,8 +4560,7 @@ void TrackState::SwitchSubtitleTrack() aamp->DisableMediaDownloads(playlistMediaType); // Abort playlist timed wait for immediate download. AbortWaitForPlaylistDownload(); - // Notify that fragment collector is waiting - NotifyFragmentCollectorWait(); + // Wait for manifest update to get the new subtitle playlist downloaded WaitForManifestUpdate(); } else @@ -4679,8 +4678,7 @@ void TrackState::RunFetchLoop() aamp->DisableMediaDownloads(playlistMediaType); // Abort playlist timed wait for immediate download. AbortWaitForPlaylistDownload(); - // Notify that fragment collector is waiting - NotifyFragmentCollectorWait(); + // Wait for manifest update to get the new playlist downloaded WaitForManifestUpdate(); } else @@ -4717,7 +4715,7 @@ void TrackState::RunFetchLoop() { // Notify that fragment collector is waiting AAMPLOG_INFO("EOS wait for playlist refresh"); - NotifyFragmentCollectorWait(); + // Wait for playlist refresh WaitForManifestUpdate(); } @@ -5307,7 +5305,7 @@ bool StreamAbstractionAAMP_HLS::SetThumbnailTrack( int thumbIndex ) aamp->getAampCacheHandler()->InsertToPlaylistCache(streamInfo.uri, thumbnailManifest.GetVector(), tempEffectiveUrl,false,eMEDIATYPE_PLAYLIST_IFRAME); if( ContentType_SLE != type && ContentType_LINEAR != type ) { - lstring iter = lstring(thumbnailManifest.GetPtr(), thumbnailManifest.size()); + lstring iter = lstring(reinterpret_cast(thumbnailManifest.data()), thumbnailManifest.size()); indexedTileInfo = IndexThumbnails( iter ); rc = !indexedTileInfo.empty(); } @@ -5334,7 +5332,7 @@ bool StreamAbstractionAAMP_HLS::SetThumbnailTrack( int thumbIndex ) void StreamAbstractionAAMP_HLS::HandleSleThumbnailData(double tStart, double tEnd) { std::vector newIndexedTileInfo; - lstring thumbNailIter = lstring(thumbnailManifest.GetPtr(),thumbnailManifest.size()); + lstring thumbNailIter = lstring(reinterpret_cast(thumbnailManifest.data()),thumbnailManifest.size()); if(!aamp->mThumbnailLastProgramDateTime ) { //First Time; @@ -5585,7 +5583,7 @@ DrmReturn TrackState::DrmDecrypt( CachedFragment * cachedFragment, ProfilerBucke } if(mDrm) { - drmReturn = mDrm->Decrypt(bucketTypeFragmentDecrypt, cachedFragment->fragment.GetPtr(), + drmReturn = mDrm->Decrypt(bucketTypeFragmentDecrypt, cachedFragment->fragment.data(), cachedFragment->fragment.size(), MAX_LICENSE_ACQ_WAIT_TIME); } @@ -5698,7 +5696,7 @@ void TrackState::UpdateDrmCMSha1Hash( const std::string &newSha1Hash ) AAMPLOG_MIL("drmMetadataNode[%d].sha1Hash = %s", j, drmMetadataNode.sha1Hash.c_str() ); } // use printf to avoid 2048 char syslog limitation - printf("***playlist***:\n\n%.*s\n************\n", (int)playlist.size(), playlist.GetPtr()); + printf("***playlist***:\n\n%.*s\n************\n", (int)playlist.size(), reinterpret_cast(playlist.data())); assert(false); } } @@ -6423,7 +6421,7 @@ void TrackState::FindTimedMetadata(bool reportBulkMeta, bool bInitCall) AampTime totalDuration{}; if (ISCONFIGSET(eAAMPConfig_EnableSubscribedTags) && (eTRACK_VIDEO == type)) { - lstring iter = lstring(playlist.GetPtr(),playlist.size()); + lstring iter = lstring(reinterpret_cast(playlist.data()),playlist.size()); if( !iter.empty() ) { lstring ptr = iter.mystrpbrk(); @@ -7003,7 +7001,8 @@ void StreamAbstractionAAMP_HLS::RefreshTrack(AampMediaType type) aamp->mDisableRateCorrection = true; if(aamp->IsLive() && !track->seamlessAudioSwitchInProgress) { - track->AbortFragmentDownloaderWait(); + // Abort ongoing wait for playlist refresh, so the track change can be processed immediately. + track->AbortWaitForManifestUpdate(); } } } @@ -7040,8 +7039,7 @@ void TrackState::SwitchAudioTrack() // Abort ongoing playlist download or wait for refresh if any. aamp->DisableMediaDownloads(playlistMediaType); AbortWaitForPlaylistDownload(); - // Notify that fragment collector is waiting - NotifyFragmentCollectorWait(); + // Wait for manifest update to get the new audio playlist downloaded WaitForManifestUpdate(); } else @@ -7351,7 +7349,7 @@ void TrackState::getNextFetchRequestUri( void ) auto ptr = fragmentURI.getPtr(); if( ptr ) { - size_t offs = ptr - playlist.GetPtr(); + size_t offs = ptr - reinterpret_cast(playlist.data()); lstring iter( ptr, playlist.size() - offs ); while( !iter.empty() ) { diff --git a/fragmentcollector_mpd.cpp b/fragmentcollector_mpd.cpp index 3ff3119ccd..8e8b10a112 100644 --- a/fragmentcollector_mpd.cpp +++ b/fragmentcollector_mpd.cpp @@ -1616,7 +1616,7 @@ bool StreamAbstractionAAMP_MPD::PushNextFragment( class MediaStreamContext *pMed { unsigned int firstOffset; ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), 0, NULL, @@ -1633,7 +1633,7 @@ bool StreamAbstractionAAMP_MPD::PushNextFragment( class MediaStreamContext *pMed for (int i = 0; i < pMediaStreamContext->fragmentIndex; i++) { if (ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), i, &referenced_size, @@ -1650,7 +1650,7 @@ bool StreamAbstractionAAMP_MPD::PushNextFragment( class MediaStreamContext *pMed unsigned int referenced_size; float fragmentDuration; if (ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), pMediaStreamContext->fragmentIndex++, &referenced_size, @@ -1664,7 +1664,7 @@ bool StreamAbstractionAAMP_MPD::PushNextFragment( class MediaStreamContext *pMed float nextfragmentDuration; uint64_t nextfragmentOffset; if (ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), pMediaStreamContext->fragmentIndex, &nextReferencedSize, @@ -2446,7 +2446,7 @@ double StreamAbstractionAAMP_MPD::SkipFragments( MediaStreamContext *pMediaStrea while ((fragmentTime < skipTime + presentationTimeOffsetSec ) || skipToEnd) { if (ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), fragmentIndex++, &referenced_size, @@ -2742,7 +2742,7 @@ AAMPStatusType StreamAbstractionAAMP_MPD::GetMPDFromManifest( ManifestDownloadRe aamp->SetManifestUrl(locationUrl[0].c_str()); } - mLastPlaylistDownloadTimeMs = aamp_GetCurrentTimeMS(); + mLastPlaylistDownloadTimeMs = mpdDnldResp->mLastPlaylistDownloadTimeMs; if(mIsLiveStream && ISCONFIGSET(eAAMPConfig_EnableClientDai)) { mCdaiObject->PlaceAds(mMPDParseHelper); @@ -4419,6 +4419,8 @@ void StreamAbstractionAAMP_MPD::MPDUpdateCallbackExec() AAMPLOG_ERR("manifest download failed"); } } + // Inform fetch loop to proceed with the new manifest + AbortWaitForManifestUpdate(); } /** @@ -8276,7 +8278,7 @@ void StreamAbstractionAAMP_MPD::FetchAndInjectInitialization(int trackIdx, bool unsigned int referenced_size; float fragmentDuration; if (ParseSegmentIndexBox( - pMediaStreamContext->IDX.GetPtr(), + pMediaStreamContext->IDX.data(), pMediaStreamContext->IDX.size(), pMediaStreamContext->fragmentIndex, &referenced_size, @@ -9183,9 +9185,13 @@ bool StreamAbstractionAAMP_MPD::SelectSourceOrAdPeriod(bool &periodChanged, bool // Update manifest and check for period validity in the next iteration // For CDAI empty period at the end, we should re-iterate the loop AAMPLOG_WARN("Period ID not changed WaitForManifestUpdate"); + // Snapshot counter before UpdateMPD(); if AbortWaitForManifestUpdate() + // fires inside UpdateMPD() the predicate will fire immediately. + uint32_t snapshotCounter = GetManifestUpdateCounter(); if (AAMPStatusType::eAAMPSTATUS_OK != UpdateMPD()) { - aamp->interruptibleMsSleep(500); // Sleep for 500ms to avoid tight looping + // Wait for manifest update and check for period change again + WaitForManifestUpdate(snapshotCounter); } mpdChanged = true; ret = false; @@ -9521,14 +9527,21 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() break; } } + // Adbreak placement needs to be completed before we can move to next period + // so wait for manifest update and check for period change again. + // Snapshot counter before UpdateMPD() so a concurrent AbortWaitForManifestUpdate() + // inside UpdateMPD() is not missed. + uint32_t snapshotCounter = GetManifestUpdateCounter(); AAMPStatusType ret = UpdateMPD(); if (eAAMPSTATUS_MANIFEST_DOWNLOAD_ERROR == ret) { + // For download error, wait for manifest update to avoid unnecessary looping. AAMPLOG_TRACE("Wait for manifest refresh"); - aamp->interruptibleMsSleep(MAX_WAIT_TIMEOUT_MS); + WaitForManifestUpdate(snapshotCounter); } else if (eAAMPSTATUS_MANIFEST_CONTENT_ERROR == ret) { + // For content error, disable downloads and exit fetcher loop. aamp->DisableDownloads(); AAMPLOG_WARN("Exiting from fetcher loop due to manifest content error"); break; @@ -9542,14 +9555,20 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() } else if(mIterPeriodIndex >= mNumberOfPeriods) { + // New period is not available yet, so wait for manifest update and check for period change again. + // Snapshot counter before UpdateMPD() so a concurrent AbortWaitForManifestUpdate() + // inside UpdateMPD() is not missed. + uint32_t snapshotCounter = GetManifestUpdateCounter(); AAMPStatusType ret = UpdateMPD(); if (eAAMPSTATUS_MANIFEST_DOWNLOAD_ERROR == ret) { + // For download error, wait for manifest update to avoid unnecessary looping. AAMPLOG_TRACE("Wait for manifest refresh"); - aamp->interruptibleMsSleep(MAX_WAIT_TIMEOUT_MS); + WaitForManifestUpdate(snapshotCounter); } else if (eAAMPSTATUS_MANIFEST_CONTENT_ERROR == ret) { + // For content error, disable downloads and exit fetcher loop. aamp->DisableDownloads(); AAMPLOG_WARN("Exiting from fetcher loop due to manifest content error"); break; @@ -9739,6 +9758,8 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() // TODO: This is required now as we profile ABR from current period, after decoupling the ABR // dependency by saving period based profile data, this wait can be removed. + // Snapshot before ShouldWaitForFragments() which calls UpdateMPD() internally; + uint32_t snapshotCounter = GetManifestUpdateCounter(); // Wait for pending fragment downloads if necessary if (ShouldWaitForFragments()) { @@ -9762,7 +9783,9 @@ void StreamAbstractionAAMP_MPD::FetcherLoop() // Decide whether to wait or exit if (IsAtLiveEdge() && mCdaiObject->mAdState != AdState::IN_ADBREAK_WAIT2CATCHUP) { - aamp->interruptibleMsSleep(500); + // At live edge, wait for manifest update to get new segments or period. + // Avoids tight looping. + WaitForManifestUpdate(snapshotCounter); } else { @@ -9959,7 +9982,10 @@ void StreamAbstractionAAMP_MPD::TsbReader() break; } AAMPLOG_TRACE("EOS from both tracks - Wait for next fragment"); - aamp->interruptibleMsSleep(500); + // Snapshot counter before waiting. If AbortWaitForManifestUpdate() + // fired between EOS detection and here, the predicate fires + // immediately and we re-enter the loop to check for new segments. + WaitForManifestUpdate(GetManifestUpdateCounter()); } if(cacheFullStatus[eMEDIATYPE_VIDEO] || (vEOS && !aEOS)) { @@ -10020,8 +10046,6 @@ AAMPStatusType StreamAbstractionAAMP_MPD::UpdateMPD(bool init) if(mIsLiveManifest) { // let all the track threads to pause for manifest update - //playlistDownloaderContext->NotifyFragmentCollectorWait(); - //playlistDownloaderContext->WaitForManifestUpdate(); AampMPDDownloader *dnldInstance = aamp->GetMPDDownloader(); // Get the Manifest with a wait of Manifest Timeout time ManifestDownloadResponsePtr tmpManifestDnldRespPtr ; @@ -10437,6 +10461,8 @@ void StreamAbstractionAAMP_MPD::Stop(bool clearChannelData) if (!aamp->DownloadsAreEnabled()) { aamp->GetAampTrackWorkerManager()->StopWorkers(); + // Signal collector thread to exit wait for manifest update if waiting + AbortWaitForManifestUpdate(); if (fragmentCollectorThreadID.joinable()) { fragmentCollectorThreadID.join(); @@ -10448,11 +10474,12 @@ void StreamAbstractionAAMP_MPD::Stop(bool clearChannelData) { AAMPLOG_INFO("Abort TsbReader"); abortTsbReader = true; + // Signal TsbReader thread to exit wait for manifest update if waiting + AbortWaitForManifestUpdate(); tsbReaderThreadID.join(); AAMPLOG_INFO("Joined tsbReaderThreadID"); } - for (int iTrack = 0; iTrack < mMaxTracks; iTrack++) { MediaStreamContext *track = mMediaStreamContext[iTrack]; @@ -10495,9 +10522,10 @@ void StreamAbstractionAAMP_MPD::Stop(bool clearChannelData) if(tsbReaderThreadID.joinable()) { abortTsbReader = true; + // Signal TsbReader thread to exit wait for manifest update if waiting + AbortWaitForManifestUpdate(); tsbReaderThreadID.join(); } - } if (!aamp->DownloadsAreEnabled()) @@ -13850,6 +13878,14 @@ void StreamAbstractionAAMP_MPD::UpdateMPDPeriodDetails(std::vector& { durMs += periodInfo.duration; } + if (mLowLatencyMode && iter == periods.size() - 1) + { + // Lets see how far behind the live edge is compared to wall clock time. Also lets compare against publish time + double manifestEndDelta = ((double)mLastPlaylistDownloadTimeMs / 1000.00) - periodInfo.periodEndTime; + AAMPLOG_DEBUG("LLD manifest publishTime:%lf periodEndTime:%lf wallClockTime:%lf manifestEndDelta:%lf", + mMPDParseHelper->GetPublishTime(), periodInfo.periodEndTime, + (double)mLastPlaylistDownloadTimeMs / 1000.00, manifestEndDelta); + } } } @@ -14353,3 +14389,78 @@ bool StreamAbstractionAAMP_MPD::IsEmptyPeriod(int iPeriodIndex) const { return mMPDParseHelper->IsEmptyPeriod(iPeriodIndex, ShouldCheckOnlyIframeAdaptation()); } + +/** + * @fn GetManifestUpdateCounter + * @brief Returns the current manifest update counter from the video track. + * Callers should snapshot this before performing any check or download work + * that leads to WaitForManifestUpdate(sinceGeneration). + */ +uint32_t StreamAbstractionAAMP_MPD::GetManifestUpdateCounter() +{ + MediaTrack *video = GetMediaTrack(eTRACK_VIDEO); + if (video) + { + return video->GetManifestUpdateCounter(); + } + AAMPLOG_WARN("BUG! Video track is not available to get manifest update counter"); + return 0; +} + +/** + * @fn WaitForManifestUpdate + * @brief Waits for the manifest update to complete by waiting on the video track's manifest update condition variable. + */ +void StreamAbstractionAAMP_MPD::WaitForManifestUpdate() +{ + // Let's use video as the primary track for synchronizing manifest updates. + // Compared to HLS, there is only one fetcher thread for DASH for video, audio and subtitle tracks. + MediaTrack *video = GetMediaTrack(eTRACK_VIDEO); + if (video) + { + video->WaitForManifestUpdate(); + } + else + { + // Video should be always present. In case of audio-only playback, it's currently masked as a video track. + AAMPLOG_WARN("BUG! Video track is not available to wait for manifest update"); + } +} + +/** + * @fn WaitForManifestUpdate (sinceGeneration overload) + * @brief Waits until the update counter advances past snapshotCounter. + * If AbortWaitForManifestUpdate() already ran after the snapshot was + * taken, the predicate fires immediately — no lost-wakeup. + * @param[in] snapshotCounter The manifest update counter snapshot taken before the update that triggers this wait. + */ +void StreamAbstractionAAMP_MPD::WaitForManifestUpdate(uint32_t snapshotCounter) +{ + MediaTrack *video = GetMediaTrack(eTRACK_VIDEO); + if (video) + { + video->WaitForManifestUpdate(snapshotCounter); + } + else + { + AAMPLOG_WARN("BUG! Video track is not available to wait for manifest update"); + } +} + +/** + * @fn AbortWaitForManifestUpdate + * @brief Aborts waiting for the manifest update by signaling the video track's manifest update condition variable. + */ +void StreamAbstractionAAMP_MPD::AbortWaitForManifestUpdate() +{ + MediaTrack *video = GetMediaTrack(eTRACK_VIDEO); + if (video) + { + video->AbortWaitForManifestUpdate(); + } + else + { + // Video should be always present. In case of audio-only playback, it's currently masked as a video track. + AAMPLOG_WARN("BUG! Video track is not available to abort wait for manifest update"); + } +} \ No newline at end of file diff --git a/fragmentcollector_mpd.h b/fragmentcollector_mpd.h index 4bde994546..c637f883d3 100644 --- a/fragmentcollector_mpd.h +++ b/fragmentcollector_mpd.h @@ -1276,6 +1276,41 @@ class StreamAbstractionAAMP_MPD : public StreamAbstractionAAMP */ bool IsEmptyPeriod(int iPeriodIndex) const; + /** + * @fn GetManifestUpdateCounter + * @brief Returns the current manifest update counter. + * Snapshot this BEFORE any check or download work that might + * lead to WaitForManifestUpdate(snapshotCounter), so that a + * concurrent AbortWaitForManifestUpdate() cannot be missed. + */ + uint32_t GetManifestUpdateCounter(); + + /** + * @fn WaitForManifestUpdate + * @brief Wait for manifest to be updated. + * Called when the fetcher loop is waiting for the next manifest update. + * This is used to avoid tight looping in fetcher loop and also to sync the manifest update and fetcher loop. + */ + void WaitForManifestUpdate(); + + /** + * @fn WaitForManifestUpdate + * @brief Overload accepting a caller-supplied counter snapshot. + * Blocks until the counter advances past snapshotCounter. + * Handles the case where AbortWaitForManifestUpdate() fires between + * the snapshot and the wait call — no lost-wakeup. + * @param[in] snapshotCounter Snapshot from GetManifestUpdateCounter() + * taken before the caller's check or download work. + */ + void WaitForManifestUpdate(uint32_t snapshotCounter); + + /** + * @fn AbortWaitForManifestUpdate + * @brief Abort waiting for manifest update. + * This is used to stop the fetcher loop from waiting either on a manifest update or tear down. + */ + void AbortWaitForManifestUpdate(); + std::vector thumbnailtrack; std::vector indexedTileInfo; double mFirstPeriodStartTime; /*< First period start time for progress report*/ diff --git a/isobmff/isobmffbuffer.cpp b/isobmff/isobmffbuffer.cpp index 159a308401..653e3c3438 100644 --- a/isobmff/isobmffbuffer.cpp +++ b/isobmff/isobmffbuffer.cpp @@ -62,6 +62,20 @@ void IsoBmffBuffer::setBuffer(uint8_t* buffer, size_t bufferLen) this->bufSize = bufferLen; } +/** + * @brief Set buffer from a read-only pointer and size. + * The const_cast here is intentional and contained: the internal + * member must remain uint8_t* to support mutating operations on + * non-const buffers. Callers using this overload must ensure that + * no mutating IsoBmffBuffer operations (e.g. restampPTS) are + * subsequently invoked. + */ +void IsoBmffBuffer::setBuffer(const uint8_t* buffer, size_t bufferLen) +{ + this->buffer = const_cast(buffer); + this->bufSize = bufferLen; +} + /** * @fn ParseChunkData * @param[in] name - name of the track @@ -1319,4 +1333,4 @@ int IsoBmffBuffer::getLastMdatBoxIndex() const } } return index; -} \ No newline at end of file +} diff --git a/isobmff/isobmffbuffer.h b/isobmff/isobmffbuffer.h index b2c77ebd8d..460570cc9f 100644 --- a/isobmff/isobmffbuffer.h +++ b/isobmff/isobmffbuffer.h @@ -206,6 +206,19 @@ class IsoBmffBuffer */ void setBuffer(uint8_t* buffer, size_t bufferLen); + /** + * @fn setBuffer + * + * @brief Read-only overload for callers that hold a const pointer (e.g. a + * const std::vector). The buffer will only be read by + * parsing operations; mutating operations (e.g. restampPTS) must + * not be called after using this overload. + * @param[in] buffer - read-only buffer pointer + * @param[in] bufferLen - buffer length in bytes + * @return void + */ + void setBuffer(const uint8_t* buffer, size_t bufferLen); + /** * @fn parseBuffer * diff --git a/isobmff/isobmffhelper.cpp b/isobmff/isobmffhelper.cpp index d26e690539..f834d704f2 100644 --- a/isobmff/isobmffhelper.cpp +++ b/isobmff/isobmffhelper.cpp @@ -33,7 +33,7 @@ bool IsoBmffHelper::ConvertToKeyFrame(AampGrowableBuffer &buffer) bool retval{true}; IsoBmffBuffer isoBmffBuffer{}; - isoBmffBuffer.setBuffer(reinterpret_cast(buffer.GetPtr()), buffer.size() ); + isoBmffBuffer.setBuffer(buffer.data(), buffer.size() ); if(isoBmffBuffer.parseBuffer()) { @@ -53,7 +53,7 @@ bool IsoBmffHelper::RestampPts(AampGrowableBuffer &buffer, int64_t ptsOffset, st bool retval{false}; IsoBmffBuffer isoBmffBuffer{}; - isoBmffBuffer.setBuffer(reinterpret_cast(buffer.GetPtr()), buffer.size() ); + isoBmffBuffer.setBuffer(buffer.data(), buffer.size() ); if (!isoBmffBuffer.parseBuffer()) { @@ -79,7 +79,7 @@ bool IsoBmffHelper::SetTimescale(AampGrowableBuffer &buffer, uint32_t timeScale) bool retval{false}; IsoBmffBuffer isoBmffBuffer{}; - isoBmffBuffer.setBuffer(reinterpret_cast(buffer.GetPtr()), buffer.size()); + isoBmffBuffer.setBuffer(buffer.data(), buffer.size()); if (!isoBmffBuffer.parseBuffer()) { @@ -98,7 +98,7 @@ bool IsoBmffHelper::SetPtsAndDuration(AampGrowableBuffer &buffer, uint64_t pts, bool retval{false}; IsoBmffBuffer isoBmffBuffer{}; - isoBmffBuffer.setBuffer(reinterpret_cast(buffer.GetPtr()), buffer.size()); + isoBmffBuffer.setBuffer(buffer.data(), buffer.size()); if (!isoBmffBuffer.parseBuffer()) { @@ -118,7 +118,7 @@ bool IsoBmffHelper::ClearMediaHeaderDuration(AampGrowableBuffer &buffer) bool retval{false}; IsoBmffBuffer isoBmffBuffer{}; - isoBmffBuffer.setBuffer(reinterpret_cast(buffer.GetPtr()), buffer.size()); + isoBmffBuffer.setBuffer(buffer.data(), buffer.size()); if (!isoBmffBuffer.parseBuffer()) { diff --git a/isobmff/isobmffprocessor.cpp b/isobmff/isobmffprocessor.cpp index 41a5bec3d8..5a4dcf7314 100644 --- a/isobmff/isobmffprocessor.cpp +++ b/isobmff/isobmffprocessor.cpp @@ -589,7 +589,7 @@ void IsoBmffProcessor::restampPTSAndSendSegment(AampGrowableBuffer *pBuffer,doub } //Step 6.Now time to restamp the PTS - buffer.restampPTS(sumPTS,currentPTS,(uint8_t *)(pBuffer->GetPtr()),(uint32_t)(pBuffer->size())); + buffer.restampPTS(sumPTS,currentPTS,pBuffer->data(),static_cast(pBuffer->size())); double newPos = ((double)sumPTS / (double) currTimeScale); prevPTS = currentPTS; diff --git a/main_aamp.cpp b/main_aamp.cpp index 3257b127b9..cf1212bb8a 100755 --- a/main_aamp.cpp +++ b/main_aamp.cpp @@ -2811,7 +2811,27 @@ std::string PlayerInstanceAAMP::GetAppName() } /** - * @brief Enable/disable the native CC rendering feature + * @brief Enable or disable AAMP-managed CC rendering. + * + * When enable is true, AAMP takes ownership of the CC rendering lifecycle + * via PlayerCCManager: initialization on first frame, trickplay muting, + * parental control gating (SERVICE_PIN_LOCKED events), CEA-608/708 track + * selection, and session teardown on stop. + * + * When enable is false (the default), AAMP does not drive CC rendering + * behaviour or policy decisions (e.g. trickplay muting, parental-control + * integration, or CC-specific teardown). Internal components such as + * PlayerCCManager may still be initialised but internally it will be + * using PlayerFakeCCManager. + * + * This is the correct setting for X1 platforms where XREReceiver controls + * CC independently of AAMP. Enabling it on X1 would cause AAMP to overlap + * with XREReceiver's CC management responsibilities. + * + * Must be called before Tune() to take effect for a given session. + * + * @param[in] enable true — AAMP manages CC (platforms without XREReceiver) + * false — external controller manages CC (X1 / XREReceiver) */ void PlayerInstanceAAMP::SetNativeCCRendering(bool enable) { diff --git a/mp4demux/AampMp4Demuxer.cpp b/mp4demux/AampMp4Demuxer.cpp index 52c23f5cad..2d21834b72 100644 --- a/mp4demux/AampMp4Demuxer.cpp +++ b/mp4demux/AampMp4Demuxer.cpp @@ -64,10 +64,10 @@ bool AampMp4Demuxer::sendSegment(AampGrowableBuffer* pBuffer, double position, d { bool ret = true; (void) processor; - if (mMp4Demux.get() && pBuffer && pBuffer->GetPtr() && pBuffer->size()) + if (mMp4Demux.get() && pBuffer && pBuffer->data() && pBuffer->size()) { AAMPLOG_INFO("Processing segment with type:%d position: %f, duration: %f, isInit: %d", mMediaType, position, duration, isInit); - ret = mMp4Demux->Parse(pBuffer->GetPtr(), pBuffer->size()); + ret = mMp4Demux->Parse(pBuffer->data(), pBuffer->size()); if (!ret) { AAMPLOG_ERR("Failed to parse MP4 segment [err:%d] for type:%d position: %f, duration: %f, isInit: %d", mMp4Demux->GetLastError(), mMediaType, position, duration, isInit); @@ -102,7 +102,7 @@ bool AampMp4Demuxer::sendSegment(AampGrowableBuffer* pBuffer, double position, d } else { - AAMPLOG_ERR("Demuxer instance(%p) is invalid or buffer invalid (%p, %p, %zu)", mMp4Demux.get(), pBuffer, pBuffer ? pBuffer->GetPtr() : nullptr, pBuffer ? pBuffer->size() : 0); + AAMPLOG_ERR("Demuxer instance(%p) is invalid or buffer invalid (%p, %p, %zu)", mMp4Demux.get(), pBuffer, pBuffer ? pBuffer->data() : nullptr, pBuffer ? pBuffer->size() : 0); ret = false; } ptsError = false; diff --git a/priv_aamp.cpp b/priv_aamp.cpp index ffc42e27ff..4b410f2fcf 100644 --- a/priv_aamp.cpp +++ b/priv_aamp.cpp @@ -724,7 +724,7 @@ static bool IdentifyMp4ChunkBoundary(AampMediaType type, AampGrowableBuffer *buf chunkDurationInTicks = 0; IsoBmffBuffer isobmffBuffer; - isobmffBuffer.setBuffer(reinterpret_cast(buffer->GetPtr()) + bufferOffset, buffer->size() - bufferOffset); + isobmffBuffer.setBuffer(buffer->data() + bufferOffset, buffer->size() - bufferOffset); try { @@ -1162,7 +1162,7 @@ size_t PrivateInstanceAAMP::HandleSSLWriteCallback ( char *ptr, size_t size, siz } if (ret > 0) { - const char *bufferPtr = reinterpret_cast(context->buffer->data()) + context->bufferOffset; + const uint8_t *bufferPtr = context->buffer->data() + context->bufferOffset; if (context->chunkBoundary > context->bufferOffset) { size_t bufferLen = context->chunkBoundary - context->bufferOffset; @@ -5203,7 +5203,7 @@ bool PrivateInstanceAAMP::GetFile( std::string remoteUrl, AampMediaType mediaTyp } if (buffer->capacity() != 0) { - if(aamp_WriteFile(remoteUrl, buffer->GetPtr(), buffer->size(), mediaType, mManifestRefreshCount,harvestPath.c_str())) + if(aamp_WriteFile(remoteUrl, reinterpret_cast(buffer->data()), buffer->size(), mediaType, mManifestRefreshCount,harvestPath.c_str())) mHarvestCountLimit--; } // CID:168113 - forward null } @@ -7065,7 +7065,7 @@ MediaFormat PrivateInstanceAAMP::GetMediaFormatType(const char *url) if(gotManifest) { - if(sniffedBytes.size() >= 7 && memcmp(sniffedBytes.GetPtr(), "#EXTM3U8", 7) == 0) + if(sniffedBytes.size() >= 7 && memcmp(sniffedBytes.data(), "#EXTM3U8", 7) == 0) { rc = eMEDIAFORMAT_HLS; } @@ -8932,6 +8932,19 @@ void PrivateInstanceAAMP::ScheduleRetune(PlaybackErrorType errorType, AampMediaT lock.unlock(); + // Take a local copy of the stream abstraction to check for tune + // NOTE - this copy is taken outside of the mutex protection as it is only used to see if it has changed while waiting + // for the mutex (i.e. checks to see if a 'retune' occurred). It should NOT be dereferenced as it may be invalid after the lock + const StreamAbstractionAAMP *oldStreamAbstraction = mpStreamAbstractionAAMP; + std::lock_guard guard(mStreamLock); // protect mpStreamAbstractionAAMP (this must cover calls to AdditionalTuneFailLogEntries) + + // Check that the stream abstraction has not changed while waiting for the mutex (we have not done some form of tune). + if (oldStreamAbstraction != mpStreamAbstractionAAMP) + { + AAMPLOG_WARN("PrivateInstanceAAMP: Ignore reTune due to stream changed"); + return; + } + if (mpStreamAbstractionAAMP && mpStreamAbstractionAAMP->IsStreamerStalled()) { AAMPLOG_WARN("PrivateInstanceAAMP: Ignore reTune due to playback stall"); @@ -9052,6 +9065,19 @@ void PrivateInstanceAAMP::ScheduleRetune(PlaybackErrorType errorType, AampMediaT //pipeline error during trickplay if(errorType == eGST_ERROR_GST_PIPELINE_INTERNAL) { + // Take a local copy of the stream abstraction to check for tune + // NOTE - this copy is taken outside of the mutex protection as it is only used to see if it has changed while waiting + // for the mutex (i.e. checks to see if a 'retune' occurred). It should NOT be dereferenced as it may be invalid after the lock + const StreamAbstractionAAMP *oldStreamAbstraction = mpStreamAbstractionAAMP; + std::lock_guard guard(mStreamLock); // protect mpStreamAbstractionAAMP (this must cover calls to AdditionalTuneFailLogEntries) + + // Check that the stream abstraction has not changed while waiting for the mutex (we have not done some form of tune). + if (oldStreamAbstraction != mpStreamAbstractionAAMP) + { + AAMPLOG_WARN("PrivateInstanceAAMP: Ignore reTune due to stream changed"); + return; + } + AAMPLOG_WARN("Processing retune for GstPipeline Internal Error and rate %f", rate); SendAnomalyEvent(ANOMALY_WARNING, "%s GstPipeline Internal Error", GetMediaTypeName(trackType)); gLock.lock(); diff --git a/streamabstraction.cpp b/streamabstraction.cpp index 1ebc080cc3..476626e2c1 100644 --- a/streamabstraction.cpp +++ b/streamabstraction.cpp @@ -102,7 +102,7 @@ void MediaTrack::StopPlaylistDownloaderThread() { abortPlaylistDownloader = true; AbortWaitForPlaylistDownload(); - AbortFragmentDownloaderWait(); + AbortWaitForManifestUpdate(); playlistDownloaderThread->join(); SAFE_DELETE(playlistDownloaderThread); AAMPLOG_WARN("[%s] Aborted", name); @@ -978,7 +978,7 @@ bool MediaTrack::ProcessFragmentChunk() } if (mSubtitleParser && type == eTRACK_SUBTITLE) { - mSubtitleParser->processData(cachedFragment->fragment.GetPtr(), cachedFragment->fragment.size(), cachedFragment->position, cachedFragment->duration); + mSubtitleParser->processData(reinterpret_cast(cachedFragment->fragment.data()), cachedFragment->fragment.size(), cachedFragment->position, cachedFragment->duration); } if (type != eTRACK_SUBTITLE || (aamp->IsGstreamerSubsEnabled())) { @@ -1009,7 +1009,7 @@ bool MediaTrack::ProcessFragmentChunk() IsoBmffBuffer isobuf; /**< Fragment Chunk buffer box parser*/ char *unParsedBuffer = NULL; size_t parsedBufferSize = 0, unParsedBufferSize = 0; - unParsedBuffer = unparsedBufferChunk.GetPtr(); + unParsedBuffer = reinterpret_cast(unparsedBufferChunk.data()); unParsedBufferSize = parsedBufferSize = unparsedBufferChunk.size(); isobuf.setBuffer(unparsedBufferChunk.GetVector()); AAMPLOG_TRACE("[%s] Unparsed Buffer Size: %zu", name,unparsedBufferChunk.size() ); @@ -1107,7 +1107,7 @@ bool MediaTrack::ProcessFragmentChunk() if (mSubtitleParser && type == eTRACK_SUBTITLE) { - mSubtitleParser->processData(parsedBufferChunk.GetPtr(), parsedBufferChunk.size(), fpts, fduration); + mSubtitleParser->processData(reinterpret_cast(parsedBufferChunk.data()), parsedBufferChunk.size(), fpts, fduration); } if (type != eTRACK_SUBTITLE || (aamp->IsGstreamerSubsEnabled())) { @@ -1408,7 +1408,7 @@ void MediaTrack::ProcessAndInjectFragment(CachedFragment *cachedFragment, bool f } if ((mSubtitleParser || (aamp->IsGstreamerSubsEnabled())) && type == eTRACK_SUBTITLE) { - auto ptr = cachedFragment->fragment.GetPtr(); + auto ptr = reinterpret_cast(cachedFragment->fragment.data()); auto len = cachedFragment->fragment.size(); if( ISCONFIGSET(eAAMPConfig_HlsTsEnablePTSReStamp) ) { @@ -1532,8 +1532,7 @@ bool MediaTrack::InjectFragment() } AAMPLOG_TRACE("[%s] - fragmentIdxToInject %d cachedFragment %p ptr %p", - name, fragmentIdxToInject, cachedFragment, cachedFragment->fragment.GetPtr()); - + name, fragmentIdxToInject, cachedFragment, cachedFragment->fragment.data()); if (cachedFragment->fragment.capacity() != 0) { // This is currently supported for non-LL DASH streams only at normal play rate @@ -1876,7 +1875,7 @@ CachedFragment* MediaTrack::GetFetchChunkBuffer(bool initialize) { if (cachedFragment->fragment.capacity() != 0) { - AAMPLOG_WARN("[%s] fragment.ptr[%p] already set - possible memory leak (len=[%zu],avail=[%zu])",name, cachedFragment->fragment.GetPtr(), cachedFragment->fragment.size(), cachedFragment->fragment.capacity() ); + AAMPLOG_WARN("[%s] fragment.ptr[%p] already set - possible memory leak (len=[%zu],avail=[%zu])",name, cachedFragment->fragment.data(), cachedFragment->fragment.size(), cachedFragment->fragment.capacity() ); } cachedFragment->fragment.clear(); } @@ -2048,8 +2047,8 @@ MediaTrack::MediaTrack(TrackType type, PrivateInstanceAAMP* aamp, const char* na mCachedFragmentChunks{}, unparsedBufferChunk{"unparsedBufferChunk"}, parsedBufferChunk{"parsedBufferChunk"}, fragmentChunkFetched(), fragmentChunkInjected(), maxCachedFragmentChunksPerTrack(0), noMDATCount(0), loadNewAudio(false), audioFragmentCached(), audioMutex(), loadNewSubtitle(false), subtitleFragmentCached(), subtitleMutex(), abortPlaylistDownloader(true), plDownloadWait() - ,dwnldMutex(), playlistDownloaderThread(NULL), fragmentCollectorWaitingForPlaylistUpdate(false) - ,frDownloadWait(),prevDownloadStartTime(-1) + ,dwnldMutex(), playlistDownloaderThread(NULL), mManifestUpdateCounter(0) + ,mManifestUpdateWait(),prevDownloadStartTime(-1) ,playContext(nullptr), seamlessAudioSwitchInProgress(false), lastInjectedPosition(0), lastInjectedDuration(0), seamlessSubtitleSwitchInProgress(false) ,mIsLocalTSBInjection(false), mCachedFragmentChunksSize(0) ,mIsoBmffHelper(std::make_shared()) @@ -4269,7 +4268,7 @@ void StreamAbstractionAAMP::DisablePlaylistDownloads() if (track && track->enabled) { track->AbortWaitForPlaylistDownload(); - track->AbortFragmentDownloaderWait(); + track->AbortWaitForManifestUpdate(); } } } @@ -4280,6 +4279,7 @@ void StreamAbstractionAAMP::DisablePlaylistDownloads() void MediaTrack::AbortWaitForPlaylistDownload() { std::unique_lock lock(dwnldMutex); + // This API is called to trigger an immediate playlist download after updating the playlist URL. if((playlistDownloaderThread) && (playlistDownloaderThread->joinable())) { plDownloadWait.notify_one(); @@ -4310,30 +4310,73 @@ void MediaTrack::EnterTimedWaitForPlaylistRefresh(int timeInMs) } /** - * @brief Abort fragment downloader wait + * @brief Abort wait for manifest update + * This function is called to signal the fragment collector thread to wake up from WaitForManifestUpdate. */ -void MediaTrack::AbortFragmentDownloaderWait() +void MediaTrack::AbortWaitForManifestUpdate() { - std::unique_lock lock(dwnldMutex); - if(fragmentCollectorWaitingForPlaylistUpdate) + std::lock_guard lock(dwnldMutex); + // Increment the update counter while holding the mutex, then wake all + // waiters. Each waiter holds its own snapshot of the previous value, so + // every thread that was blocked will find (live != snapshot) == true and + // proceed. No thread can "steal" the signal from another by resetting a + // shared flag. + ++mManifestUpdateCounter; + mManifestUpdateWait.notify_all(); +} + +/** + * @brief Return the current manifest update counter. + * The caller must snapshot this value BEFORE any check or download work + * that might cause it to decide to wait, then pass it to + * WaitForManifestUpdate(snapshotCounter). + */ +uint32_t MediaTrack::GetManifestUpdateCounter() +{ + std::lock_guard lock(dwnldMutex); + return mManifestUpdateCounter; +} + +/** + * @brief Wait for manifest update — caller-snapshot overload. + * Blocks until the counter advances past snapshotCounter. + * If AbortWaitForManifestUpdate() already ran after the snapshot was + * taken, the predicate is immediately true and wait() skips blocking. + * @param[in] snapshotCounter - the value that was snapshot before deciding to wait + */ +void MediaTrack::WaitForManifestUpdate(uint32_t snapshotCounter) +{ + if(aamp->DownloadsAreEnabled()) { - frDownloadWait.notify_one(); + std::unique_lock lock(dwnldMutex); + AAMPLOG_DEBUG("[%d] Waiting for manifest update (snapshotCounter=%u, currentCounter=%u)...", + type, snapshotCounter, mManifestUpdateCounter); + mManifestUpdateWait.wait(lock, [this, snapshotCounter] + { + return mManifestUpdateCounter != snapshotCounter; + }); + AAMPLOG_DEBUG("[%d] Manifest update received (counter=%u).", type, mManifestUpdateCounter); } } /** - * @brief Wait for playlist download and update + * @brief Wait for manifest update + * This function is called by the fragment collector thread to wait until a manifest update is received. */ void MediaTrack::WaitForManifestUpdate() { - if(aamp->DownloadsAreEnabled() && fragmentCollectorWaitingForPlaylistUpdate) + if(aamp->DownloadsAreEnabled()) { std::unique_lock lock(dwnldMutex); - AAMPLOG_INFO("[%s] Waiting for manifest update", name); - frDownloadWait.wait(lock); + // Snapshot the manifest update counter under the mutex before blocking. + const uint32_t snapshotCounter = mManifestUpdateCounter; + AAMPLOG_DEBUG("[%d] Waiting for manifest update (snapshotCounter=%u)...", type, snapshotCounter); + mManifestUpdateWait.wait(lock, [this, snapshotCounter] + { + return mManifestUpdateCounter != snapshotCounter; + }); + AAMPLOG_DEBUG("[%d] Manifest update received (counter=%u).", type, mManifestUpdateCounter); } - fragmentCollectorWaitingForPlaylistUpdate = false; - AAMPLOG_INFO("Exit"); } /** @@ -4520,12 +4563,11 @@ void MediaTrack::PlaylistDownloader() aamp->SendHTTPHeaderResponse(); } - if(fragmentCollectorWaitingForPlaylistUpdate && gotManifest) + if(gotManifest) { // (gotManifest => false) If manifest download failed due to ABR request from HLS, don't abort wait. - // DASH waits for manifest update only at EOS from all tracks, proceed only with fresh manifest. // Signal fragment collector to abort it's wait for playlist process - AbortFragmentDownloaderWait(); + AbortWaitForManifestUpdate(); } // Check whether downloads are still enabled after processing playlist diff --git a/test/utests/fakes/FakeAampMPDUtils.cpp b/test/utests/fakes/FakeAampMPDUtils.cpp index 67159acc2e..76176f1c3d 100644 --- a/test/utests/fakes/FakeAampMPDUtils.cpp +++ b/test/utests/fakes/FakeAampMPDUtils.cpp @@ -62,7 +62,7 @@ void ConstructFragmentURL( std::string& fragmentUrl, const FragmentDescriptor *f * @param[out] referenced_duration referenced duration * @retval true on success */ -bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset) +bool ParseSegmentIndexBox( const uint8_t *start, size_t size, int segmentIndex, unsigned int *referenced_size, float *referenced_duration, unsigned int *firstOffset) { return true; } @@ -72,7 +72,7 @@ bool ParseSegmentIndexBox( const char *start, size_t size, int segmentIndex, uns * @param pptr pointer to read from * @retval word value */ -unsigned int Read16( const char **pptr) +unsigned int Read16( const uint8_t **pptr) { return 0; } @@ -82,7 +82,7 @@ unsigned int Read16( const char **pptr) * @param pptr pointer to read from * @retval word value */ -unsigned int Read32( const char **pptr) +unsigned int Read32( const uint8_t **pptr) { return 0; } @@ -92,7 +92,7 @@ unsigned int Read32( const char **pptr) * @param pptr pointer to read from * @retval word value */ -uint64_t Read64( const char **pptr) +uint64_t Read64( const uint8_t **pptr) { return 0; } @@ -103,7 +103,7 @@ uint64_t Read64( const char **pptr) * @param[in] n word size in bytes * @retval 32 bit value */ -uint64_t ReadWordHelper( const char **pptr, int n ) +uint64_t ReadWordHelper( const uint8_t **pptr, int n ) { return 0; } diff --git a/test/utests/fakes/FakeFragmentCollector_MPD.cpp b/test/utests/fakes/FakeFragmentCollector_MPD.cpp index 880f9df568..5150ed6318 100644 --- a/test/utests/fakes/FakeFragmentCollector_MPD.cpp +++ b/test/utests/fakes/FakeFragmentCollector_MPD.cpp @@ -311,3 +311,20 @@ bool StreamAbstractionAAMP_MPD::ExtractAndAddSubtitleMediaHeader() { return false; } + +void StreamAbstractionAAMP_MPD::WaitForManifestUpdate() +{ +} + +void StreamAbstractionAAMP_MPD::WaitForManifestUpdate(uint32_t counter) +{ +} + +void StreamAbstractionAAMP_MPD::AbortWaitForManifestUpdate() +{ +} + +uint32_t StreamAbstractionAAMP_MPD::GetManifestUpdateCounter() +{ + return 0; +} diff --git a/test/utests/fakes/FakeMediaStreamContext.cpp b/test/utests/fakes/FakeMediaStreamContext.cpp index c8c3a35439..e1d7347cce 100644 --- a/test/utests/fakes/FakeMediaStreamContext.cpp +++ b/test/utests/fakes/FakeMediaStreamContext.cpp @@ -22,7 +22,7 @@ MockMediaStreamContext *g_mockMediaStreamContext = nullptr; -bool MediaStreamContext::CacheFragmentChunk(AampMediaType actualType, const char *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks) +bool MediaStreamContext::CacheFragmentChunk(AampMediaType actualType, const uint8_t *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks) { if (g_mockMediaStreamContext != nullptr) { diff --git a/test/utests/fakes/FakeMetadataProcessor.cpp b/test/utests/fakes/FakeMetadataProcessor.cpp index 7276392f74..9d0525f350 100644 --- a/test/utests/fakes/FakeMetadataProcessor.cpp +++ b/test/utests/fakes/FakeMetadataProcessor.cpp @@ -40,7 +40,7 @@ bool IsoBMFFMetadataProcessor::SetTuneTimePTS() return true; } -void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const char * data_ptr, size_t data_len) +void IsoBMFFMetadataProcessor::ProcessID3Metadata(AampMediaType type, const std::vector& data) {} TSMetadataProcessor::TSMetadataProcessor(id3_callback_t id3_hdl, diff --git a/test/utests/fakes/FakePrivateInstanceAAMP.cpp b/test/utests/fakes/FakePrivateInstanceAAMP.cpp index 0c76e8fd29..e41e7d5e54 100644 --- a/test/utests/fakes/FakePrivateInstanceAAMP.cpp +++ b/test/utests/fakes/FakePrivateInstanceAAMP.cpp @@ -1296,7 +1296,10 @@ void PrivateInstanceAAMP::SendHTTPHeaderResponse() void PrivateInstanceAAMP::LoadIDX(ProfilerBucketType bucketType, std::string fragmentUrl, std::string& effectiveUrl, AampGrowableBuffer *fragment, unsigned int curlInstance, const char *range, int * http_code, double *downloadTime, AampMediaType mediaType,int * fogError) { - return; + if (g_mockPrivateInstanceAAMP != nullptr){ + g_mockPrivateInstanceAAMP->LoadIDX(bucketType, fragmentUrl, effectiveUrl, fragment, curlInstance, range, http_code, downloadTime, mediaType, fogError); + } + return; } bool PrivateInstanceAAMP::IsAudioLanguageSupported (const char *checkLanguage) diff --git a/test/utests/fakes/FakeStreamAbstractionAamp.cpp b/test/utests/fakes/FakeStreamAbstractionAamp.cpp index 66699a3336..4020996ee1 100644 --- a/test/utests/fakes/FakeStreamAbstractionAamp.cpp +++ b/test/utests/fakes/FakeStreamAbstractionAamp.cpp @@ -293,6 +293,19 @@ void MediaTrack::WaitForManifestUpdate() { } +void MediaTrack::WaitForManifestUpdate(uint32_t counter) +{ +} + +void MediaTrack::AbortWaitForManifestUpdate() +{ +} + +uint32_t MediaTrack::GetManifestUpdateCounter() +{ + return 0; +} + bool MediaTrack::WaitForCachedFragmentChunkInjected(int timeoutMs) { return true; diff --git a/test/utests/mocks/MockMediaStreamContext.h b/test/utests/mocks/MockMediaStreamContext.h index e4094eceba..be07fa3c44 100644 --- a/test/utests/mocks/MockMediaStreamContext.h +++ b/test/utests/mocks/MockMediaStreamContext.h @@ -28,7 +28,7 @@ class MockMediaStreamContext public: MOCK_METHOD(bool, CacheFragment, (std::string fragmentUrl, unsigned int curlInstance, double position, double duration, const char *range, bool initSegment, bool discontinuity, bool playingAd, uint32_t scale)); MOCK_METHOD(bool, CacheTsbFragment, (std::shared_ptr fragment)); - MOCK_METHOD(bool, CacheFragmentChunk, (AampMediaType actualType, const char *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks)); + MOCK_METHOD(bool, CacheFragmentChunk, (AampMediaType actualType, const uint8_t *ptr, size_t size, std::string remoteUrl, uint64_t dnldStartTime, uint64_t durationInTicks)); MOCK_METHOD(bool, IsLocalTSBInjection, ()); }; diff --git a/test/utests/mocks/MockPrivateInstanceAAMP.h b/test/utests/mocks/MockPrivateInstanceAAMP.h index 278412355c..7483bdcde6 100644 --- a/test/utests/mocks/MockPrivateInstanceAAMP.h +++ b/test/utests/mocks/MockPrivateInstanceAAMP.h @@ -95,6 +95,7 @@ class MockPrivateInstanceAAMP MOCK_METHOD(bool, IsLiveStream, ()); MOCK_METHOD(bool, TrackDownloadsAreEnabled, (AampMediaType type)); MOCK_METHOD(void, NotifyReservationComplete, (const std::string& reservationId)); + MOCK_METHOD(void, LoadIDX, (ProfilerBucketType bucketType, std::string fragmentUrl, std::string& effectiveUrl, AampGrowableBuffer *fragment, unsigned int curlInstance, const char *range, int * http_code, double *downloadTime, AampMediaType mediaType,int * fogError)); }; extern MockPrivateInstanceAAMP *g_mockPrivateInstanceAAMP; diff --git a/test/utests/run.sh b/test/utests/run.sh index 2f1eeee7e1..75654e65a6 100755 --- a/test/utests/run.sh +++ b/test/utests/run.sh @@ -20,6 +20,9 @@ # This script will build and run microtests. # Use option: -c to additionally build coverage tests # Use option: -h to halt coverage tests on error +# Use option: -t to override the per-test CTest timeout (default: 60) +# The timeout can also be set via the CTEST_TIMEOUT environment variable; +# the -t option takes precedence over the environment variable. if [[ -z "${MAKEFLAGS}" ]]; then export MAKEFLAGS=-j$(nproc) @@ -56,8 +59,10 @@ find . -name "*.gcda" -print0 | xargs -0 --no-run-if-empty rm build_coverage=0 halt_on_error=0 rdke_build=0 +# Default timeout: honour CTEST_TIMEOUT env var if set, otherwise 60 seconds. +CTEST_TIMEOUT=${CTEST_TIMEOUT:-60} -while getopts "ceh" opt; do +while getopts "ceht:" opt; do case ${opt} in c ) echo Do build coverage build_coverage=1 @@ -68,6 +73,9 @@ while getopts "ceh" opt; do h ) echo Halt on error halt_on_error=1 ;; + t ) CTEST_TIMEOUT=${OPTARG} + echo "CTest timeout set to ${CTEST_TIMEOUT}s" + ;; * ) ;; esac @@ -125,7 +133,7 @@ if [ "$rdke_build" -eq "1" ]; then echo "RDKE build" export GTEST_OUTPUT="json" - ctest -j 4 --output-on-failure --no-compress-output -T Test $CT_TESTDIR || true # Don't exit script if a test fails + ctest -j 4 --timeout "${CTEST_TIMEOUT}" --output-on-failure --no-compress-output -T Test $CT_TESTDIR || true # Don't exit script if a test fails cd tests @@ -176,7 +184,7 @@ EOF find . -name test_detail\*.json | xargs cat | jq -s '{test_cases_results: {tests: map(.tests) | add,failures: map(.failures) | add,disabled: map(.disabled) | add,errors: map(.errors) | add,time: ((map(.time | rtrimstr("s") | tonumber) | add) | tostring + "s"),name: .[0].name,testsuites: map(.testsuites[])}}' > L1Report.json else - ctest -j 4 --output-on-failure --no-compress-output -T Test $CT_TESTDIR --output-junit ctest-results.xml + ctest -j 4 --timeout "${CTEST_TIMEOUT}" --output-on-failure --no-compress-output -T Test $CT_TESTDIR --output-junit ctest-results.xml fi if [ "$build_coverage" -eq "1" ]; then diff --git a/test/utests/tests/AampTSBSessionManager/FunctionalTests.cpp b/test/utests/tests/AampTSBSessionManager/FunctionalTests.cpp index 7b3e3dd339..beb01d68e8 100644 --- a/test/utests/tests/AampTSBSessionManager/FunctionalTests.cpp +++ b/test/utests/tests/AampTSBSessionManager/FunctionalTests.cpp @@ -178,15 +178,15 @@ TEST_F(FunctionalTests, TSBWriteTests) cachedFragment->type = eMEDIATYPE_INIT_VIDEO; - // After std::vector refactoring, use fragment.GetPtr() which returns the internal buffer pointer - EXPECT_CALL(*g_mockTSBStore, Write(UNIQUE_INIT_URL, cachedFragment->fragment.GetPtr(), strlen(TEST_DATA))).WillOnce(Return(TSB::Status::OK)); + // After std::vector refactoring, use fragment.data() to access the internal buffer pointer + EXPECT_CALL(*g_mockTSBStore, Write(UNIQUE_INIT_URL, cachedFragment->fragment.data(), strlen(TEST_DATA))).WillOnce(Return(TSB::Status::OK)); EXPECT_CALL(*g_mockPrivateInstanceAAMP, GetVidTimeScale()).WillRepeatedly(Return(1)); EXPECT_CALL(*g_mockPrivateInstanceAAMP, RecalculatePTS(_,_,_)).WillRepeatedly(Return(0)); mAampTSBSessionManager->EnqueueWrite(INIT_URL, cachedFragment, TEST_PERIOD_ID); std::this_thread::sleep_for(std::chrono::milliseconds(25)); // Add video init fragment to TSB which already exists - EXPECT_CALL(*g_mockTSBStore, Write(UNIQUE_INIT_URL, cachedFragment->fragment.GetPtr(), strlen(TEST_DATA))).WillOnce(Return(TSB::Status::ALREADY_EXISTS)); + EXPECT_CALL(*g_mockTSBStore, Write(UNIQUE_INIT_URL, cachedFragment->fragment.data(), strlen(TEST_DATA))).WillOnce(Return(TSB::Status::ALREADY_EXISTS)); mAampTSBSessionManager->EnqueueWrite(INIT_URL, cachedFragment, TEST_PERIOD_ID); std::this_thread::sleep_for(std::chrono::milliseconds(25)); diff --git a/test/utests/tests/IsoBmffConvertToKeyFrameTests/IsoBmffConvertToKeyFrameTests.cpp b/test/utests/tests/IsoBmffConvertToKeyFrameTests/IsoBmffConvertToKeyFrameTests.cpp index 6a617b0b93..cfc95d915e 100644 --- a/test/utests/tests/IsoBmffConvertToKeyFrameTests/IsoBmffConvertToKeyFrameTests.cpp +++ b/test/utests/tests/IsoBmffConvertToKeyFrameTests/IsoBmffConvertToKeyFrameTests.cpp @@ -130,12 +130,12 @@ TEST_P(IsoBmffConvertToKeyFrameTestsP, converToIFrame) EXPECT_TRUE(helper->ConvertToKeyFrame(src_data)); EXPECT_EQ(src_data.size(), td.expected_data_len); - auto memcmp_actual_vs_expected = std::memcmp(src_data.GetPtr(), td.expected_data, td.expected_data_len); + auto memcmp_actual_vs_expected = std::memcmp(src_data.data(), td.expected_data, td.expected_data_len); EXPECT_EQ(0, memcmp_actual_vs_expected); if (memcmp_actual_vs_expected) { std::cout << "Result differs from expected!" << std::endl; - uint8_t* res = (uint8_t*)src_data.GetPtr(); + uint8_t* res = src_data.data(); uint8_t* exp = td.expected_data; uint32_t ii = 0; dumpCommonBytes(res, exp, ii); diff --git a/test/utests/tests/IsoBmffHelperTests/IsoBmffHelperTests.cpp b/test/utests/tests/IsoBmffHelperTests/IsoBmffHelperTests.cpp index 69cbde69fa..125df168a6 100644 --- a/test/utests/tests/IsoBmffHelperTests/IsoBmffHelperTests.cpp +++ b/test/utests/tests/IsoBmffHelperTests/IsoBmffHelperTests.cpp @@ -65,7 +65,7 @@ TEST_F(IsoBmffHelperTests, restampPtsTest) const char* trackName = "video"; uint32_t timeScale = 48000; // Check that setBuffer receives the actual buffer pointer - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, restampPts(ptsOffset)); @@ -89,7 +89,7 @@ TEST_F(IsoBmffHelperTests, restampPtsNegativeTest) std::string url("Dummy"); const char* trackName = "video"; uint32_t timeScale = 48000; - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(false)); EXPECT_CALL(*g_mockIsoBmffBuffer, restampPts(_)).Times(0); @@ -110,7 +110,7 @@ TEST_F(IsoBmffHelperTests, setPtsAndDurationTest) buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); uint64_t pts{123}; uint64_t duration{1}; - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, setPtsAndDuration(pts, duration)); @@ -131,7 +131,7 @@ TEST_F(IsoBmffHelperTests, setPtsAndDurationNegativeTest) buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); uint64_t pts{123}; uint64_t duration{1}; - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(false)); EXPECT_CALL(*g_mockIsoBmffBuffer, setPtsAndDuration(_, _)).Times(0); @@ -148,7 +148,7 @@ TEST_F(IsoBmffHelperTests, setTimescaleTest) uint8_t bufferContent[]{"IsoBmff buffer content"}; // Set the pointer and length in the AampGrowableBuffer fake buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, setTrickmodeTimescale(1000)).WillOnce(Return(true)); @@ -167,7 +167,7 @@ TEST_F(IsoBmffHelperTests, setTimescaleTestNegativeTest) uint8_t bufferContent[]{"IsoBmff buffer content"}; // Set the pointer and length in the AampGrowableBuffer fake buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, setTrickmodeTimescale(1000)).WillOnce(Return(false)); @@ -184,7 +184,7 @@ TEST_F(IsoBmffHelperTests, clearMediaHeaderDurationTest) uint8_t bufferContent[]{"IsoBmff buffer content"}; // Set the pointer and length in the AampGrowableBuffer fake buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, isInitSegment()).WillOnce(Return(true)); @@ -203,7 +203,7 @@ TEST_F(IsoBmffHelperTests, clearMediaHeaderDurationNegativeTest_1) uint8_t bufferContent[]{"IsoBmff buffer content"}; // Set the pointer and length in the AampGrowableBuffer fake buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, isInitSegment()).WillOnce(Return(false)); @@ -221,7 +221,7 @@ TEST_F(IsoBmffHelperTests, clearMediaHeaderDurationNegativeTest_2) uint8_t bufferContent[]{"IsoBmff buffer content"}; // Set the pointer and length in the AampGrowableBuffer fake buffer.assign(bufferContent, bufferContent + sizeof(bufferContent)); - auto expectedPtr = reinterpret_cast(buffer.GetPtr()); + auto expectedPtr = buffer.data(); EXPECT_CALL(*g_mockIsoBmffBuffer, setBuffer(expectedPtr, sizeof(bufferContent))); EXPECT_CALL(*g_mockIsoBmffBuffer, parseBuffer(false, -1)).WillOnce(Return(true)); EXPECT_CALL(*g_mockIsoBmffBuffer, isInitSegment()).WillOnce(Return(true)); diff --git a/test/utests/tests/MediaStreamContextTests/MediaStreamContextTests.cpp b/test/utests/tests/MediaStreamContextTests/MediaStreamContextTests.cpp index 54515dba95..832ec9f872 100644 --- a/test/utests/tests/MediaStreamContextTests/MediaStreamContextTests.cpp +++ b/test/utests/tests/MediaStreamContextTests/MediaStreamContextTests.cpp @@ -210,7 +210,7 @@ TEST_F(MediaStreamContextTest, DefaultDurationTest) TEST_F(MediaStreamContextTest, CacheFragmentChunkTestWithDurationInTicks) { - char testData[] = "This is a test data buffer"; + uint8_t testData[] = "This is a test data buffer"; size_t testDataSize = sizeof(testData) - 1; // Exclude null terminator std::string remoteUrl = "http://example.com"; uint64_t dnldStartTime = 123456789; diff --git a/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp b/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp index 1d44f29934..66fca5f52a 100644 --- a/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp +++ b/test/utests/tests/MediaTrackTests/MediaTrackTests.cpp @@ -19,6 +19,8 @@ #include #include +#include +#include #include "AampUtils.h" #include "AampConfig.h" @@ -61,7 +63,7 @@ AampConfig* gpGlobalConfig{nullptr}; MATCHER_P(AampGrowableBufferRefEq, bufferStdConstRef, "") { const AampGrowableBuffer& buffer = bufferStdConstRef.get(); - return std::memcmp(arg.GetPtr(), buffer.GetPtr(), buffer.size()) == 0; + return std::memcmp(arg.GetPtr(), buffer.data(), buffer.size()) == 0; } MATCHER_P(AampGrowableBufferPtrEq, bufferPtr, "") @@ -821,3 +823,69 @@ TEST_F(MediaTrackTests, MediaTrackConstructorChunkModeTest) TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", mStreamAbstractionAAMP_MPD}; EXPECT_EQ(videoTrack.GetCachedFragmentChunksSize(), kMaxFragmentChunkCached); } + +/** + * @brief Test that WaitForManifestUpdate can be aborted successfully. + * This is important to avoid deadlocks if the manifest update takes a long time or fails to complete. + */ +TEST_F(MediaTrackTests, WaitForManifestUpdateTest) +{ + EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()).WillRepeatedly(Return(true)); + TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", mStreamAbstractionAAMP_MPD}; + + std::thread manifestUpdateThread([&videoTrack]() { + videoTrack.WaitForManifestUpdate(); + }); + + std::this_thread::sleep_for(std::chrono::milliseconds(200)); // Give the thread a moment to start and block on the wait + videoTrack.AbortWaitForManifestUpdate(); + manifestUpdateThread.join(); +} + +/** + * @brief Test that GetManifestUpdateCounter() returns the current counter value and + * that AbortWaitForManifestUpdate() increments it. + */ +TEST_F(MediaTrackTests, GetManifestUpdateCounterTest) +{ + TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", mStreamAbstractionAAMP_MPD}; + + const uint32_t initialCounter = videoTrack.GetManifestUpdateCounter(); + + videoTrack.AbortWaitForManifestUpdate(); + EXPECT_EQ(videoTrack.GetManifestUpdateCounter(), initialCounter + 1); + + videoTrack.AbortWaitForManifestUpdate(); + EXPECT_EQ(videoTrack.GetManifestUpdateCounter(), initialCounter + 2); +} + +/** + * @brief Test the race prevention pattern: snapshot the counter with + * GetManifestUpdateCounter() *before* doing work, then call + * WaitForManifestUpdate(snapshotCounter). If AbortWaitForManifestUpdate() fires + * between the snapshot and the wait call, the predicate is already satisfied and + * WaitForManifestUpdate(snapshotCounter) must return immediately without blocking. + */ +TEST_F(MediaTrackTests, WaitForManifestUpdateSnapshotRacePreventionTest) +{ + EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()).WillRepeatedly(Return(true)); + TestableMediaTrack videoTrack{eTRACK_VIDEO, mPrivateInstanceAAMP, "video", mStreamAbstractionAAMP_MPD}; + + // Step 1: snapshot the counter before any work begins + const uint32_t snapshot = videoTrack.GetManifestUpdateCounter(); + + // Step 2: simulate the race — AbortWaitForManifestUpdate() fires BEFORE the wait call + videoTrack.AbortWaitForManifestUpdate(); + + // Step 3: WaitForManifestUpdate(snapshot) must return immediately because the + // counter has already advanced past the snapshot. Run it on a background thread + // so we can enforce a tight deadline without hanging the test runner. + auto future = std::async(std::launch::async, [&videoTrack, snapshot]() { + videoTrack.WaitForManifestUpdate(snapshot); + }); + + // 500 ms is generous for an already-satisfied predicate; any blocking would fail this. + EXPECT_EQ(future.wait_for(std::chrono::milliseconds(500)), std::future_status::ready) + << "WaitForManifestUpdate(snapshotCounter) blocked even though the counter was " + "already incremented — lost-wakeup race prevention is broken"; +} \ No newline at end of file diff --git a/test/utests/tests/PrivAampTests/PrivAampTests.cpp b/test/utests/tests/PrivAampTests/PrivAampTests.cpp index 0770dc8093..49070ea990 100644 --- a/test/utests/tests/PrivAampTests/PrivAampTests.cpp +++ b/test/utests/tests/PrivAampTests/PrivAampTests.cpp @@ -1171,7 +1171,7 @@ TEST_F(PrivAampTests, HandleSSLWriteCallbackWithPartialMp4Chunk) // This happens in the second call to HandleSSLWriteCallback when the complete chunked mdat is received in the buffer. The first call should not trigger CacheFragmentChunk() as the chunk is not complete yet. // Lets make this a strict check using expected values EXPECT_CALL(*g_mockMediaStreamContext, - CacheFragmentChunk(eMEDIATYPE_VIDEO, reinterpret_cast(buffer.data()) + startBufferOffset, chunkBoundary - startBufferOffset, _, _, mdatDuration)) + CacheFragmentChunk(eMEDIATYPE_VIDEO, buffer.data() + startBufferOffset, chunkBoundary - startBufferOffset, _, _, mdatDuration)) .Times(1); size_t result = p_aamp->HandleSSLWriteCallback(testDataPart2, strlen(testDataPart2), 1, &context); diff --git a/test/utests/tests/PrivateInstanceAAMP/ChunkedTransferTests.cpp b/test/utests/tests/PrivateInstanceAAMP/ChunkedTransferTests.cpp index 4559383e82..f043507957 100644 --- a/test/utests/tests/PrivateInstanceAAMP/ChunkedTransferTests.cpp +++ b/test/utests/tests/PrivateInstanceAAMP/ChunkedTransferTests.cpp @@ -52,7 +52,7 @@ struct ChunkHarness { std::string GetBufferAsString() { - return std::string( buffer.GetPtr(), buffer.size() ); + return std::string( reinterpret_cast(buffer.data()), buffer.size() ); } }; diff --git a/test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp b/test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp index cdefdd3ad9..25e84426b5 100644 --- a/test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_HLS/FunctionalTests.cpp @@ -1563,9 +1563,6 @@ TEST_F(TrackStateTests, FindTimedMetadata_New) bool reportBulkMeta = false; bool bInitCall = true; // Simulate the bInitCall flag being set EXPECT_CALL(*g_mockAampConfig, IsConfigSet(eAAMPConfig_EnableSubscribedTags)).WillOnce(Return(true)); - AampGrowableBuffer buffer("tsProcessor PAT/PMT test"); - //buffer.AppendBytes(10); - buffer.GetPtr(); TrackStateobj->FindTimedMetadata(reportBulkMeta, bInitCall); } @@ -2117,16 +2114,6 @@ TEST_F(TrackStateTests, EnterTimedWaitForPlaylistRefreshTests) TrackStateobj->EnterTimedWaitForPlaylistRefresh(-22); } -TEST_F(TrackStateTests, AbortFragmentDownloaderWaitTests) -{ - TrackStateobj->AbortFragmentDownloaderWait(); -} - -TEST_F(TrackStateTests, WaitForManifestUpdateTests) -{ - TrackStateobj->WaitForManifestUpdate(); -} - TEST_F(TrackStateTests, GetBufferStatusTest) { // Call the function under test @@ -2152,11 +2139,6 @@ TEST_F(TrackStateTests, ProcessFragmentChunkTests) ASSERT_FALSE(result); } -TEST_F(TrackStateTests, NotifyFragmentCollectorWaittest) -{ - TrackStateobj->NotifyFragmentCollectorWait(); -} - TEST_F(TrackStateTests, GetTotalInjectedDurationtest) { double result = TrackStateobj->GetTotalInjectedDuration(); diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp index 505a43b10c..a03dc346a0 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FetcherLoopTests.cpp @@ -44,6 +44,8 @@ using ::testing::NiceMock; using ::testing::Return; using ::testing::SetArgReferee; using ::testing::StrictMock; +using ::testing::Invoke; +using ::testing::WithArg; using ::testing::WithArgs; using ::testing::WithoutArgs; @@ -2149,10 +2151,12 @@ struct TestParams { const char *manifest; double seekPos; + bool mockIDXDownload; const char *videoInitFragment; const char *audioInitFragment; const char *videoFragmentP1; const char *audioFragmentP1; + const char *endVideoFragmentP1; }; // Test cases @@ -2203,10 +2207,12 @@ TestParams testCases[] = { )", 24.0, + false, "video_p0_init.mp4", "audio_p0_init.mp4", "video_p1_init.mp4", - "audio_p1_init.mp4"}, + "audio_p1_init.mp4", + "video_p1_18.m4s"}, { R"( @@ -2253,10 +2259,12 @@ TestParams testCases[] = { )", 0, + false, "video_p0_init.m4s", "audio_p0_init.m4s", "video_p1_init.m4s", - "audio_p1_init.m4s"}, + "audio_p1_init.m4s", + "video_p1_2.m4s"}, { R"( @@ -2307,10 +2315,12 @@ TestParams testCases[] = { )", 0, + false, "video_p0.m4s", "audio_p0.m4s", "video_p1.m4s", - "audio_p1.m4s"}, + "audio_p1.m4s", + "video_p1.m4s"}, { R"( @@ -2345,10 +2355,12 @@ TestParams testCases[] = { )", 0, + true, "video_p0.m4s", "audio_p0.m4s", "video_p1.m4s", - "audio_p1.m4s"}, + "audio_p1.m4s", + "video_p1.m4s"}, { R"( @@ -2391,17 +2403,52 @@ TestParams testCases[] = { )", 0, + true, "video_p0.m4s", "audio_p0.m4s", "video_p1.m4s", - "audio_p1.m4s"}}; + "audio_p1.m4s", + "video_p1.m4s"} +}; + +// Sample SIDX box for testing IndexedSegment download scenarios. +// Contains 2 segment references with 2-second durations each. +// timescale=1000, total duration=4000ms (4 seconds) +static const uint8_t sidxBox[] = { + // Box size = 56 + 0x00, 0x00, 0x00, 0x38, + // Type = 'sidx' + 0x73, 0x69, 0x64, 0x78, + // version=0, flags=0 + 0x00, 0x00, 0x00, 0x00, + // reference_ID = 1 + 0x00, 0x00, 0x00, 0x01, + // timescale = 1000 + 0x00, 0x00, 0x03, 0xE8, + // earliest_presentation_time = 0 (32-bit, version 0) + 0x00, 0x00, 0x00, 0x00, + // first_offset = 0 + 0x00, 0x00, 0x00, 0x00, + // reserved = 0 + 0x00, 0x00, + // reference_count = 2 + 0x00, 0x02, + // Reference 0: size=16384, duration=2000, flags + 0x00, 0x00, 0x40, 0x00, + 0x00, 0x00, 0x07, 0xD0, + 0x90, 0x00, 0x00, 0x00, + // Reference 1: size=12288, duration=2000, flags + 0x00, 0x00, 0x30, 0x00, + 0x00, 0x00, 0x07, 0xD0, + 0x90, 0x00, 0x00, 0x00, +}; class AdvancedFetcherLoopTests : public FetcherLoopTests, public ::testing::WithParamInterface { public: void SetUp() override { - counter = 0; + shouldExitTest = false; FetcherLoopTests::SetUp(); } @@ -2409,7 +2456,7 @@ class AdvancedFetcherLoopTests : public FetcherLoopTests, public ::testing::With { FetcherLoopTests::TearDown(); } - int counter; + bool shouldExitTest; }; /** @@ -2428,14 +2475,24 @@ TEST_P(AdvancedFetcherLoopTests, FetcherLoopTestsWithDifferentMPD) TestParams param = GetParam(); const char *manifest = param.manifest; double seekPos = param.seekPos; + bool mockIDXDownload = param.mockIDXDownload; const char *videoInitFragment = param.videoInitFragment; const char *audioInitFragment = param.audioInitFragment; const char *videoFragmentP1 = param.videoFragmentP1; const char *audioFragmentP1 = param.audioFragmentP1; + std::string endVideoFragmentUrl = std::string(TEST_BASE_URL) + std::string(param.endVideoFragmentP1); /* Initialize MPD. The video/audio initialization segment is cached. */ videoFragmentUrl = std::string(TEST_BASE_URL) + std::string(videoInitFragment); audioFragmentUrl = std::string(TEST_BASE_URL) + std::string(audioInitFragment); + if (mockIDXDownload) + { + EXPECT_CALL(*g_mockPrivateInstanceAAMP, LoadIDX(_, _, _, _, _, _, _, _, _, _)) + .WillRepeatedly(WithArg<3>(Invoke([](AampGrowableBuffer *idxBuffer) + { + idxBuffer->AppendBytes((const uint8_t *)sidxBox, sizeof(sidxBox)); + }))); + } EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(videoFragmentUrl, _, _, _, _, true, _, _, _)).Times(1).WillOnce(Return(true)); EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(audioFragmentUrl, _, _, _, _, true, _, _, _)).Times(1).WillOnce(Return(true)); EXPECT_CALL(*g_mockPrivateInstanceAAMP, IsLocalAAMPTsbInjection()).WillRepeatedly(Return(false)); @@ -2447,19 +2504,20 @@ TEST_P(AdvancedFetcherLoopTests, FetcherLoopTestsWithDifferentMPD) EXPECT_EQ(status, eAAMPSTATUS_OK); - /* Push the first video segment to present. - * The segment starts at time 40.0s and has a duration of 2.0s. - */ + // Run test until the end segment of the period is cached, then exit EXPECT_CALL(*g_mockPrivateInstanceAAMP, DownloadsAreEnabled()) .Times(AnyNumber()) - .WillRepeatedly([this]() { return (++counter < 20); }); + .WillRepeatedly([this]() { return !shouldExitTest; }); videoFragmentUrl = std::string(TEST_BASE_URL) + std::string(videoFragmentP1); audioFragmentUrl = std::string(TEST_BASE_URL) + std::string(audioFragmentP1); EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(videoFragmentUrl, _, _, _, _, true, _, _, _)).Times(1).WillOnce(Return(true)); EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(audioFragmentUrl, _, _, _, _, true, _, _, _)).Times(1).WillOnce(Return(true)); + // Default expect EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(_, _, _, _, _, false, _, _, _)).WillRepeatedly(Return(true)); - + // Expect the last segment of the period to be cached, and then exit the test + EXPECT_CALL(*g_mockMediaStreamContext, CacheFragment(endVideoFragmentUrl, _, _, _, _, false, _, _, _)) + .WillOnce([this]() { shouldExitTest = true; return true; }); /* Invoke the fetcher loop. */ mTestableStreamAbstractionAAMP_MPD->InvokeFetcherLoop(); EXPECT_EQ(mTestableStreamAbstractionAAMP_MPD->GetCurrentPeriodIdx(), 1); diff --git a/test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp b/test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp index fbfafa4724..8014120bcc 100644 --- a/test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp +++ b/test/utests/tests/StreamAbstractionAAMP_MPD/FunctionalTests.cpp @@ -65,6 +65,7 @@ class FunctionalTestsBase StreamAbstractionAAMP_MPD *mStreamAbstractionAAMP_MPD; CDAIObject *mCdaiObj; const char *mManifest; + uint64_t mLastPlaylistDownloadTimeMs; static constexpr const char *TEST_HOST_URL = "http://host/"; static constexpr const char *TEST_BASE_URL = "http://host/asset/"; static constexpr const char *TEST_MANIFEST_URL = "http://host/asset/manifest.mpd"; @@ -163,6 +164,7 @@ class FunctionalTestsBase mBoolConfigSettings = mDefaultBoolConfigSettings; mIntConfigSettings = mDefaultIntConfigSettings; mCdaiObj = nullptr; + mLastPlaylistDownloadTimeMs = 0; } void TearDown() @@ -273,6 +275,7 @@ class FunctionalTestsBase response->mMPDDownloadResponse->iHttpRetValue = 200; response->mMPDDownloadResponse->sEffectiveUrl = mManifestUrl; response->mMPDDownloadResponse->mDownloadData.assign(mManifest, mManifest + strlen(mManifest)); + response->mLastPlaylistDownloadTimeMs = mLastPlaylistDownloadTimeMs; GetMPDFromManifest(response); mResponse = response; return response; @@ -414,6 +417,11 @@ R"( mStreamAbstractionAAMP_MPD->PushNextFragment(pMediaStreamContext, 0); } + + void SetLastPlaylistDownloadTimeMs(uint64_t timeMs) + { + mLastPlaylistDownloadTimeMs = timeMs; + } }; class FunctionalTests_1 : public ::testing::Test @@ -710,15 +718,13 @@ class StreamAbstractionAAMP_MPDTest : public FunctionalTestsBase, g_mockPrivateInstanceAAMP = new NiceMock(); g_mockAampConfig = new NiceMock(); mStreamAbstractionAAMP_MPD = new TestableStreamAbstractionAAMP_MPD(mPrivateInstanceAAMP, 0.0, 1.0); + g_mockAampMPDDownloader = new StrictMock(); + g_mockAampUtils = new StrictMock(); // Ensure mMPDParseHelper is initialized to avoid NULL dereference mStreamAbstractionAAMP_MPD->SetMPDParseHelper( std::make_shared() ); - g_MockPrivateCDAIObjectMPD = new NiceMock(); g_mockTSBSessionManager = new NiceMock(mPrivateInstanceAAMP); - - g_mockAampMPDDownloader = new StrictMock(); - g_mockAampUtils = new StrictMock(); g_mockABRManager = new NiceMock(); } @@ -973,6 +979,7 @@ R"( availabilityStartTime = ISO8601DateTimeToUTCSeconds(availabilityStartTimeISO); deltaTime = currentTime - availabilityStartTime; timeMS = 1000LL*((long long)currentTime); + SetLastPlaylistDownloadTimeMs(timeMS); EXPECT_CALL(*g_mockAampUtils, aamp_GetCurrentTimeMS()) .Times(AnyNumber()) .WillRepeatedly(Return(timeMS)); @@ -4906,3 +4913,4 @@ R"( double availabilityStartTime = ISO8601DateTimeToUTCSeconds("2025-11-15T00:00:00Z"); EXPECT_EQ(actualPosition, availabilityStartTime + seekPosition); } + diff --git a/tsDemuxer.cpp b/tsDemuxer.cpp index 6660e572e5..152b567776 100644 --- a/tsDemuxer.cpp +++ b/tsDemuxer.cpp @@ -160,7 +160,7 @@ void Demuxer::sendInternal(MediaProcessor::process_fcn_t processor) if (CheckForSteadyState()) { // Copy the segment data into a vector and pass it to the processing function - uint8_t * data_ptr = reinterpret_cast(es.GetPtr()); + uint8_t * data_ptr = es.data(); const auto len = es.size(); std::vector buf(len); @@ -465,17 +465,17 @@ void Demuxer::processPacket(const unsigned char * packetStart, bool &basePtsUpda size -= bytes_to_read; if (pes_header.size() == aamp_ts::pes_min_data) { - if (!IS_PES_PACKET_START(pes_header.GetPtr())) + if (!IS_PES_PACKET_START(pes_header.data())) { - AAMPLOG_WARN("Packet start prefix check failed 0x%x 0x%x 0x%x", pes_header.GetPtr()[0], - pes_header.GetPtr()[1], pes_header.GetPtr()[2]); + AAMPLOG_WARN("Packet start prefix check failed 0x%x 0x%x 0x%x", pes_header.data()[0], + pes_header.data()[1], pes_header.data()[2]); pes_state = PES_STATE_WAITING_FOR_HEADER; break; } - if (PES_OPTIONAL_HEADER_PRESENT(pes_header.GetPtr())) + if (PES_OPTIONAL_HEADER_PRESENT(pes_header.data())) { pes_state = PES_STATE_GETTING_HEADER_EXTENSION; - pes_header_ext_len = PES_OPTIONAL_HEADER_LENGTH(pes_header.GetPtr()); + pes_header_ext_len = PES_OPTIONAL_HEADER_LENGTH(pes_header.data()); pes_header_ext_read = 0; AAMPLOG_DEBUG( "Optional header preset len = %d. Switching to PES_STATE_GETTING_HEADER_EXTENSION", @@ -485,7 +485,7 @@ void Demuxer::processPacket(const unsigned char * packetStart, bool &basePtsUpda { AAMPLOG_WARN( "Optional header not preset pesStart[6] 0x%x bytes_to_read %d- switching to PES_STATE_WAITING_FOR_HEADER", - pes_header.GetPtr()[6], bytes_to_read); + pes_header.data()[6], bytes_to_read); pes_state = PES_STATE_WAITING_FOR_HEADER; } } diff --git a/tsFragmentProcessor.cpp b/tsFragmentProcessor.cpp index e2ceb835fb..2e57e75b4c 100644 --- a/tsFragmentProcessor.cpp +++ b/tsFragmentProcessor.cpp @@ -140,7 +140,7 @@ bool TSFragmentProcessor::ProcessFragment(const AampGrowableBuffer & fragment, { constexpr int m_ttsSize {0}; const size_t frag_size = fragment.size(); - const uint8_t * base_frag_ptr = reinterpret_cast(fragment.GetPtr()); + const uint8_t * base_frag_ptr = fragment.data(); uint8_t * curr_packet_ptr = const_cast(base_frag_ptr) + m_ttsSize; size_t curr_packet_len = frag_size; @@ -187,7 +187,7 @@ bool TSFragmentProcessor::ProcessFragment(const AampGrowableBuffer & fragment, bool TSFragmentProcessor::ValidateFragment(const AampGrowableBuffer & fragment, uint8_t * & curr_packet_ptr, size_t & curr_len) const { - const auto base_frag_ptr = reinterpret_cast(fragment.GetPtr()); + const auto base_frag_ptr = fragment.data(); // It seems some ts have an invalid packet at the start, so try skipping it while (((curr_packet_ptr[0] != 0x47) || ((curr_packet_ptr[1] & 0x80) != 0x00) || ((curr_packet_ptr[3] & 0xC0) != 0x00)) && (curr_len > ts_packet_size)) diff --git a/tsprocessor.cpp b/tsprocessor.cpp index 1fa9246291..93c773ca62 100644 --- a/tsprocessor.cpp +++ b/tsprocessor.cpp @@ -1780,7 +1780,7 @@ void TSProcessor::setBasePTS(double position, long long pts) double TSProcessor::getFirstPts( AampGrowableBuffer* pBuffer ) { double firstPts = 0.0; - auto tsDemux = new TsDemux( eMEDIATYPE_VIDEO, pBuffer->GetPtr(), pBuffer->size(), true ); + auto tsDemux = new TsDemux( eMEDIATYPE_VIDEO, pBuffer->data(), pBuffer->size(), true ); if( tsDemux ) { firstPts = tsDemux->getPts(0); @@ -1812,9 +1812,9 @@ bool TSProcessor::sendSegment(AampGrowableBuffer* pBuffer, double position, doub bool isInit, process_fcn_t processor, bool &ptsError) { bool insPatPmt = false; //CID:84507 - Initialization - unsigned char * packetStart; - char *segment = pBuffer->GetPtr(); - int len = (int)(pBuffer->size()); + uint8_t *packetStart = nullptr; + uint8_t *segment = pBuffer->data(); + size_t len = pBuffer->size(); bool ret = false; ptsError = false; { @@ -1841,19 +1841,29 @@ bool TSProcessor::sendSegment(AampGrowableBuffer* pBuffer, double position, doub } m_framesProcessedInSegment = 0; m_lastPTSOfSegment = -1; - packetStart = (unsigned char *)segment; + packetStart = segment; + + // Guard against null or too-short buffer before dereferencing TS header bytes + if (segment == nullptr || len < 4) + { + AAMPLOG_ERR("Empty or invalid segment buffer, discarding."); + std::lock_guard guard(m_mutex); + m_processing = false; + m_throttleCond.notify_one(); + return false; + } // It seems some ts have an invalid packet at the start, so try skipping it while (((packetStart[0] != 0x47) || ((packetStart[1] & 0x80) != 0x00) || ((packetStart[3] & 0xC0) != 0x00)) && (len > 188)) { packetStart += 188; // Just jump a packet len -= 188; - if ((((char *)packetStart - segment) > 376) || + if (((packetStart - segment) > 376) || (len < 188)) { AAMPLOG_ERR("No valid ts packet found near the start of the segment"); - packetStart = (unsigned char *)segment; - len = (int)(pBuffer->size()); + packetStart = segment; + len = pBuffer->size(); break; } } @@ -1868,11 +1878,11 @@ bool TSProcessor::sendSegment(AampGrowableBuffer* pBuffer, double position, doub } if (len % m_packetSize) { - int discardAtEnd = len % m_packetSize; - AAMPLOG_INFO("Discarding %d bytes at end", discardAtEnd); + size_t discardAtEnd = len % m_packetSize; + AAMPLOG_INFO("Discarding %zu bytes at end", discardAtEnd); len = len - discardAtEnd; } - ret = processBuffer((unsigned char*)packetStart, len, insPatPmt, discontinuous); + ret = processBuffer(packetStart, static_cast(len), insPatPmt, discontinuous); if (ret) { if (-1.0 == m_startPosition)