Skip to content

VPAAMP-764 Guard GetBufferStatus against uninitialized mStartTimeStamp before first fragment injection#1718

Open
pstroffolino wants to merge 2 commits into
dev_sprint_25_2from
feature/VPAAMP-764
Open

VPAAMP-764 Guard GetBufferStatus against uninitialized mStartTimeStamp before first fragment injection#1718
pstroffolino wants to merge 2 commits into
dev_sprint_25_2from
feature/VPAAMP-764

Conversation

@pstroffolino

Copy link
Copy Markdown
Contributor

MediaTrack::GetBufferStatus() can produce anomalous negative bufferedTime and epoch-scale elapsedTime values when called before NotifyFirstFragmentInjected() has set mStartTimeStamp. This occurs because MonitorBufferHealth may start polling before the fragment injector thread has injected the first fragment, at which point mStartTimeStamp is still -1, causing GetElapsedTime() to compute against epoch (Jan 1 1970).

Root cause: Race between MonitorBufferHealth thread launch and first fragment injection in the fragment injector thread. mStartTimeStamp is initialized to -1 and only set to a valid value in NotifyFirstFragmentInjected().

Fix: Add an early return of BUFFER_STATUS_GREEN in GetBufferStatus() when pContext->mStartTimeStamp < 0. Buffer monitoring is meaningless before first fragment injection, so reporting GREEN is the correct behavior.

Impact: Eliminates spurious buffer underflow warnings and misleading GetBufferStatus log entries during startup. These bogus entries were visible in AutoTriage BufferStatus visualizations as large negative bufferedTime outliers.

MediaTrack::GetBufferStatus() can produce anomalous negative bufferedTime and epoch-scale elapsedTime values when called before NotifyFirstFragmentInjected() has set mStartTimeStamp. This occurs because MonitorBufferHealth may start polling before the fragment injector thread has injected the first fragment, at which point mStartTimeStamp is still -1, causing GetElapsedTime() to compute against epoch (Jan 1 1970).

Root cause: Race between MonitorBufferHealth thread launch and first fragment injection in the fragment injector thread. mStartTimeStamp is initialized to -1 and only set to a valid value in NotifyFirstFragmentInjected().

Fix: Add an early return of BUFFER_STATUS_GREEN in GetBufferStatus() when pContext->mStartTimeStamp < 0. Buffer monitoring is meaningless before first fragment injection, so reporting GREEN is the correct behavior.

Impact: Eliminates spurious buffer underflow warnings and misleading GetBufferStatus log entries during startup. These bogus entries were visible in AutoTriage BufferStatus visualizations as large negative bufferedTime outliers.

Signed-off-by: Philip Stroffolino <philip_stroffolino@cable.comcast.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR prevents MediaTrack::GetBufferStatus() from reporting bogus startup buffer metrics when buffer health polling begins before the first fragment has been injected (i.e., before StreamAbstractionAAMP::NotifyFirstFragmentInjected() initializes mStartTimeStamp).

Changes:

  • Add an early return of BUFFER_STATUS_GREEN in MediaTrack::GetBufferStatus() when mStartTimeStamp is still uninitialized (< 0), avoiding epoch-scale elapsed time calculations and negative buffered time.

Comment thread streamabstraction.cpp
Comment thread streamabstraction.cpp Outdated
Before NotifyFirstFragmentInjected(), mStartTimeStamp is -1 which causes
GetElapsedTime() to return epoch-scale values, producing bogus negative
bufferedTime.  Buffer monitoring is meaningless before first fragment
injection, so return BUFFER_STATUS_GREEN immediately.

- Add public IsFirstFragmentInjected() to StreamAbstractionAAMP that
  checks mStartTimeStamp >= 0 under mLock (thread-safe)
- Use IsFirstFragmentInjected() in GetBufferStatus() guard instead of
  direct protected field access
- Fix 3 existing LLD GetBufferStatus tests to call
  NotifyFirstFragmentInjected() (they model post-injection buffering)
- Add 2 new tests: pre-injection GREEN guard, post-injection evaluation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants