VPLAY-13112: CC fails to display on linear channel change, and by psiva01 · Pull Request #1247 · rdkcentral/aamp

psiva01 · 2026-03-26T18:36:32Z

appears only after D-pad right key press

appears only after D-pad right key press Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>

Copilot

Pull request overview

This PR adjusts how closed-caption (CC) enable/disable state is applied and persisted in PrivateInstanceAAMP::SetCCStatusInternal, aiming to prevent CC from failing to display after a linear channel change until user input occurs.

Changes:

Refactors SetCCStatusInternal() to compute and log mute/feature flags up front.
Updates CC manager status even when mpStreamAbstractionAAMP is null (e.g., after Stop()).
Applies subtitle muting to the stream abstraction only when using the GstSubtec OOB path.

Copilot · 2026-03-30T03:43:22Z

+     * Keep persisted CC/subtitle state in sync even after Stop(), when
+     * mpStreamAbstractionAAMP can be NULL.
+     */
+    PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);


PlayerCCManager::SetStatus() is now called unconditionally (even when mpStreamAbstractionAAMP is null and even on the GstSubtec OOB path). This is a behavioral change from the previous logic and will break existing unit tests that assert SetStatus() is not invoked pre-tune (e.g., SetCCStatusPreTune* in PrivAampTests). If the goal is only to cache/persist the preference, consider restoring the previous gating (or adding a cache-only API) so CCManager start/stop side effects aren’t triggered when the player isn’t tuned / when GstSubtec handles subtitle rendering.

Suggested change

* Keep persisted CC/subtitle state in sync even after Stop(), when

* mpStreamAbstractionAAMP can be NULL.

*/

PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);

* Keep CC/subtitle state in sync with PlayerCCManager only when the

* AAMP stream abstraction is active and GstSubtec is not handling

* subtitle rendering. This preserves legacy behavior where

* PlayerCCManager::SetStatus() is not invoked pre-tune or on the

* GstSubtec OOB path.

*/

if (mpStreamAbstractionAAMP && !isGstSubtecEnabled)

{

PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);

}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

test/utests/tests/PrivAampTests/PrivAampTests.cpp:4056

In SetVideoMute(true) pre-tune, PrivateInstanceAAMP::SetVideoMute only calls SetCCStatusInternal() when mpStreamAbstractionAAMP is non-null (priv_aamp.cpp:7960-7966). In this test (pre-tune), the EXPECT_CALL(...SetStatus(_)).Times(1) is therefore not asserting behavior of SetVideoMute and will instead be satisfied later by the SetCCStatus(true) call (which invokes SetCCStatusInternal() and may call SetStatus(false) due to video mute). To make the test accurately reflect behavior, either expect SetStatus Times(0) around SetVideoMute(true) and add a separate expectation for the SetCCStatus(true) call, or move/rename the expectation/comment accordingly.

	// Set video mute - should call SetStatus()
	EXPECT_CALL(*g_mockAampGstPlayer, SetSubtitleMute(_)).Times(0);
	EXPECT_CALL(*g_mockPlayerCCManager, SetStatus(_)).Times(1);
	p_aamp->SetVideoMute(true);
	EXPECT_FALSE(p_aamp->GetCCStatus());

	// Enable CC and check that status is stored, 
	p_aamp->SetCCStatusSetByApp();
	p_aamp->SetCCStatus(true);
	EXPECT_TRUE(p_aamp->GetCCStatus());

Copilot · 2026-04-06T13:48:49Z

 	// Now call TuneHelper - subtitles_muted is false (from SetCCStatus(true) above)
 	// SetCCStatusInternal sees mute_subtitles_applied=false, calls SetStatus(true)
-	// RestoreCC(false) reflects the CC manager state was false before this tune
+	// RestoreCC(true) reflects the CC manager state was false before this tune


The updated comment says "RestoreCC(true) reflects the CC manager state was false before this tune", but at this point previousCCEnabled is captured via PlayerCCManager::GetInstance()->GetStatus() inside TuneHelper() (priv_aamp.cpp:6351). Since SetCCStatus(true) now calls SetStatus(true) pre-tune, the CC manager state immediately before TuneHelper() should be true, not false. Please correct the comment to reflect the actual meaning of the RestoreCC(...) argument (restore previous enabled state).

Suggested change

