From 26cf49ad4c6bb39985280887383192b8c72aac5f Mon Sep 17 00:00:00 2001 From: Sasa Mudri Date: Wed, 15 Jul 2026 16:24:41 +0200 Subject: [PATCH 1/4] Added ref/unref for GST element for DeepElementAdded task. --- media/server/gstplayer/source/GstGenericPlayer.cpp | 1 + media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/media/server/gstplayer/source/GstGenericPlayer.cpp b/media/server/gstplayer/source/GstGenericPlayer.cpp index fc1dc1811..7e1306166 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -396,6 +396,7 @@ void GstGenericPlayer::deepElementAdded(GstBin *pipeline, GstBin *bin, GstElemen RIALTO_SERVER_LOG_DEBUG("Deep element %s added to the pipeline", GST_ELEMENT_NAME(element)); if (self->m_workerThread) { + self->m_gstWrapper->gstObjectRef(element); self->m_workerThread->enqueueTask( self->m_taskFactory->createDeepElementAdded(self->m_context, *self, pipeline, bin, element)); } diff --git a/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp b/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp index 59d9cf21b..df24966c6 100644 --- a/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp +++ b/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp @@ -64,6 +64,7 @@ DeepElementAdded::~DeepElementAdded() { RIALTO_SERVER_LOG_DEBUG("DeepElementAdded finished"); m_glibWrapper->gFree(m_elementName); + m_gstWrapper->gstObjectUnref(m_element); } void DeepElementAdded::execute() const From 9817d97b7d83923c5ab5c152c4675c93d53fb75f Mon Sep 17 00:00:00 2001 From: Sasa Mudri Date: Thu, 16 Jul 2026 10:09:22 +0200 Subject: [PATCH 2/4] Added ref/unref for GST element for UpdatePlaybackGroup task. --- media/server/gstplayer/source/GstGenericPlayer.cpp | 7 ++++++- .../gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp | 5 +++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/media/server/gstplayer/source/GstGenericPlayer.cpp b/media/server/gstplayer/source/GstGenericPlayer.cpp index 7e1306166..3a5b3f693 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -2761,7 +2761,12 @@ void GstGenericPlayer::handleBusMessage(GstMessage *message) void GstGenericPlayer::updatePlaybackGroup(GstElement *typefind, const GstCaps *caps) { - m_workerThread->enqueueTask(m_taskFactory->createUpdatePlaybackGroup(m_context, *this, typefind, caps)); + if (m_workerThread) + { + m_gstWrapper->gstObjectRef(typefind); + GstCaps *ownedCaps{caps ? m_gstWrapper->gstCapsCopy(caps) : nullptr}; + m_workerThread->enqueueTask(m_taskFactory->createUpdatePlaybackGroup(m_context, *this, typefind, ownedCaps)); + } } void GstGenericPlayer::addAutoVideoSinkChild(GObject *object) diff --git a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp index 489ccf047..1021a851b 100644 --- a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp +++ b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp @@ -35,6 +35,11 @@ UpdatePlaybackGroup::UpdatePlaybackGroup(GenericPlayerContext &context, IGstGene UpdatePlaybackGroup::~UpdatePlaybackGroup() { RIALTO_SERVER_LOG_DEBUG("UpdatePlaybackGroup finished"); + m_gstWrapper->gstObjectUnref(m_typefind); + if (m_caps) + { + m_gstWrapper->gstCapsUnref(const_cast(m_caps)); + } } void UpdatePlaybackGroup::execute() const From 8a757cc9c10615393565011f109a3a07cde79cd0 Mon Sep 17 00:00:00 2001 From: Sasa Mudri Date: Thu, 16 Jul 2026 12:25:02 +0200 Subject: [PATCH 3/4] Added UTs following given changes. --- .../GstGenericPlayerPrivateTest.cpp | 17 ++++++++++++++++- .../genericPlayer/GstGenericPlayerTest.cpp | 1 + .../common/GenericTasksTestsBase.cpp | 10 ++++++++++ .../tasksTests/GenericPlayerTaskFactoryTest.cpp | 4 ++++ 4 files changed, 31 insertions(+), 1 deletion(-) diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp index 511fe71c7..ee31236bc 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp @@ -1846,14 +1846,29 @@ TEST_F(GstGenericPlayerPrivateTest, shouldUpdatePlaybackGroup) { GstElement typefind; GstCaps caps; + GstCaps copiedCaps; std::unique_ptr task{std::make_unique>()}; EXPECT_CALL(dynamic_cast &>(*task), execute()); - EXPECT_CALL(m_taskFactoryMock, createUpdatePlaybackGroup(_, _, &typefind, &caps)) + EXPECT_CALL(*m_gstWrapperMock, gstObjectRef(&typefind)); + EXPECT_CALL(*m_gstWrapperMock, gstCapsCopy(&caps)).WillOnce(Return(&copiedCaps)); + EXPECT_CALL(m_taskFactoryMock, createUpdatePlaybackGroup(_, _, &typefind, &copiedCaps)) .WillOnce(Return(ByMove(std::move(task)))); m_sut->updatePlaybackGroup(&typefind, &caps); } +TEST_F(GstGenericPlayerPrivateTest, shouldUpdatePlaybackGroupWithNullCaps) +{ + GstElement typefind; + std::unique_ptr task{std::make_unique>()}; + EXPECT_CALL(dynamic_cast &>(*task), execute()); + EXPECT_CALL(*m_gstWrapperMock, gstObjectRef(&typefind)); + EXPECT_CALL(m_taskFactoryMock, createUpdatePlaybackGroup(_, _, &typefind, nullptr)) + .WillOnce(Return(ByMove(std::move(task)))); + + m_sut->updatePlaybackGroup(&typefind, nullptr); +} + TEST_F(GstGenericPlayerPrivateTest, shouldAddAutoVideoSinkChildSink) { const GenericPlayerContext *context = getPlayerContext(); diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp index 6096a5b3d..a38eda7e9 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp @@ -283,6 +283,7 @@ TEST_F(GstGenericPlayerTest, shouldAddDeepElement) GstElement element{}; std::unique_ptr task{std::make_unique>()}; EXPECT_CALL(dynamic_cast &>(*task), execute()); + EXPECT_CALL(*m_gstWrapperMock, gstObjectRef(&element)); EXPECT_CALL(m_taskFactoryMock, createDeepElementAdded(_, _, _, _, &element)).WillOnce(Return(ByMove(std::move(task)))); triggerDeepElementAdded(&element); diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp index 83c9685ae..eba5222f2 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp @@ -2081,6 +2081,8 @@ void GenericTasksTestsBase::shouldNotRegisterCallbackWhenPtrsAreNotEqual() void GenericTasksTestsBase::constructDeepElementAdded() { + // The element reference taken in GstGenericPlayer::deepElementAdded is released in the task destructor. + EXPECT_CALL(*testContext->m_gstWrapper, gstObjectUnref(testContext->m_element)); firebolt::rialto::server::tasks::generic::DeepElementAdded task{testContext->m_context, testContext->m_gstPlayer, testContext->m_gstWrapper, @@ -2163,6 +2165,8 @@ void GenericTasksTestsBase::shouldSetTypefindElement() void GenericTasksTestsBase::triggerDeepElementAdded() { + // The element reference taken in GstGenericPlayer::deepElementAdded is released in the task destructor. + EXPECT_CALL(*testContext->m_gstWrapper, gstObjectUnref(testContext->m_element)); firebolt::rialto::server::tasks::generic::DeepElementAdded task{testContext->m_context, testContext->m_gstPlayer, testContext->m_gstWrapper, @@ -2327,6 +2331,8 @@ void GenericTasksTestsBase::checkAudioSinkPlaybackGroupAdded() void GenericTasksTestsBase::triggerUpdatePlaybackGroupNoCaps() { + // The typefind reference taken in GstGenericPlayer::updatePlaybackGroup is released in the task destructor. + EXPECT_CALL(*testContext->m_gstWrapper, gstObjectUnref(testContext->m_element)); firebolt::rialto::server::tasks::generic::UpdatePlaybackGroup task{testContext->m_context, testContext->m_gstPlayer, testContext->m_gstWrapper, @@ -2349,6 +2355,10 @@ void GenericTasksTestsBase::shouldReturnNullCaps() void GenericTasksTestsBase::triggerUpdatePlaybackGroup() { + // The typefind and caps references (owned copy) taken in GstGenericPlayer::updatePlaybackGroup are released in the + // task destructor. + EXPECT_CALL(*testContext->m_gstWrapper, gstObjectUnref(testContext->m_element)); + EXPECT_CALL(*testContext->m_gstWrapper, gstCapsUnref(&testContext->m_gstCaps1)); firebolt::rialto::server::tasks::generic::UpdatePlaybackGroup task{testContext->m_context, testContext->m_gstPlayer, testContext->m_gstWrapper, diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp index d1d1b67ec..087b53c1b 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp @@ -117,6 +117,8 @@ TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateDeepElementAdded) EXPECT_CALL(*m_gstWrapper, gstObjectCast(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*m_gstWrapper, gstElementGetName(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*m_glibWrapper, gFree(nullptr)); + // The task destructor releases the element reference taken when the task is scheduled. + EXPECT_CALL(*m_gstWrapper, gstObjectUnref(nullptr)); auto task = m_sut.createDeepElementAdded(m_context, m_gstPlayer, nullptr, nullptr, nullptr); EXPECT_NE(task, nullptr); EXPECT_NO_THROW(dynamic_cast(*task)); @@ -320,6 +322,8 @@ TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateSetPlaybackRate) TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateUpdatePlaybackGroup) { + // The task destructor releases the typefind reference taken when the task is scheduled. + EXPECT_CALL(*m_gstWrapper, gstObjectUnref(nullptr)); auto task = m_sut.createUpdatePlaybackGroup(m_context, m_gstPlayer, nullptr, nullptr); EXPECT_NE(task, nullptr); EXPECT_NO_THROW(dynamic_cast(*task)); From 51db707721dd92d6a07a970cbe7dd230ffc879ec Mon Sep 17 00:00:00 2001 From: Sasa Mudri Date: Thu, 16 Jul 2026 17:06:10 +0200 Subject: [PATCH 4/4] Added changes suggested by copilot. --- .../include/tasks/IGenericPlayerTaskFactory.h | 4 ++-- .../include/tasks/generic/GenericPlayerTaskFactory.h | 2 +- .../include/tasks/generic/UpdatePlaybackGroup.h | 4 ++-- .../source/tasks/generic/GenericPlayerTaskFactory.cpp | 3 +-- .../source/tasks/generic/UpdatePlaybackGroup.cpp | 4 ++-- .../tasksTests/GenericPlayerTaskFactoryTest.cpp | 10 ++++++---- .../mocks/gstplayer/GenericPlayerTaskFactoryMock.h | 3 +-- 7 files changed, 15 insertions(+), 15 deletions(-) diff --git a/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h b/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h index 227b0486f..259e9065f 100644 --- a/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h +++ b/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h @@ -408,13 +408,13 @@ class IGenericPlayerTaskFactory * @param[in] context : The GstGenericPlayer context * @param[in] player : The GstGenericPlayer instance * @param[in] typefind : The typefind element. - * @param[in] caps : The GstCaps of added element + * @param[in] caps : The GstCaps of added element. * * @retval the new UpdatePlaybackGroup task instance. */ virtual std::unique_ptr createUpdatePlaybackGroup(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, - GstElement *typefind, const GstCaps *caps) const = 0; + GstElement *typefind, GstCaps *caps) const = 0; /** * @brief Creates a RenderFrame task. diff --git a/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h b/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h index 14e05b9ab..5c7ad4c3c 100644 --- a/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h +++ b/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h @@ -105,7 +105,7 @@ class GenericPlayerTaskFactory : public IGenericPlayerTaskFactory MediaSourceType sourceType) const override; std::unique_ptr createUpdatePlaybackGroup(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, GstElement *typefind, - const GstCaps *caps) const override; + GstCaps *caps) const override; std::unique_ptr createRenderFrame(GenericPlayerContext &context, IGstGenericPlayerPrivate &player) const override; std::unique_ptr createPing(std::unique_ptr &&heartbeatHandler) const override; diff --git a/media/server/gstplayer/include/tasks/generic/UpdatePlaybackGroup.h b/media/server/gstplayer/include/tasks/generic/UpdatePlaybackGroup.h index 7b4a9c557..febdae2fd 100644 --- a/media/server/gstplayer/include/tasks/generic/UpdatePlaybackGroup.h +++ b/media/server/gstplayer/include/tasks/generic/UpdatePlaybackGroup.h @@ -36,7 +36,7 @@ class UpdatePlaybackGroup : public IPlayerTask UpdatePlaybackGroup(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, const std::shared_ptr &gstWrapper, const std::shared_ptr &glibWrapper, - GstElement *typefind, const GstCaps *caps); + GstElement *typefind, GstCaps *caps); ~UpdatePlaybackGroup() override; void execute() const override; @@ -46,7 +46,7 @@ class UpdatePlaybackGroup : public IPlayerTask std::shared_ptr m_gstWrapper; std::shared_ptr m_glibWrapper; GstElement *m_typefind; - const GstCaps *m_caps; + GstCaps *m_caps; }; } // namespace firebolt::rialto::server::tasks::generic diff --git a/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp b/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp index be2f3d834..af170e228 100644 --- a/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp +++ b/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp @@ -282,8 +282,7 @@ std::unique_ptr GenericPlayerTaskFactory::createFirstFrameReceived( std::unique_ptr GenericPlayerTaskFactory::createUpdatePlaybackGroup(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, - GstElement *typefind, - const GstCaps *caps) const + GstElement *typefind, GstCaps *caps) const { return std::make_unique(context, player, m_gstWrapper, m_glibWrapper, typefind, caps); diff --git a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp index 1021a851b..b944545da 100644 --- a/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp +++ b/media/server/gstplayer/source/tasks/generic/UpdatePlaybackGroup.cpp @@ -25,7 +25,7 @@ namespace firebolt::rialto::server::tasks::generic UpdatePlaybackGroup::UpdatePlaybackGroup(GenericPlayerContext &context, IGstGenericPlayerPrivate &player, const std::shared_ptr &gstWrapper, const std::shared_ptr &glibWrapper, - GstElement *typefind, const GstCaps *caps) + GstElement *typefind, GstCaps *caps) : m_context{context}, m_player{player}, m_gstWrapper{gstWrapper}, m_glibWrapper{glibWrapper}, m_typefind{typefind}, m_caps{caps} { @@ -38,7 +38,7 @@ UpdatePlaybackGroup::~UpdatePlaybackGroup() m_gstWrapper->gstObjectUnref(m_typefind); if (m_caps) { - m_gstWrapper->gstCapsUnref(const_cast(m_caps)); + m_gstWrapper->gstCapsUnref(m_caps); } } diff --git a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp index 087b53c1b..c03d3d68e 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp @@ -113,13 +113,14 @@ TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateAttachSource) TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateDeepElementAdded) { + GstElement element{}; EXPECT_CALL(*m_gstWrapper, gstObjectParent(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*m_gstWrapper, gstObjectCast(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*m_gstWrapper, gstElementGetName(_)).WillOnce(Return(nullptr)); EXPECT_CALL(*m_glibWrapper, gFree(nullptr)); // The task destructor releases the element reference taken when the task is scheduled. - EXPECT_CALL(*m_gstWrapper, gstObjectUnref(nullptr)); - auto task = m_sut.createDeepElementAdded(m_context, m_gstPlayer, nullptr, nullptr, nullptr); + EXPECT_CALL(*m_gstWrapper, gstObjectUnref(&element)); + auto task = m_sut.createDeepElementAdded(m_context, m_gstPlayer, nullptr, nullptr, &element); EXPECT_NE(task, nullptr); EXPECT_NO_THROW(dynamic_cast(*task)); } @@ -322,9 +323,10 @@ TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateSetPlaybackRate) TEST_F(GenericPlayerTaskFactoryTest, ShouldCreateUpdatePlaybackGroup) { + GstElement typefind{}; // The task destructor releases the typefind reference taken when the task is scheduled. - EXPECT_CALL(*m_gstWrapper, gstObjectUnref(nullptr)); - auto task = m_sut.createUpdatePlaybackGroup(m_context, m_gstPlayer, nullptr, nullptr); + EXPECT_CALL(*m_gstWrapper, gstObjectUnref(&typefind)); + auto task = m_sut.createUpdatePlaybackGroup(m_context, m_gstPlayer, &typefind, nullptr); EXPECT_NE(task, nullptr); EXPECT_NO_THROW(dynamic_cast(*task)); } diff --git a/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h b/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h index 77fac21c3..5f056ed70 100644 --- a/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h +++ b/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h @@ -113,8 +113,7 @@ class GenericPlayerTaskFactoryMock : public IGenericPlayerTaskFactory MediaSourceType sourceType), (const, override)); MOCK_METHOD(std::unique_ptr, createUpdatePlaybackGroup, - (GenericPlayerContext & context, IGstGenericPlayerPrivate &player, GstElement *typefind, - const GstCaps *caps), + (GenericPlayerContext & context, IGstGenericPlayerPrivate &player, GstElement *typefind, GstCaps *caps), (const, override)); MOCK_METHOD(std::unique_ptr, createRenderFrame, (GenericPlayerContext & context, IGstGenericPlayerPrivate &player), (const, override));