Skip to content
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions media/client/ipc/include/MediaPipelineIpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ class MediaPipelineIpc : public IMediaPipelineIpc, public IpcModule

bool setImmediateOutput(int32_t sourceId, bool immediateOutput) override;

bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) override;

bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) override;

bool getImmediateOutput(int32_t sourceId, bool &immediateOutput) override;

bool getStats(int32_t sourceId, uint64_t &renderedFrames, uint64_t &droppedFrames) override;
Expand Down
24 changes: 24 additions & 0 deletions media/client/ipc/interface/IMediaPipelineIpc.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,30 @@ class IMediaPipelineIpc
*/
virtual bool setImmediateOutput(int32_t sourceId, bool immediateOutput) = 0;

/**
* @brief Sets the "Report Decode Errors" property for this source.
*
* This method is asynchronous, it will set the "Report Decode Errors" property
*
* @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
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The documentation incorrectly states that the property is set "on the sink". The report_decode_errors property is set on the decoder, not the sink. This should be corrected to "Set Report Decode Errors mode on the decoder".

Suggested change
* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink
* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the decoder

Copilot uses AI. Check for mistakes.
*
* @retval true on success.
*/
virtual bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) = 0;

/**
* @brief Gets the queued frames for this source.
*
* This method is synchronous, it gets the queued frames property
*
* @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()
* @param[out] queuedFrames : Get queued frames on the decoder
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The documentation says "Get queued frames on the decoder" but should be more descriptive for an output parameter. It should read "The number of queued frames on the decoder" or "Returns the number of queued frames on the decoder" to match the documentation style for similar getter methods.

Suggested change
* @param[out] queuedFrames : Get queued frames on the decoder
* @param[out] queuedFrames : The number of queued frames on the decoder

Copilot uses AI. Check for mistakes.
*
* @retval true on success.
*/
virtual bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) = 0;

Comment on lines 191 to 214
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The MediaPipelineIpcMock is missing MOCK_METHOD declarations for the new setReportDecodeErrors and getQueuedFrames methods. These need to be added after the setImmediateOutput/getImmediateOutput methods to maintain consistency with the interface and enable proper unit testing.

Copilot uses AI. Check for mistakes.
/**
* @brief Gets the "Immediate Output" property for this source.
*
Expand Down
67 changes: 67 additions & 0 deletions media/client/ipc/source/MediaPipelineIpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,73 @@ bool MediaPipelineIpc::setImmediateOutput(int32_t sourceId, bool immediateOutput
return true;
}

bool MediaPipelineIpc::setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors)
{
if (!reattachChannelIfRequired())
{
RIALTO_CLIENT_LOG_ERROR("Reattachment of the ipc channel failed, ipc disconnected");
return false;
}

firebolt::rialto::ReportDecodeErrorsRequest request;

request.set_session_id(m_sessionId);
request.set_source_id(sourceId);
request.set_report_decode_errors(reportDecodeErrors);

firebolt::rialto::ReportDecodeErrorsResponse response;
auto ipcController = m_ipc.createRpcController();
auto blockingClosure = m_ipc.createBlockingClosure();
m_mediaPipelineStub->setReportDecodeErrors(ipcController.get(), &request, &response, blockingClosure.get());

// wait for the call to complete
blockingClosure->wait();

// check the result
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to set report decode error due to '%s'", ipcController->ErrorText().c_str());
return false;
}

return true;
}
Comment on lines +570 to +600
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The new setReportDecodeErrors method is missing unit test coverage. Test files should be added similar to SetImmediateOutputTest.cpp to verify success, failure, channel disconnection, and reconnection scenarios.

Copilot uses AI. Check for mistakes.

bool MediaPipelineIpc::getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames)
{
if (!reattachChannelIfRequired())
{
RIALTO_CLIENT_LOG_ERROR("Reattachment of the ipc channel failed, ipc disconnected");
return false;
}

firebolt::rialto::GetQueuedFramesRequest request;

request.set_session_id(m_sessionId);
request.set_source_id(sourceId);

firebolt::rialto::GetQueuedFramesResponse response;
auto ipcController = m_ipc.createRpcController();
auto blockingClosure = m_ipc.createBlockingClosure();
m_mediaPipelineStub->getQueuedFrames(ipcController.get(), &request, &response, blockingClosure.get());

// wait for the call to complete
blockingClosure->wait();

// check the result
if (ipcController->Failed())
{
RIALTO_CLIENT_LOG_ERROR("failed to get queued frames due to '%s'", ipcController->ErrorText().c_str());
return false;
}
else
{
queuedFrames = response.queued_frames();
}

return true;
}
Comment on lines 602 to 635
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The new getQueuedFrames method is missing unit test coverage. Test files should be added to verify success, failure, channel disconnection, reconnection scenarios, and response parsing.

Copilot uses AI. Check for mistakes.

bool MediaPipelineIpc::getImmediateOutput(int32_t sourceId, bool &immediateOutput)
{
if (!reattachChannelIfRequired())
Expand Down
4 changes: 4 additions & 0 deletions media/client/main/include/MediaPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ class MediaPipeline : public IMediaPipelineAndIControlClient, public IMediaPipel

bool setImmediateOutput(int32_t sourceId, bool immediateOutput) override;

bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) override;

bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) override;

bool getImmediateOutput(int32_t sourceId, bool &immediateOutput) override;

bool getStats(int32_t sourceId, uint64_t &renderedFrames, uint64_t &droppedFrames) override;
Expand Down
8 changes: 8 additions & 0 deletions media/client/main/include/MediaPipelineProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ class MediaPipelineProxy : public IMediaPipelineAndIControlClient
{
return m_mediaPipeline->setImmediateOutput(sourceId, immediateOutput);
}
bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors)
{
return m_mediaPipeline->setReportDecodeErrors(sourceId, reportDecodeErrors);
}
bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames)
{
return m_mediaPipeline->getQueuedFrames(sourceId, queuedFrames);
}
bool getImmediateOutput(int32_t sourceId, bool &immediateOutput)
{
return m_mediaPipeline->getImmediateOutput(sourceId, immediateOutput);
Expand Down
10 changes: 10 additions & 0 deletions media/client/main/source/MediaPipeline.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,16 @@ bool MediaPipeline::setImmediateOutput(int32_t sourceId, bool immediateOutput)
return m_mediaPipelineIpc->setImmediateOutput(sourceId, immediateOutput);
}

bool MediaPipeline::setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors)
{
return m_mediaPipelineIpc->setReportDecodeErrors(sourceId, reportDecodeErrors);
}

bool MediaPipeline::getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames)
{
return m_mediaPipelineIpc->getQueuedFrames(sourceId, queuedFrames);
}
Comment on lines +316 to +324
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The new setReportDecodeErrors and getQueuedFrames methods are missing unit test coverage. Tests should be added to verify the forwarding to the IPC layer.

Copilot uses AI. Check for mistakes.

bool MediaPipeline::getImmediateOutput(int32_t sourceId, bool &immediateOutput)
{
return m_mediaPipelineIpc->getImmediateOutput(sourceId, immediateOutput);
Expand Down
24 changes: 24 additions & 0 deletions media/public/include/IMediaPipeline.h
Original file line number Diff line number Diff line change
Expand Up @@ -1227,6 +1227,30 @@ class IMediaPipeline
*/
virtual bool setImmediateOutput(int32_t sourceId, bool immediateOutput) = 0;

/**
* @brief Sets the "Report Decode Errors" property for this source.
*
* This method is asynchronous, it will set the "Report Decode Errors" property
*
* @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
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The documentation incorrectly states that the property is set "on the sink". According to the PR description and the implementation, the report_decode_errors property is set on the decoder, not the sink. This should be corrected to "Set Report Decode Errors mode on the decoder" to accurately reflect where the property is applied.

Suggested change
* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the sink
* @param[in] reportDecodeErrors : Set Report Decode Errors mode on the decoder

Copilot uses AI. Check for mistakes.
*
* @retval true on success.
*/
virtual bool setReportDecodeErrors(int32_t sourceId, bool reportDecodeErrors) = 0;

