From c75068b49aadb6d1e099f3ddea07537ff4d3452f 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 11b9e9aa4..2781cbe40 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -351,6 +351,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 4eb337ead..01dcdb90a 100644 --- a/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp +++ b/media/server/gstplayer/source/tasks/generic/DeepElementAdded.cpp @@ -62,6 +62,7 @@ DeepElementAdded::~DeepElementAdded() { RIALTO_SERVER_LOG_DEBUG("DeepElementAdded finished"); m_glibWrapper->gFree(m_elementName); + m_gstWrapper->gstObjectUnref(m_element); } void DeepElementAdded::execute() const From f95259b9d42f2fc4521078afcf1e3aea155bc3b7 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 2781cbe40..95001e2c9 100644 --- a/media/server/gstplayer/source/GstGenericPlayer.cpp +++ b/media/server/gstplayer/source/GstGenericPlayer.cpp @@ -2647,7 +2647,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 b5807ac23..31d6dcd1f 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 54ef739e3ffe9c548593462f30d4ab39169324a1 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 b0e226139..c92e58ffe 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerPrivateTest.cpp @@ -1757,14 +1757,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 7971f6f7e..35748aeba 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/GstGenericPlayerTest.cpp @@ -282,6 +282,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 05c352150..23d1c82de 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/common/GenericTasksTestsBase.cpp @@ -1951,6 +1951,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, @@ -2033,6 +2035,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, @@ -2197,6 +2201,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, @@ -2219,6 +2225,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 6bd95d4a8..d2156db4f 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp @@ -116,6 +116,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)); @@ -312,6 +314,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 6216a55ae48ce80d4093daa0ba7e18936dc93e33 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 35b90cbad..2a7e9fe74 100644 --- a/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h +++ b/media/server/gstplayer/include/tasks/IGenericPlayerTaskFactory.h @@ -395,13 +395,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 76b419ead..51890e2e9 100644 --- a/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h +++ b/media/server/gstplayer/include/tasks/generic/GenericPlayerTaskFactory.h @@ -103,7 +103,7 @@ class GenericPlayerTaskFactory : public IGenericPlayerTaskFactory bool underflowEnable, 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 f1cb25000..457d3af50 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, std::shared_ptr gstWrapper, std::shared_ptr glibWrapper, GstElement *typefind, - const GstCaps *caps); + 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 4cf83e0df..9c4d02115 100644 --- a/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp +++ b/media/server/gstplayer/source/tasks/generic/GenericPlayerTaskFactory.cpp @@ -274,8 +274,7 @@ std::unique_ptr GenericPlayerTaskFactory::createUnderflow(GenericPl 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 31d6dcd1f..d3e24c73c 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, std::shared_ptr gstWrapper, 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 d2156db4f..4ce2209cd 100644 --- a/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp +++ b/tests/unittests/media/server/gstplayer/genericPlayer/tasksTests/GenericPlayerTaskFactoryTest.cpp @@ -112,13 +112,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)); } @@ -314,9 +315,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 65319af05..970d50a39 100644 --- a/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h +++ b/tests/unittests/media/server/mocks/gstplayer/GenericPlayerTaskFactoryMock.h @@ -110,8 +110,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));