Fixed playback info reporting for not asynchronous playbacks by aczs · Pull Request #435 · rdkcentral/rialto

aczs · 2026-01-14T09:58:40Z

Summary: Fixed playback info reporting for not asynchronous playbacks
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: ENTDAI-2231

github-actions · 2026-01-14T09:58:50Z

Pull request must be merged with a description containing the required fields,

Summary:
Type: Feature/Fix/Cleanup
Test Plan:
Jira:

If there is no jira releated to this change, please put 'Jira: NO-JIRA'.

Description can be changed by editing the top comment on your pull request and making a new commit.

Copilot

Pull request overview

This PR separates the playback info reporting timer from the position reporting timer to fix playback info reporting for non-asynchronous playbacks. Previously, the playback info timer was created and managed together with the position reporting timer, but now they are managed independently with dedicated start/stop methods.

Changes:

Introduced new startNotifyPlaybackInfoTimer() and stopNotifyPlaybackInfoTimer() methods to manage the playback info timer separately
Modified state transition handling to start the playback info timer when entering PAUSED state and stop it when entering READY state
Updated tests to reflect the separation of the two timers

Reviewed changes

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

Show a summary per file

File	Description
`media/server/gstplayer/include/IGstGenericPlayerPrivate.h`	Added interface methods for starting/stopping the playback info timer
`media/server/gstplayer/include/GstGenericPlayer.h`	Declared new timer control methods
`media/server/gstplayer/source/GstGenericPlayer.cpp`	Implemented separate timer management with dedicated start/stop methods
`media/server/gstplayer/source/tasks/generic/HandleBusMessage.cpp`	Updated state transition logic to control the playback info timer appropriately
`media/server/gstplayer/source/tasks/generic/Stop.cpp`	Added call to stop playback info timer on stop
`media/server/gstplayer/source/tasks/generic/Flush.cpp`	Removed unnecessary stopPositionReportingAndCheckAudioUnderflowTimer call
`media/server/gstplayer/source/tasks/generic/RemoveSource.cpp`	Unchanged in diff but referenced by tests
`tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h`	Added mock methods for new timer control functions
`tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp`	Added comprehensive tests for separate timer management
`tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/HandleBusMessageTest.cpp`	Updated expectations to reflect separated timer control
`tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/RemoveSourceTest.cpp`	Updated test expectations for timer control during source removal
`tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h`	Added helper method declaration
`tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp`	Implemented helper method and updated flush expectations

Comments suppressed due to low confidence (1)

media/server/gstplayer/source/tasks/generic/HandleBusMessage.cpp:122

The playback info timer is not stopped when entering PLAYING state. Since it's started in PAUSED state and should report playback info only when paused, it should be stopped here to avoid unnecessary periodic notifications during playback. Add m_player.stopNotifyPlaybackInfoTimer(); before or after line 122.

            case GST_STATE_PLAYING:
            {
                m_context.flushOnPrerollController.stateReached(newState);
                m_player.executePostponedFlushes();
                // If async flush was requested before HandleBusMessage task creation (but it was not executed yet)
                // or if async flush was created after HandleBusMessage task creation (but before its execution)
                // we can't report playback state, because async flush causes state loss - reported state is probably invalid.
                if (m_isAsyncFlushOngoingDuringCreation || m_flushWatcher.isAsyncFlushOngoing())
                {
                    RIALTO_SERVER_LOG_WARN("Skip PLAYING notification - flush is ongoing");
                    break;
                }
                if (m_context.pendingPlaybackRate != kNoPendingPlaybackRate)
                {
                    m_player.setPendingPlaybackRate();
                }
                m_player.startPositionReportingAndCheckAudioUnderflowTimer();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T10:06:14Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp

-}
+
+    m_sut->startNotifyPlaybackInfoTimer();
+    m_sut->stopNotifyPlaybackInfoTimer();}


Missing newline before closing brace. The closing brace should be on its own line for consistency with other test methods.

Suggested change

m_sut->stopNotifyPlaybackInfoTimer();}

m_sut->stopNotifyPlaybackInfoTimer();

}

Copilot · 2026-01-14T10:06:14Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp

+    m_sut->stopPositionReportingAndCheckAudioUnderflowTimer();
+}
+
+TEST_F(GstGenericPlayerPrivateTest, shouldStopActivePlaybackInfoTimerTimer)


The test name has a duplicated 'Timer' suffix ('PlaybackInfoTimerTimer'). It should be 'shouldStopActivePlaybackInfoTimer' to match the naming pattern of similar tests.

Copilot · 2026-01-14T10:06:15Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp

+    m_sut->stopPositionReportingAndCheckAudioUnderflowTimer();
+}
+
+TEST_F(GstGenericPlayerPrivateTest, shouldNotStopInactiveActivePlaybackInfoTimer)


The test name contains both 'Inactive' and 'Active' which is contradictory. It should be 'shouldNotStopInactivePlaybackInfoTimer' to match the pattern of 'shouldNotStopInactivePositionReportingTimer'.

Suggested change

TEST_F(GstGenericPlayerPrivateTest, shouldNotStopInactiveActivePlaybackInfoTimer)

TEST_F(GstGenericPlayerPrivateTest, shouldNotStopInactivePlaybackInfoTimer)

github-actions · 2026-01-14T10:08:22Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T11:58:54Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T12:13:40Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

github-actions · 2026-01-14T12:45:40Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

Copilot

Pull request overview

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

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T13:15:35Z

tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp

+    m_sut->stopPositionReportingAndCheckAudioUnderflowTimer();
+}
+
+TEST_F(GstGenericPlayerPrivateTest, shouldStopActivePlaybackInfoTimerTimer)


Test name contains a redundant "Timer" suffix. The name should be "shouldStopActivePlaybackInfoTimer" instead of "shouldStopActivePlaybackInfoTimerTimer".

Suggested change

TEST_F(GstGenericPlayerPrivateTest, shouldStopActivePlaybackInfoTimerTimer)

TEST_F(GstGenericPlayerPrivateTest, shouldStopActivePlaybackInfoTimer)

github-actions · 2026-01-14T13:20:11Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

Copilot

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T13:23:05Z

media/server/gstplayer/source/GstGenericPlayer.cpp


+void GstGenericPlayer::startNotifyPlaybackInfoTimer()
+{
+    static constexpr std::chrono::milliseconds kPlaybackInfoTimerMs{32};


The constant kPlaybackInfoTimerMs is defined locally inside startNotifyPlaybackInfoTimer(), while the similar constant kPositionReportTimerMs is defined at file scope (line 44). For consistency and maintainability, consider moving kPlaybackInfoTimerMs to file scope alongside kPositionReportTimerMs.

github-actions · 2026-01-14T13:28:17Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T13:29:33Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T13:32:34Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T14:10:33Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T14:37:55Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T14:39:35Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

This reverts commit a4dfd51.

github-actions · 2026-01-14T14:50:43Z

tests/componenttests/server/tests/mediaPipeline/FlushTest.cpp:59:0: style: The function 'willFlush' is never used. [unusedFunction]
void willFlush()
^
nofile:0:0: information: Active checkers: 161/592 (use --checkers-report= to see details) [checkersReport]

github-actions · 2026-01-14T14:52:05Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T14:58:12Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T15:00:40Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

github-actions · 2026-01-14T15:05:42Z

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 85.2%
Functions coverage stays unchanged and is: 92.5%

Fix

32fb320

Copilot AI review requested due to automatic review settings January 14, 2026 09:58

Copilot started reviewing on behalf of aczs January 14, 2026 09:59 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

skywojciechowskim previously approved these changes Jan 14, 2026

View reviewed changes

small fixes

a2630ff

aczs dismissed skywojciechowskim’s stale review via a2630ff January 14, 2026 11:49

small fixes

10201b2

Copilot AI review requested due to automatic review settings January 14, 2026 12:04

Copilot started reviewing on behalf of aczs January 14, 2026 12:04 View session

small fixes

a20a66a

Copilot AI reviewed Jan 14, 2026

View reviewed changes

small fixes

f2e3ac6

Copilot AI review requested due to automatic review settings January 14, 2026 13:10

Copilot started reviewing on behalf of aczs January 14, 2026 13:11 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

aczs added 2 commits January 14, 2026 13:18

small fixes

af92a35

small fixes

5181d53

Copilot AI review requested due to automatic review settings January 14, 2026 13:19

Copilot started reviewing on behalf of aczs January 14, 2026 13:20 View session

small fixes

a42db23

Copilot AI reviewed Jan 14, 2026

View reviewed changes

small fixes

183e3a4

small fixes

a4dfd51

Copilot AI review requested due to automatic review settings January 14, 2026 14:28

Copilot started reviewing on behalf of aczs January 14, 2026 14:28 View session

Merge branch 'master' into PostionReportingInAsyncFalseFix

a5c4da9

aczs added 3 commits January 14, 2026 14:40

Revert "small fixes"

356340f

This reverts commit a4dfd51.

small fixes

463f79b

small fixes

137deed

skywojciechowskim previously approved these changes Jan 14, 2026

View reviewed changes

small fixes

7a7260d

aczs dismissed skywojciechowskim’s stale review via 7a7260d January 14, 2026 14:50

small fixes

09d3e2c

skywojciechowskim approved these changes Jan 14, 2026

View reviewed changes

aczs merged commit e918186 into master Jan 14, 2026
50 checks passed

github-actions bot locked and limited conversation to collaborators Jan 14, 2026

	m_sut->stopNotifyPlaybackInfoTimer();}
	m_sut->stopNotifyPlaybackInfoTimer();
	}

	TEST_F(GstGenericPlayerPrivateTest, shouldNotStopInactiveActivePlaybackInfoTimer)
	TEST_F(GstGenericPlayerPrivateTest, shouldNotStopInactivePlaybackInfoTimer)

Comments

Conversation

aczs commented Jan 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

github-actions bot commented Jan 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

aczs commented Jan 14, 2026 •

edited

Loading