/**
* @brief Gets the queued frames for this source.
*
* This method is synchronous, it gets the queued frames property
*
* @param[in] sourceId : The source id. Value should be set to the MediaSource.id returned after attachSource()
* @param[out] queuedFrames : Get queued frames on the decoder
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The documentation says "Get queued frames on the decoder" but this should be more clear about the parameter direction. It should read "The number of queued frames on the decoder" or "Returns the number of queued frames on the decoder" to match the style of similar getter documentation in this interface.

Suggested change
* @param[out] queuedFrames : Get queued frames on the decoder
* @param[out] queuedFrames : The number of queued frames on the decoder

Copilot uses AI. Check for mistakes.
*
* @retval true on success.
*/
virtual bool getQueuedFrames(int32_t sourceId, uint32_t &queuedFrames) = 0;

/**
* @brief Gets the "Immediate Output" property for this source.
*
Expand Down
1 change: 1 addition & 0 deletions media/server/gstplayer/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ add_library(
source/tasks/generic/SetMute.cpp
source/tasks/generic/SetPlaybackRate.cpp
source/tasks/generic/SetPosition.cpp
source/tasks/generic/SetReportDecodeErrors.cpp
source/tasks/generic/SetSourcePosition.cpp
source/tasks/generic/SetSubtitleOffset.cpp
source/tasks/generic/SetStreamSyncMode.cpp
Expand Down
5 changes: 5 additions & 0 deletions media/server/gstplayer/include/GenericPlayerContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,11 @@ struct GenericPlayerContext
*/
std::optional<bool> pendingImmediateOutputForVideo{};

/**
* @brief Pending report decode errors for MediaSourceType::VIDEO
*/
std::optional<bool> pendingReportDecodeErrorsForVideo{};

/**
* @brief Pending low latency
*/
Expand Down
3 changes: 3 additions & 0 deletions media/server/gstplayer/include/GstGenericPlayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,9 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva
void setPlaybackRate(double rate) override;
bool getPosition(std::int64_t &position) override;
bool setImmediateOutput(const MediaSourceType &mediaSourceType, bool immediateOutput) override;
bool setReportDecodeErrors(const MediaSourceType &mediaSourceType, bool reportDecodeErrors) override;
bool getImmediateOutput(const MediaSourceType &mediaSourceType, bool &immediateOutput) override;
bool getQueuedFrames(const MediaSourceType &mediaSourceType, uint32_t &queuedFrames) override;
bool getStats(const MediaSourceType &mediaSourceType, uint64_t &renderedFrames, uint64_t &droppedFrames) override;
void setVolume(double targetVolume, uint32_t volumeDuration, firebolt::rialto::EaseType easeType) override;
bool getVolume(double &volume) override;
Expand Down Expand Up @@ -153,6 +155,7 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva
void scheduleAllSourcesAttached() override;
bool setVideoSinkRectangle() override;
bool setImmediateOutput() override;
bool setReportDecodeErrors() override;
bool setShowVideoWindow() override;
bool setLowLatency() override;
bool setSync() override;
Expand Down
7 changes: 7 additions & 0 deletions media/server/gstplayer/include/IGstGenericPlayerPrivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ class IGstGenericPlayerPrivate
*/
virtual bool setImmediateOutput() = 0;

/**
* @brief Sets report decode error. Called by the worker thread.
*
* @retval true on success.
*/
virtual bool setReportDecodeErrors() = 0;
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The interface method signature declares a parameter bool reportDecodeErrors, but this should not take any parameters following the same pattern as setImmediateOutput(). The implementation in GstGenericPlayer::setReportDecodeErrors() (private method on line 1380 of GstGenericPlayer.cpp) also doesn't use parameters - it reads the value from m_context.pendingReportDecodeErrorsForVideo. The parameter should be removed from this interface declaration.

Suggested change
virtual bool setReportDecodeErrors() = 0;
virtual bool setReportDecodeErrors() = 0;

Copilot uses AI. Check for mistakes.

/**
* @brief Sets the low latency property. Called by the worker thread.
*
Expand Down
15 changes: 15 additions & 0 deletions media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,21 @@ class IGenericPlayerTaskFactory
const firebolt::rialto::MediaSourceType &type,
bool immediateOutput) const = 0;

/**
* @brief Creates a SetReportDecodeErrors task.
*
* @param[in] context : The GstPlayer context
* @param[in] player : The GstPlayer instance
* @param[in] type : The media source type
* @param[in] reportDecodeErrors : the value to set for report decode error
*
* @retval the new SetReportDecodeErrors task instance.
*/
virtual std::unique_ptr<IPlayerTask> createSetReportDecodeErrors(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player,
const firebolt::rialto::MediaSourceType &type,
bool reportDecodeErrors) const = 0;