// RestoreCC(true) reflects the CC manager state was false before this tune

// RestoreCC(true) restores the previously enabled CC manager state

Copilot · 2026-04-06T13:48:49Z


 	// During TuneHelper: video is muted so CC is forced off via SetStatus(false)
-	// RestoreCC(false) is called because CC manager internal state was false before this tune
+	// RestoreCC(true) is called because CC manager internal state was false before this tune


The comment claims the CC manager internal state was false before the tune, but in this test SetCCStatus(true) is called pre-tune and (with the new behavior) invokes PlayerCCManager::SetStatus(...), so previousCCEnabled captured in TuneHelper() should reflect that updated state. Please update the comment to match the semantics of RestoreCC(previousCCEnabled) (priv_aamp.cpp:6489).

Suggested change

// RestoreCC(true) is called because CC manager internal state was false before this tune

// RestoreCC(true) is called to restore the CC manager state captured before tune,

// which was enabled by the pre-tune SetCCStatus(true) call in this test

Copilot · 2026-04-06T13:48:50Z

+	/*
+	 * Keep persisted CC/subtitle state in sync even after Stop(), when
+	 * mpStreamAbstractionAAMP can be NULL.
+	 */
+	if (mIsInbandCC || !isGstSubtecEnabled)
+	{
+		PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);
+	}
+	else if (mpStreamAbstractionAAMP)


The new block comment says CC/subtitle state is kept in sync even after Stop() when mpStreamAbstractionAAMP is NULL, but the implementation only performs sync without mpStreamAbstractionAAMP for the in-band (or !GstSubtecEnabled) path. In the OOB + GstSubtecEnabled path, the function does nothing when mpStreamAbstractionAAMP is NULL (due to the else if (mpStreamAbstractionAAMP) guard). Please clarify the comment to match the actual conditions under which state is synchronized.

psiva01 · 2026-04-07T09:55:59Z

Fixed by #1213

VPLAY-13112: CC fails to display on linear channel change, and

cdce527

appears only after D-pad right key press Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>

psiva01 requested a review from a team as a code owner March 26, 2026 18:36

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13112_fix

4618646

psiva01 requested a review from Copilot March 30, 2026 03:39

Copilot started reviewing on behalf of psiva01 March 30, 2026 03:40 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

psiva01 requested a review from jfagunde March 31, 2026 08:00

psiva01 and others added 3 commits April 6, 2026 11:35

Merge branch 'dev_sprint_25_2' into feature/VPLAY-13112_fix

0118f23

Apply suggestion from @Copilot

a0605f7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

VPLAY-13112: L1 fixes

505577a

Signed-off-by: psiva01 <sivasubramanian.patchaiperumal@ltts.com>

psiva01 requested a review from Copilot April 6, 2026 13:44

Copilot started reviewing on behalf of psiva01 April 6, 2026 13:45 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

psiva01 closed this Apr 7, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VPLAY-13112: CC fails to display on linear channel change, and#1247

VPLAY-13112: CC fails to display on linear channel change, and#1247
psiva01 wants to merge 5 commits intodev_sprint_25_2from
feature/VPLAY-13112_fix

psiva01 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

Copilot AI Apr 6, 2026

Uh oh!

psiva01 commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-     * Keep persisted CC/subtitle state in sync even after Stop(), when
-     * mpStreamAbstractionAAMP can be NULL.
-     */
-    PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);
+     * Keep CC/subtitle state in sync with PlayerCCManager only when the
+     * AAMP stream abstraction is active and GstSubtec is not handling
+     * subtitle rendering. This preserves legacy behavior where
+     * PlayerCCManager::SetStatus() is not invoked pre-tune or on the
+     * GstSubtec OOB path.
+     */
+    if (mpStreamAbstractionAAMP && !isGstSubtecEnabled)
+    {
+        PlayerCCManager::GetInstance()->SetStatus(!mute_subtitles_applied);
+    }

	// RestoreCC(true) reflects the CC manager state was false before this tune
	// RestoreCC(true) restores the previously enabled CC manager state

	// RestoreCC(true) is called because CC manager internal state was false before this tune
	// RestoreCC(true) is called to restore the CC manager state captured before tune,
	// which was enabled by the pre-tune SetCCStatus(true) call in this test

Conversation

psiva01 commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

psiva01 commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants