Skip to content

[VIPA][Rialto]gstDecode regression in RDKE Sprint NG builds#475

Open
rekhap2kandhavelan wants to merge 14 commits intomasterfrom
feature/RDKEMW-13498
Open

[VIPA][Rialto]gstDecode regression in RDKE Sprint NG builds#475
rekhap2kandhavelan wants to merge 14 commits intomasterfrom
feature/RDKEMW-13498

Conversation

@rekhap2kandhavelan
Copy link
Copy Markdown

@rekhap2kandhavelan rekhap2kandhavelan commented Apr 13, 2026

[VIPA][Rialto]gstDecode regression in RDKE Sprint NG builds

Summary: retain preroll frames count to resolve vipa tune time issue
Type: Fix
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-13498

facing yoututbe test fail for frame dropped so increasing the preroll frames
Copilot AI review requested due to automatic review settings April 13, 2026 11:02
@github-actions

This comment was marked as resolved.

@github-actions
Copy link
Copy Markdown

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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adjusts server-side NeedMediaData behavior to request fewer frames while the pipeline is not yet playing (preroll/non-playing), aiming to address a gstDecode regression in RDKE Sprint NG builds by retaining an intended preroll frame request count.

Changes:

  • Dynamically sets NeedMediaData’s requested frame count to a smaller value when playback state is not PLAYING.
  • Introduces a new shared constant kPrerollNumFrames to represent the preroll frame request count.

Reviewed changes

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

File Description
media/server/main/source/NeedMediaData.cpp Chooses between max frames vs preroll frames based on current playback state.
media/server/main/include/ShmUtils.h Adds kPrerollNumFrames constant alongside existing SHM sizing constants.

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

Comment thread media/server/main/source/NeedMediaData.cpp Outdated
Comment thread media/server/main/source/NeedMediaData.cpp
Comment thread media/server/main/include/ShmUtils.h
@svc-rdkeportal01
Copy link
Copy Markdown

CCI-Build-Verified -1
sha: ac271e6

Copilot AI review requested due to automatic review settings April 19, 2026 13:42
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Copilot AI review requested due to automatic review settings April 20, 2026 10:36
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment thread tests/unittests/media/server/main/needMediaData/NeedMediaDataTestsFixture.h Outdated
Comment thread tests/unittests/media/server/main/mediaPipeline/CallbackTest.cpp
Comment thread tests/unittests/media/server/main/needMediaData/NeedMediaDataTestsFixture.h Outdated
@github-actions
Copy link
Copy Markdown

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

@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 84.5%
Functions coverage stays unchanged and is: 92.6%

@rekhap2kandhavelan
Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@rekhap2kandhavelan rekhap2kandhavelan changed the title RDKEMW-13498 : [VIPA][Rialto]gstDecode regression in RDKE Sprint NG builds [VIPA][Rialto]gstDecode regression in RDKE Sprint NG builds Apr 22, 2026
@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 84.5%
Functions coverage stays unchanged and is: 92.6%

Copilot AI review requested due to automatic review settings April 22, 2026 09:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.


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

Copilot AI review requested due to automatic review settings April 22, 2026 11:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment on lines 31 to 35
constexpr std::uint32_t kMetadataOffset{1024};
constexpr int kRequestId{0};
constexpr int kPrerollingNumFrames{10};
constexpr int kMaxFrames{24};
constexpr int kMaxMetadataBytes{2500};
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

The new test hard-codes the preroll frame count (kPrerollingNumFrames{10}) even though production now defines kPrerollNumFrames in ShmUtils.h. To avoid tests drifting from production behavior, consider reusing the production constant (or including it via a shared header) rather than duplicating the value here.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +38
TEST_F(NeedMediaDataTests, shouldSendMessageInPrerollingState)
{
initialize(firebolt::rialto::PlaybackState::PAUSED);
needMediaDataWillBeSentBelowPlayingState();
}
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

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

This test is named "shouldSendMessageInPrerollingState" but it initializes the SUT with PlaybackState::PAUSED. Since PAUSED is a distinct state (and not described as prerolling in the PlaybackState docs), the name is misleading—either use a state that actually represents preroll behavior in this codebase or rename the test to match the state being exercised (e.g. paused/below-playing).

Copilot uses AI. Check for mistakes.
Comment thread cmake/googletest.cmake Outdated
@github-actions
Copy link
Copy Markdown

Coverage statistics of your commit:
Lines coverage stays unchanged and is: 84.5%
Functions coverage stays unchanged and is: 92.6%

rkandh015 and others added 2 commits April 22, 2026 16:52
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 22, 2026 11:35
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

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.

4 participants