Skip to content

RDKEMW-15424: [RDKE] Workaround for Audioloss in BCM decoder #461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
balasaraswathy-n wants to merge 1 commit into
base: master
Choose a base branch
from
Open

RDKEMW-15424: [RDKE] Workaround for Audioloss in BCM decoder #461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 1 addition & 9 deletions media/server/gstplayer/include/GstGenericPlayer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva
bool reattachSource(const std::unique_ptr<IMediaPipeline::MediaSource> &source) override;
bool hasSourceType(const MediaSourceType &mediaSourceType) const override;
GstElement *getSink(const MediaSourceType &mediaSourceType) const override;
GstElement *getDecoder(const MediaSourceType &mediaSourceType) override;
void setSourceFlushed(const MediaSourceType &mediaSourceType) override;
bool isAsync(const MediaSourceType &mediaSourceType) const;
void notifyPlaybackInfo() override;
Expand Down Expand Up @@ -297,15 +298,6 @@ class GstGenericPlayer : public IGstGenericPlayer, public IGstGenericPlayerPriva
*/
GstElement *getSinkChildIfAutoAudioSink(GstElement *sink) const;

/**
* @brief Gets the decoder element for source type.
*
* @param[in] mediaSourceType : the source type to obtain the decoder for
*
* @retval The decoder, NULL if not found
*/
GstElement *getDecoder(const MediaSourceType &mediaSourceType);

/**
* @brief Gets the parser element for source type.
*
Expand Down
9 changes: 9 additions & 0 deletions media/server/gstplayer/include/IGstGenericPlayerPrivate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,15 @@ class IGstGenericPlayerPrivate
*/
virtual GstElement *getSink(const MediaSourceType &mediaSourceType) const = 0;

/**
* @brief Gets the decoder element for source type.
*
* @param[in] mediaSourceType : the source type to obtain the decoder for
*
* @retval The decoder, NULL if not found. Please call getObjectUnref() if it's non-null
*/
virtual GstElement *getDecoder(const MediaSourceType &mediaSourceType) = 0;
Comment on lines +291 to +298

/**
* @brief Pushes GstSample if playback position has changed or new segment needs to be sent.
*
Expand Down
5 changes: 4 additions & 1 deletion media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,11 +202,14 @@ class IGenericPlayerTaskFactory
* @brief Creates a SetPlaybackRate task.
*
* @param[in] context : The GstGenericPlayer context
* @param[in] player : The GstGenericPlayer instance
* @param[in] rate : The new playback rate.
*
* @retval the new SetPlaybackRate task instance.
*/
virtual std::unique_ptr<IPlayerTask> createSetPlaybackRate(GenericPlayerContext &context, double rate) const = 0;
virtual std::unique_ptr<IPlayerTask> createSetPlaybackRate(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player,
double rate) const = 0;
Comment on lines 204 to +212