/**
* @brief Creates a SetBufferingLimit task.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ class GenericPlayerTaskFactory : public IGenericPlayerTaskFactory
std::unique_ptr<IPlayerTask> createSetImmediateOutput(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
const firebolt::rialto::MediaSourceType &type,
bool immediateOutput) const override;
std::unique_ptr<IPlayerTask> createSetReportDecodeErrors(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player,
const firebolt::rialto::MediaSourceType &type,
bool reportDecodeErrors) const override;
std::unique_ptr<IPlayerTask> createSetBufferingLimit(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
std::uint32_t limit) const override;
std::unique_ptr<IPlayerTask> createSetUseBuffering(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:
*
* Copyright 2026 Sky UK
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The copyright year is inconsistent across the new files. This file has "Copyright 2026 Sky UK" while other new files in this PR have "Copyright 2025 Sky UK" and "Copyright 2024 Sky UK". Since the PR is being created in early 2025 (based on context), the copyright year should be "2025" to match other recently created files in the PR.

Suggested change
* Copyright 2026 Sky UK
* Copyright 2025 Sky UK

Copilot uses AI. Check for mistakes.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#ifndef FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERRORS_H_
#define FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERRORS_H_

#include "GenericPlayerContext.h"
#include "IGlibWrapper.h"
#include "IGstGenericPlayerPrivate.h"
#include "IGstWrapper.h"
#include "IPlayerTask.h"

#include <memory>

namespace firebolt::rialto::server::tasks::generic
{
class SetReportDecodeErrors : public IPlayerTask
{
public:
explicit SetReportDecodeErrors(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
const MediaSourceType &type, bool reportDecodeErrors);
~SetReportDecodeErrors() override;
void execute() const override;

private:
GenericPlayerContext &m_context;
IGstGenericPlayerPrivate &m_player;
const MediaSourceType m_type;
bool m_reportDecodeErrors;
};
} // namespace firebolt::rialto::server::tasks::generic

#endif // FIREBOLT_RIALTO_SERVER_TASKS_GENERIC_SET_REPORT_DECODE_ERRORS_H_
20 changes: 20 additions & 0 deletions media/server/gstplayer/interface/IGstGenericPlayer.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,26 @@ class IGstGenericPlayer
*/
virtual bool setImmediateOutput(const MediaSourceType &mediaSourceType, bool immediateOutput) = 0;

/**
* @brief Sets the "Report Decode Error" property for this source.
*
* @param[in] mediaSourceType : The media source type
* @param[in] reportDecodeErrors : Set report decode error
*
* @retval true on success.
*/
virtual bool setReportDecodeErrors(const MediaSourceType &mediaSourceType, bool reportDecodeErrors) = 0;

/**
* @brief Gets the queued frames for this source.
*
* @param[in] mediaSourceType : The media source type
* @param[out] queuedFrames : Get queued frames mode on the decoder
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

The documentation is unclear and inconsistent. The phrase "Get queued frames mode on the decoder" is confusing - it should be "The number of queued frames on the decoder" to match the similar documentation pattern used elsewhere in the codebase (e.g., getStats uses "The number of rendered frames").

Suggested change
* @param[out] queuedFrames : Get queued frames mode on the decoder
* @param[out] queuedFrames : The number of queued frames on the decoder

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

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

The description "Get queued frames mode on the decoder" is awkwardly phrased. It should be "Number of queued frames on the decoder" or "Gets the number of queued frames on the decoder" to be clearer and match similar documentation patterns in the codebase.

Suggested change
* @param[out] queuedFrames : Get queued frames mode on the decoder
* @param[out] queuedFrames : Number of queued frames on the decoder

Copilot uses AI. Check for mistakes.
*
* @retval true on success.
*/
virtual bool getQueuedFrames(const MediaSourceType &mediaSourceType, uint32_t &queuedFrames) = 0;

Comment on lines 199 to 217
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The GstGenericPlayerMock is missing MOCK_METHOD declarations for the new setReportDecodeErrors and getQueuedFrames methods. These need to be added to maintain consistency with the interface and enable proper unit testing.

Copilot uses AI. Check for mistakes.
/**
* @brief Gets the "Immediate Output" property for this source.
*
Expand Down
Loading
Loading