Skip to content

VPLAY-10090 VPLAY-10399 - Audio loss in Trick to play transition in L2 tests #288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 25, 2025

Conversation

anjali-syna
Copy link
Contributor

@anjali-syna anjali-syna commented Jul 18, 2025

Reason for change: Audio is lost while performing live pause and fast forward to live using LLD test asset in L2 test environment.
Changes: Perform Flush after Configure in TuneHelper. Also, call flush with shouldTearDown false, because in case of a new tune, pipeline might not be in a playing/paused state which causes Flush() to destroy the pipeline. This has to be avoided.
Test Procedure: Refer Jira Ticket

@anjali-syna anjali-syna requested review from a team as code owners July 18, 2025 06:47
@rdkcmf-jenkins
Copy link

Coverity Issue - Data race condition

Accessing "pMediaStreamContext->numberOfFragmentChunksCached" without holding lock "MediaTrack.mutex". Elsewhere, "MediaTrack.numberOfFragmentChunksCached" is written to with "MediaTrack.mutex" held 3 out of 3 times (2 of these accesses strongly imply that it is necessary).

Medium Impact, CWE-366
MISSING_LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
fragmentcollector_mpd.cpp:9107

@rdkcmf-jenkins
Copy link

rdkcmf-jenkins commented Jul 18, 2025

Coverity Issue - Unchecked return value

Calling "GetCurrentTextTrack" without checking return value (as is done elsewhere 1 out of 1 times).

Medium Impact, CWE-252
CHECKED_RETURN

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
priv_aamp.cpp:10143

@rdkcmf-jenkins
Copy link

Coverity Issue - Double unlock

"PushNextFragment" unlocks "pMediaStreamContext->mutex" while it is unlocked.

Medium Impact, CWE-765
LOCK

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
fragmentcollector_mpd.cpp:9048

@rdkcmf-jenkins
Copy link

rdkcmf-jenkins commented Jul 18, 2025

Coverity Issue - Unchecked return value

Calling "GetCurrentAudioTrack" without checking return value (as is done elsewhere 2 out of 2 times).

Medium Impact, CWE-252
CHECKED_RETURN

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
priv_aamp.cpp:10023

Copy link

@dnfaulkner dnfaulkner left a comment

Choose a reason for hiding this comment

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

One small comment

@anjali-syna anjali-syna force-pushed the feature/VPLAY-10090 branch 2 times, most recently from 1b3f4d4 to 12b124c Compare July 23, 2025 09:03
Copy link
Contributor

@jfagunde jfagunde left a comment

Choose a reason for hiding this comment

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

Good job!

@shripadbpersonal shripadbpersonal changed the title VPLAY-10090 - Audio loss in Trick to play transition in L2 tests VPLAY-10090 VPLAY-10399 - Audio loss in Trick to play transition in L2 tests Jul 25, 2025
Copy link
Contributor

@Vinish100 Vinish100 left a comment

Choose a reason for hiding this comment

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

Looks good!

…ests

Reason for change: Audio is lost while performing fast forward to live
 in L2 test environment. This is because in TuneHelper,  Configure is
 happening after Flush() call with proper seek position.
 This results in pipeline loosing the flush and seek, thus affecting AV sync.

Changes:
* Perform Flush after Configure in TuneHelper
* Also, call flush with shouldTearDown false, because in case of a new tune,
  pipeline might not be in a playing/paused state which causes Flush() to
  destroy the pipeline. This has to be avoided.
* Adding new unit test:TuneHelperWithAampTsbConfigureFlushSequence

Test Procedure: Run L2 tests

Risks: Low
…ests

Reason for change: Audio is lost while performing live pause
 and fast forward to live using LLD test asset in L2 test environment.
 This is because in TuneHelper,  Configure is happening after Flush() call
 with proper seek position. This results in pileline loosing the flush and
 seek, thus affecting AV sync.

Changes:
* Perform Flush after Configure in TuneHelper
* Also, call flush with shouldTearDown false, because in case of a new tune,
  pipeline might not be in a playing/paused state which causes Flush() to
  destroy the pipeline. This has to be avoided.
* Adding new unit test:TuneHelperWithAampTsbConfigureFlushSequence

Test Procedure: Run L2 tests

Risks: Low

Signed-off-by: anjali-syna <[email protected]>
…ests

Reason for change: Audio is lost while performing fast forward to live
 in L2 test environment. This is because in TuneHelper,  Configure is
 happening after Flush() call with proper seek position.
 This results in pipeline loosing the flush and seek, thus affecting AV sync.

Changes:
* Perform Flush after Configure in TuneHelper
* Also, call flush with shouldTearDown false, because in case of a new tune,
  pipeline might not be in a playing/paused state which causes Flush() to
  destroy the pipeline. This has to be avoided.
* Adding new unit test:TuneHelperWithAampTsbConfigureFlushSequence

Test Procedure: Refer JIRA ticket

Risks: Low

Signed-off-by: anjali-syna <[email protected]>
…ests

Reason for change: Audio is lost while performing fast forward to live
 in L2 test environment. This is because in TuneHelper,  Configure is
 happening after Flush() call with proper seek position.
 This results in pipeline loosing the flush and seek, thus affecting AV sync.

Changes:
* Perform Flush after Configure in TuneHelper
* Also, call flush with shouldTearDown false, because in case of a new tune,
  pipeline might not be in a playing/paused state which causes Flush() to
  destroy the pipeline. This has to be avoided.
* Adding new unit test:TuneHelperWithAampTsbConfigureFlushSequence

Test Procedure: Refer JIRA ticket

Risks: Low

Signed-off-by: anjali-syna <[email protected]>
@Vinish100 Vinish100 force-pushed the feature/VPLAY-10090 branch from 784f8b4 to aac79d7 Compare July 25, 2025 13:44
@Vinish100 Vinish100 merged commit 74d82ef into dev_sprint_25_2 Jul 25, 2025
6 of 7 checks passed
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.

5 participants