/**
* @brief Creates a SetPosition task.
Expand Down
3 changes: 2 additions & 1 deletion media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ class GenericPlayerTaskFactory : public IGenericPlayerTaskFactory
IGstGenericPlayerPrivate &player) const override;
std::unique_ptr<IPlayerTask> createCheckAudioUnderflow(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player) const override;
std::unique_ptr<IPlayerTask> createSetPlaybackRate(GenericPlayerContext &context, double rate) const override;
std::unique_ptr<IPlayerTask> createSetPlaybackRate(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
double rate) const override;
std::unique_ptr<IPlayerTask> createSetPosition(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
std::int64_t position) const override;
std::unique_ptr<IPlayerTask> createSetupElement(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
Expand Down
5 changes: 4 additions & 1 deletion media/server/gstplayer/include/tasks/generic/SetPlaybackRate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#include "GenericPlayerContext.h"
#include "IGlibWrapper.h"
#include "IGstGenericPlayerPrivate.h"
#include "IGstWrapper.h"
#include "IPlayerTask.h"
#include <memory>
Expand All @@ -31,13 +32,15 @@ namespace firebolt::rialto::server::tasks::generic
class SetPlaybackRate : public IPlayerTask
{
public:
SetPlaybackRate(GenericPlayerContext &context, std::shared_ptr<firebolt::rialto::wrappers::IGstWrapper> gstWrapper,
SetPlaybackRate(GenericPlayerContext &context, IGstGenericPlayerPrivate &player,
std::shared_ptr<firebolt::rialto::wrappers::IGstWrapper> gstWrapper,
std::shared_ptr<firebolt::rialto::wrappers::IGlibWrapper> glibWrapper, double rate);
~SetPlaybackRate() override;
void execute() const override;

private:
GenericPlayerContext &m_context;
IGstGenericPlayerPrivate &m_player;
std::shared_ptr<firebolt::rialto::wrappers::IGstWrapper> m_gstWrapper;
std::shared_ptr<firebolt::rialto::wrappers::IGlibWrapper> m_glibWrapper;
double m_rate;
Expand Down
2 changes: 1 addition & 1 deletion media/server/gstplayer/source/GstGenericPlayer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ void GstGenericPlayer::setPlaybackRate(double rate)
{
if (m_workerThread)
{
m_workerThread->enqueueTask(m_taskFactory->createSetPlaybackRate(m_context, rate));
m_workerThread->enqueueTask(m_taskFactory->createSetPlaybackRate(m_context, *this, rate));
}
}

Expand Down
3 changes: 2 additions & 1 deletion media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,10 @@ std::unique_ptr<IPlayerTask> GenericPlayerTaskFactory::createCheckAudioUnderflow
}

std::unique_ptr<IPlayerTask> GenericPlayerTaskFactory::createSetPlaybackRate(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player,
double rate) const
{
return std::make_unique<tasks::generic::SetPlaybackRate>(context, m_gstWrapper, m_glibWrapper, rate);
return std::make_unique<tasks::generic::SetPlaybackRate>(context, player, m_gstWrapper, m_glibWrapper, rate);
}

std::unique_ptr<IPlayerTask> GenericPlayerTaskFactory::createSetPosition(GenericPlayerContext &context,
Expand Down
39 changes: 28 additions & 11 deletions media/server/gstplayer/source/tasks/generic/SetPlaybackRate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,16 @@
namespace
{
const char kCustomInstantRateChangeEventName[] = "custom-instant-rate-change";
constexpr double DEFAULT_INITIAL_RATE_CORRECTION_SPEED = 1.000001;
} // namespace

namespace firebolt::rialto::server::tasks::generic
{
SetPlaybackRate::SetPlaybackRate(GenericPlayerContext &context,
IGstGenericPlayerPrivate &player,
std::shared_ptr<firebolt::rialto::wrappers::IGstWrapper> gstWrapper,
std::shared_ptr<firebolt::rialto::wrappers::IGlibWrapper> glibWrapper, double rate)
: m_context{context}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_rate{rate}
: m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_rate{rate}
Copy link
Copy Markdown
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Mar 13, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Variable copied when it could be moved

"glibWrapper" is passed-by-value as parameter to "std::shared_ptrfirebolt::rialto::wrappers::IGlibWrapper::shared_ptr(std::shared_ptrfirebolt::rialto::wrappers::IGlibWrapper const &) /explicit =default/", when it could be moved instead.

Low Impact, CWE-none
COPY_INSTEAD_OF_MOVE

How to fix

Use "std::move(""glibWrapper"")" instead of "glibWrapper".

Copy link
Copy Markdown
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Mar 13, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Variable copied when it could be moved

"gstWrapper" is passed-by-value as parameter to "std::shared_ptrfirebolt::rialto::wrappers::IGstWrapper::shared_ptr(std::shared_ptrfirebolt::rialto::wrappers::IGstWrapper const &) /explicit =default/", when it could be moved instead.

Low Impact, CWE-none
COPY_INSTEAD_OF_MOVE

How to fix

Use "std::move(""gstWrapper"")" instead of "gstWrapper".

{
RIALTO_SERVER_LOG_DEBUG("Constructing SetPlaybackRate");
}
Expand All @@ -46,23 +48,38 @@ SetPlaybackRate::~SetPlaybackRate()
void SetPlaybackRate::execute() const
{
RIALTO_SERVER_LOG_DEBUG("Executing SetPlaybackRate");
if (m_context.playbackRate == m_rate)
double rate{m_rate};
if (rate == DEFAULT_INITIAL_RATE_CORRECTION_SPEED)
{
RIALTO_SERVER_LOG_DEBUG("No need to change playback rate - it is already %lf", m_rate);
GstElement *decoder{m_player.getDecoder(firebolt::rialto::MediaSourceType::AUDIO)};
if (!decoder || !m_glibWrapper->gStrHasPrefix(GST_ELEMENT_NAME(decoder), "brcm"))
{
rate = 1.0f;
}

if (decoder)
{
m_gstWrapper->gstObjectUnref(decoder);
Comment on lines +54 to +62
}
}
Comment on lines +51 to +64

if (m_context.playbackRate == rate)
{
RIALTO_SERVER_LOG_DEBUG("No need to change playback rate - it is already %lf", rate);
return;
}

if (!m_context.pipeline)
{
RIALTO_SERVER_LOG_INFO("Postponing set playback rate to %lf. Pipeline is NULL", m_rate);
m_context.pendingPlaybackRate = m_rate;
RIALTO_SERVER_LOG_INFO("Postponing set playback rate to %lf. Pipeline is NULL", rate);
m_context.pendingPlaybackRate = rate;
return;
}

if (GST_STATE(m_context.pipeline) < GST_STATE_PLAYING)
{
RIALTO_SERVER_LOG_INFO("Postponing set playback rate to %lf. Pipeline state is below PLAYING", m_rate);
m_context.pendingPlaybackRate = m_rate;
RIALTO_SERVER_LOG_INFO("Postponing set playback rate to %lf. Pipeline state is below PLAYING", rate);
m_context.pendingPlaybackRate = rate;
return;
}
m_context.pendingPlaybackRate = kNoPendingPlaybackRate;
Expand All @@ -74,7 +91,7 @@ void SetPlaybackRate::execute() const
{
GstSegment *segment{m_gstWrapper->gstSegmentNew()};
m_gstWrapper->gstSegmentInit(segment, GST_FORMAT_TIME);
segment->rate = m_rate;
segment->rate = rate;
segment->start = GST_CLOCK_TIME_NONE;
segment->position = GST_CLOCK_TIME_NONE;
success = m_gstWrapper->gstPadSendEvent(GST_BASE_SINK_PAD(audioSink), m_gstWrapper->gstEventNewSegment(segment));
Expand All @@ -84,7 +101,7 @@ void SetPlaybackRate::execute() const
else
{
GstStructure *structure{
m_gstWrapper->gstStructureNew(kCustomInstantRateChangeEventName, "rate", G_TYPE_DOUBLE, m_rate, NULL)};
m_gstWrapper->gstStructureNew(kCustomInstantRateChangeEventName, "rate", G_TYPE_DOUBLE, rate, NULL)};
success = m_gstWrapper->gstElementSendEvent(m_context.pipeline,
m_gstWrapper->gstEventNewCustom(GST_EVENT_CUSTOM_DOWNSTREAM_OOB,
structure));
Expand All @@ -93,8 +110,8 @@ void SetPlaybackRate::execute() const

if (success)
{
RIALTO_SERVER_LOG_MIL("Playback rate set to: %lf", m_rate);
m_context.playbackRate = m_rate;
RIALTO_SERVER_LOG_MIL("Playback rate set to: %lf", rate);
m_context.playbackRate = rate;
}

if (audioSink)
Expand Down
3 changes: 2 additions & 1 deletion tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ TEST_F(GstGenericPlayerTest, shouldSetPlaybackRate)
double playbackRate{1.5};
std::unique_ptr<IPlayerTask> task{std::make_unique<StrictMock<PlayerTaskMock>>()};
EXPECT_CALL(dynamic_cast<StrictMock<PlayerTaskMock> &>(*task), execute());
EXPECT_CALL(m_taskFactoryMock, createSetPlaybackRate(_, playbackRate)).WillOnce(Return(ByMove(std::move(task))));
EXPECT_CALL(m_taskFactoryMock, createSetPlaybackRate(_, _, playbackRate))
.WillOnce(Return(ByMove(std::move(task))));

m_sut->setPlaybackRate(playbackRate);
}
Expand Down
3 changes: 2 additions & 1 deletion tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2822,7 +2822,8 @@ void GenericTasksTestsBase::shouldNotifyNeedVideoDataFailure()

void GenericTasksTestsBase::triggerSetPlaybackRate()
{
firebolt::rialto::server::tasks::generic::SetPlaybackRate task{testContext->m_context, testContext->m_gstWrapper,
firebolt::rialto::server::tasks::generic::SetPlaybackRate task{testContext->m_context, testContext->m_gstPlayer,
testContext->m_gstWrapper,
testContext->m_glibWrapper, kRate};
task.execute();
}
Expand Down
2 changes: 1 addition & 1 deletion ...nittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,7 @@ TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateUnderflow)

TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateSetPlaybackRate)
{
auto task = m_sut.createSetPlaybackRate(m_context, 1.25);
auto task = m_sut.createSetPlaybackRate(m_context, m_gstPlayer, 1.25);
EXPECT_NE(task, nullptr);
EXPECT_NO_THROW(dynamic_cast<firebolt::rialto::server::tasks::generic::SetPlaybackRate &>(*task));
}
Expand Down
4 changes: 2 additions & 2 deletions tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,8 @@ class GenericPlayerTaskFactoryMock : public IGenericPlayerTaskFactory
(GenericPlayerContext & context, IGstGenericPlayerPrivate &player), (const, override));
MOCK_METHOD(std::unique_ptr<IPlayerTask>, createCheckAudioUnderflow,
(GenericPlayerContext & context, IGstGenericPlayerPrivate &player), (const, override));
MOCK_METHOD(std::unique_ptr<IPlayerTask>, createSetPlaybackRate, (GenericPlayerContext & context, double rate),
(const, override));
MOCK_METHOD(std::unique_ptr<IPlayerTask>, createSetPlaybackRate,
(GenericPlayerContext & context, IGstGenericPlayerPrivate &player, double rate), (const, override));
MOCK_METHOD(std::unique_ptr<IPlayerTask>, createSetPosition,
(GenericPlayerContext & context, IGstGenericPlayerPrivate &player, std::int64_t position),
(const, override));
Expand Down
1 change: 1 addition & 0 deletions tests/unittests/media/server/mocks/gstplayer/GstGenericPlayerPrivateMock.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ class GstGenericPlayerPrivateMock : public IGstGenericPlayerPrivate
MOCK_METHOD(void, removeAutoVideoSinkChild, (GObject * object), (override));
MOCK_METHOD(void, removeAutoAudioSinkChild, (GObject * object), (override));
MOCK_METHOD(GstElement *, getSink, (const MediaSourceType &mediaSourceType), (const, override));
MOCK_METHOD(GstElement *, getDecoder, (const MediaSourceType &mediaSourceType), (override));

MOCK_METHOD(void, addAudioClippingToBuffer, (GstBuffer * buffer, uint64_t clippingStart, uint64_t clippingEnd),
(const, override));
Expand Down
Loading