Bringing back support for "queued-frames" and "report-decode-errors"#562
Open
Koky2701 wants to merge 1 commit into
Open
Bringing back support for "queued-frames" and "report-decode-errors"#562Koky2701 wants to merge 1 commit into
Koky2701 wants to merge 1 commit into
Conversation
…properties (#485) Summary: 'report-decode-errors' and 'queued-frames' properties missing from RialtoMSEVideoSink Type: Feature Test Plan: UT/CT, Fullstack Jira: RDKEMW-12692
|
Pull request must be merged with a description containing the required fields, Summary: 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. |
Contributor
There was a problem hiding this comment.
Pull request overview
Reintroduces support for the "report-decode-errors" (decoder property) and "queued-frames" (decoder query) pipeline properties end-to-end: public API → client IPC → server IPC/service → server internal → GStreamer player/tasks, with accompanying unit/component tests and proto updates.
Changes:
- Adds new IPC/proto RPCs and client/server API surface for
setReportDecodeErrorsandgetQueuedFrames. - Implements server-side plumbing down to
GstGenericPlayer(task-based setter; direct getter via decoder property). - Extends unit tests, component tests, mocks, and CMake lists to cover the new APIs.
Reviewed changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.h | Adds test helper declarations for new APIs. |
| tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTestsFixture.cpp | Implements new service test helpers and assertions. |
| tests/unittests/media/server/service/mediaPipelineService/MediaPipelineServiceTests.cpp | Adds service-level unit tests for new APIs. |
| tests/unittests/media/server/mocks/service/MediaPipelineServiceMock.h | Extends service mock interface with new methods. |
| tests/unittests/media/server/mocks/main/MediaPipelineServerInternalMock.h | Extends internal server mock with new methods. |
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h | Adds private-player mock hook for setReportDecodeErrors(). |
| tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerMock.h | Extends GstGenericPlayer mock with new methods. |
| tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h | Adds task-factory mock for createSetReportDecodeErrors. |
| tests/unittests/media/server/main/mediaPipeline/MiscellaneousFunctionsTest.cpp | Adds server media pipeline tests for both new APIs. |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.h | Adds IPC service test helpers for new RPCs. |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTestsFixture.cpp | Implements IPC service test helpers and request/response flows. |
| tests/unittests/media/server/ipc/mediaPipelineModuleService/MediaPipelineModuleServiceTests.cpp | Adds unit tests for new RPC handlers. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/SetReportDecodeErrorsTest.cpp | New unit test for the SetReportDecodeErrors task. |
| tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp | Verifies task factory can create SetReportDecodeErrors task. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp | Adds GstGenericPlayer tests for new setter/getter behavior. |
| tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp | Adds private-player tests for setting decoder property. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GstGenericPlayerTestCommon.h | Adds helper to locate video decoder in tests. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GstGenericPlayerTestCommon.cpp | Implements new video decoder helper. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.h | Adds base helpers for new task tests. |
| tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp | Implements new task test helpers. |
| tests/unittests/media/server/gstplayer/CMakeLists.txt | Registers new gstplayer unit test source. |
| tests/unittests/media/client/mocks/main/MediaPipelineAndControlClientMock.h | Extends client mock API with new methods. |
| tests/unittests/media/client/mocks/ipc/MediaPipelineIpcMock.h | Extends IPC mock with new methods. |
| tests/unittests/media/client/main/mediaPipeline/SetReportDecodeErrorsTest.cpp | New client MediaPipeline unit test for setReportDecodeErrors. |
| tests/unittests/media/client/main/mediaPipeline/MediaPipelineProxyTest.cpp | Extends proxy passthrough test for new methods. |
| tests/unittests/media/client/main/mediaPipeline/GetQueuedFramesTest.cpp | New client MediaPipeline unit test for getQueuedFrames. |
| tests/unittests/media/client/main/CMakeLists.txt | Registers new client main unit test sources. |
| tests/unittests/media/client/ipc/mediaPipelineIpc/SetReportDecodeErrorsTest.cpp | New client IPC unit tests for setReportDecodeErrors call behavior. |
| tests/unittests/media/client/ipc/mediaPipelineIpc/GetQueuedFramesTest.cpp | New client IPC unit tests for getQueuedFrames call behavior. |
| tests/unittests/media/client/ipc/CMakeLists.txt | Registers new client IPC unit test sources. |
| tests/componenttests/server/tests/mediaPipeline/PipelinePropertyTest.cpp | Extends CT plumbing to support uint32 property reads; adjusts step comment. |
| tests/componenttests/client/tests/mse/PipelinePropertyTest.cpp | Extends MSE CT to set/get the new properties. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.h | Adds CT helper methods for new APIs. |
| tests/componenttests/client/tests/base/MediaPipelineTestMethods.cpp | Implements CT helper methods for new APIs. |
| tests/componenttests/client/mocks/MediaPipelineModuleMock.h | Extends CT protobuf module mock for new RPCs and responses. |
| tests/common/matchers/MediaPipelineProtoRequestMatchers.h | Adds proto request matchers for new RPCs. |
| proto/mediapipelinemodule.proto | Adds new messages and RPC declarations for both properties. |
| media/server/service/source/MediaPipelineService.h | Extends service interface with new methods. |
| media/server/service/source/MediaPipelineService.cpp | Implements service methods with session lookup + delegation. |
| media/server/service/include/IMediaPipelineService.h | Extends pure virtual interface with new methods. |
| media/server/main/source/MediaPipelineServerInternal.cpp | Adds internal implementations calling into GstGenericPlayer. |
| media/server/main/include/MediaPipelineServerInternal.h | Declares new internal APIs and documents them. |
| media/server/ipc/source/MediaPipelineModuleService.cpp | Implements new RPC endpoints delegating to MediaPipelineService. |
| media/server/ipc/include/MediaPipelineModuleService.h | Declares new RPC handlers. |
| media/server/gstplayer/source/tasks/generic/SetupElement.cpp | Applies pending report-decode-errors when relevant element is set up. |
| media/server/gstplayer/source/tasks/generic/SetReportDecodeErrors.cpp | New worker-thread task to set pending and apply property. |
| media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp | Wires task creation for SetReportDecodeErrors. |
| media/server/gstplayer/source/GstGenericPlayer.cpp | Implements setReportDecodeErrors (task-based) and getQueuedFrames (decoder property). |
| media/server/gstplayer/interface/IGstGenericPlayer.h | Adds new virtual methods to player interface. |
| media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h | Adds factory method for SetReportDecodeErrors task. |
| media/server/gstplayer/include/tasks/generic/SetReportDecodeErrors.h | New task header. |
| media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h | Declares new factory method override. |
| media/server/gstplayer/include/IGstGenericPlayerPrivate.h | Adds private virtual setReportDecodeErrors() method. |
| media/server/gstplayer/include/GstGenericPlayer.h | Declares new public and private methods. |
| media/server/gstplayer/include/GenericPlayerContext.h | Adds pending state for report-decode-errors. |
| media/server/gstplayer/CMakeLists.txt | Registers new gstplayer task implementation source. |
| media/public/include/IMediaPipeline.h | Adds new public API methods and docs. |
| media/client/main/source/MediaPipeline.cpp | Implements new public API by forwarding to IPC. |
| media/client/main/include/MediaPipelineProxy.h | Exposes new methods on proxy. |
| media/client/main/include/MediaPipeline.h | Declares new public API methods on client pipeline. |
| media/client/ipc/source/MediaPipelineIpc.cpp | Implements new RPC calls and response handling. |
| media/client/ipc/interface/IMediaPipelineIpc.h | Declares new IPC interface methods and docs. |
| media/client/ipc/include/MediaPipelineIpc.h | Declares new IPC class methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+38
to
+51
| void SetReportDecodeErrors::execute() const | ||
| { | ||
| RIALTO_SERVER_LOG_DEBUG("Executing SetReportDecodeErrors for %s source", common::convertMediaSourceType(m_type)); | ||
|
|
||
| m_context.pendingReportDecodeErrorsForVideo = m_reportDecodeErrors; | ||
|
|
||
| if (!m_context.pipeline) | ||
| { | ||
| RIALTO_SERVER_LOG_WARN("Pipeline not available yet - cannot apply report-decode-errors setting"); | ||
| return; | ||
| } | ||
|
|
||
| m_player.setReportDecodeErrors(); | ||
| } |
Comment on lines
+609
to
+617
| auto sourceIter = std::find_if(m_attachedSources.begin(), m_attachedSources.end(), | ||
| [sourceId](const auto &src) { return src.second == sourceId; }); | ||
| if (sourceIter == m_attachedSources.end()) | ||
| { | ||
| RIALTO_SERVER_LOG_ERROR("Failed - Source not found"); | ||
| return false; | ||
| } | ||
| return m_gstPlayer->getQueuedFrames(queuedFrames); | ||
| } |
| auto mediaPipelineIter = m_mediaPipelines.find(sessionId); | ||
| if (mediaPipelineIter == m_mediaPipelines.end()) | ||
| { | ||
| RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId); |
| auto mediaPipelineIter = m_mediaPipelines.find(sessionId); | ||
| if (mediaPipelineIter == m_mediaPipelines.end()) | ||
| { | ||
| RIALTO_SERVER_LOG_ERROR("Session with id: %d does not exists", sessionId); |
Comment on lines
+424
to
+433
| /** | ||
| * @brief Gets the queued frames for this source. | ||
| * | ||
| * This method is asynchronous | ||
| * | ||
| * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource() | ||
| * @param[in] queuedFrames : Number of queued frames | ||
| * | ||
| * @retval true on success. | ||
| */ |
Comment on lines
+417
to
+419
| * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource() | ||
| * @param[in] reportDecodeErrors : The desired Set Report Decode Errors mode on the sink | ||
| * |
Comment on lines
+1236
to
+1238
| * @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource() | ||
| * @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink | ||
| * |
|
|
||
| void MediaPipelineServiceTests::mediaPipelineWillGetQueuedFrames() | ||
| { | ||
| EXPECT_CALL(m_mediaPipelineMock, getQueuedFrames(_, _)).WillOnce(DoAll(SetArgReferee<1>(123), Return(true))); |
|
Coverage statistics of your commit: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bringing back support for "queued-frames" and "report-decode-errors" properties (#485)
Summary: 'report-decode-errors' and 'queued-frames' properties missing from RialtoMSEVideoSink
Type: Feature
Test Plan: UT/CT, Fullstack
Jira: RDKEMW-12692