Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
7 changes: 7 additions & 0 deletions InterfacePlayerPriv.h
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,13 @@ struct GstPlayerPriv
GstHandlerControl syncControl;
GstHandlerControl callbackControl;


/* Guards ConfigurePipeline()/CreatePipeline() against running on this shared pipeline
* object while a TearDownStream() from a prior tune is still releasing resources. */
std::mutex mTeardownMutex;
std::condition_variable mTeardownCV;
bool mTeardownInProgress = false;

bool filterAudioDemuxBuffers; /**< flag to filter audio demux buffers */
double seekPosition; /**< the position to seek the pipeline to in seconds */
GstPlayerPriv();
Expand Down
106 changes: 104 additions & 2 deletions InterfacePlayerRDK.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#include <string.h>
#include <assert.h>
#include <stdlib.h>
#include <thread>
#include "PlayerLogManager.h"
#include "GstUtils.h"
#include <sys/time.h>
Expand Down Expand Up @@ -336,6 +337,16 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF
bool bESChangeStatus, bool setReadyAfterPipelineCreation,
bool isSubEnable, int32_t trackId, gint rate, const char *pipelineName, int PipelinePriority, bool FirstFrameFlag, std::string manifestUrl, bool enableLiveLatency)
{
{
/* Don't let a new tune start reconfiguring this shared pipeline object while a prior
* tune's TearDownStream() (on another thread) is still confirming resource release. */
std::unique_lock<std::mutex> teardownLock(interfacePlayerPriv->gstPrivateContext->mTeardownMutex);
if (!interfacePlayerPriv->gstPrivateContext->mTeardownCV.wait_for(teardownLock, std::chrono::milliseconds(3000),
[this]{ return !interfacePlayerPriv->gstPrivateContext->mTeardownInProgress; }))
Comment on lines +343 to +345
{
MW_LOG_ERR("InterfacePlayerRDK::ConfigurePipeline: timed out waiting for prior TearDownStream to complete");
}
}
StreamCodecInfo streamInfo;
streamInfo.video.mCodecFormat = static_cast<GstStreamOutputFormat>(format);
streamInfo.audio.mCodecFormat = static_cast<GstStreamOutputFormat>(audioFormat);
Expand Down Expand Up @@ -1419,6 +1430,10 @@ static GstStateChangeReturn SetStateWithWarnings(GstElement *element, GstState t

void InterfacePlayerRDK::TearDownStream(int type)
{
{
std::lock_guard<std::mutex> teardownLock(interfacePlayerPriv->gstPrivateContext->mTeardownMutex);
interfacePlayerPriv->gstPrivateContext->mTeardownInProgress = true;
}
tearDownCb(true, type);
gst_media_stream* stream = &interfacePlayerPriv->gstPrivateContext->stream[type];
RemoveProbe(type);
Expand All @@ -1441,6 +1456,27 @@ void InterfacePlayerRDK::TearDownStream(int type)
{
MW_LOG_ERR("InterfacePlayerRDK::TearDownStream: Failed to set NULL state for sinkbin");
}
else
{
/* A SUCCESS/ASYNC return here only reflects local GStreamer bookkeeping; for
* server-backed sinks (e.g. Rialto) the decoder/session release happens over IPC
* and is not guaranteed to be complete when this call returns. Always confirm
* NULL is actually reached (bounded) before a new decoder tries to acquire the
* same resource, instead of only doing so when the return is ASYNC. */
GstState current, pending;
gint retryCnt = GST_ELEMENT_GET_STATE_RETRY_CNT_MAX;
do
{
(void)gst_element_get_state(GST_ELEMENT(stream->sinkbin), &current, &pending, 100 * GST_MSECOND);
}
while ((current != GST_STATE_NULL) && (--retryCnt > 0));
if (current != GST_STATE_NULL)
{
MW_LOG_ERR("InterfacePlayerRDK::TearDownStream: sinkbin stopped abruptly, did not reach NULL (current=%s) - forcing synchronous reset before next decoder configuration", gst_element_state_get_name(current));
gst_element_set_state(GST_ELEMENT(stream->sinkbin), GST_STATE_NULL);
}
}

if (!gst_bin_remove(GST_BIN(interfacePlayerPriv->gstPrivateContext->pipeline), GST_ELEMENT(stream->sinkbin))) /* Removes the sinkbin element from the pipeline */
{
MW_LOG_ERR("InterfacePlayerRDK::TearDownStream: Unable to remove sinkbin from pipeline");
Expand Down Expand Up @@ -1485,7 +1521,37 @@ void InterfacePlayerRDK::TearDownStream(int type)
pthread_mutex_lock(&stream->sourceLock);
if (stream->sinkbin)
{
MW_LOG_WARN("InterfacePlayerRDK::TearDownStream: CC sinkbin still assigned, clearing");
/* This sinkbin was skipped by the main teardown block above (format was already
* INVALID, or usingClosedCaptionsControl was already cleared by another media
* type's teardown in this same call sequence) - clearing the reference without
* first requesting GST_STATE_NULL never runs the Rialto sink's teardown handler,
* so the underlying RialtoServer session for it is leaked. Tear it down properly
* before dropping the reference. */
MW_LOG_WARN("InterfacePlayerRDK::TearDownStream: CC sinkbin still assigned, tearing down before clearing");

if (interfacePlayerPriv->gstPrivateContext->pipeline)
{
GstStateChangeReturn nullRc = SetStateWithWarnings(GST_ELEMENT(stream->sinkbin), GST_STATE_NULL);
if (GST_STATE_CHANGE_FAILURE != nullRc)
{
GstState current, pending;
gint retryCnt = GST_ELEMENT_GET_STATE_RETRY_CNT_MAX;
do
{
gst_element_get_state(GST_ELEMENT(stream->sinkbin), &current, &pending, 100 * GST_MSECOND);
}
while ((current != GST_STATE_NULL) && (--retryCnt > 0));
if (current != GST_STATE_NULL)
{
MW_LOG_ERR("InterfacePlayerRDK::TearDownStream: CC sinkbin did not reach NULL (current=%s), forcing", gst_element_state_get_name(current));
gst_element_set_state(GST_ELEMENT(stream->sinkbin), GST_STATE_NULL);
}
}
if (!gst_bin_remove(GST_BIN(interfacePlayerPriv->gstPrivateContext->pipeline), GST_ELEMENT(stream->sinkbin)))
{
MW_LOG_INFO("InterfacePlayerRDK::TearDownStream: Unable to remove CC sinkbin from pipeline");
}
}
g_clear_object(&stream->sinkbin);
}
if (stream->source)
Expand All @@ -1496,6 +1562,12 @@ void InterfacePlayerRDK::TearDownStream(int type)
pthread_mutex_unlock(&stream->sourceLock);
}
tearDownCb(false, mediaType);
{
std::lock_guard<std::mutex> teardownLock(interfacePlayerPriv->gstPrivateContext->mTeardownMutex);
interfacePlayerPriv->gstPrivateContext->mTeardownInProgress = false;
Comment thread
rekhap2kandhavelan marked this conversation as resolved.
}
interfacePlayerPriv->gstPrivateContext->mTeardownCV.notify_all();

MW_LOG_MIL("InterfacePlayerRDK::TearDownStream: exit mediaType = %d", mediaType);
}

Expand Down Expand Up @@ -2296,7 +2368,37 @@ void InterfacePlayerRDK::SetupClosedCaptionControlStream()
// Check elements are not already assigned
if (stream->sinkbin)
{
MW_LOG_ERR("Sinkbin already assigned");
/* A stale sinkbin here means a previous TearDownStream()/teardown path left this
* element attached to the pipeline without requesting GST_STATE_NULL first - clearing
* the reference alone would leak the underlying RialtoServer session and leave a
* dangling child element in the pipeline bin. Tear it down properly before clearing. */
MW_LOG_WARN("Sinkbin already assigned, tearing down before clearing");
if (privatePlayer->gstPrivateContext->pipeline)
{
GstStateChangeReturn nullRc = SetStateWithWarnings(GST_ELEMENT(stream->sinkbin), GST_STATE_NULL);

if (GST_STATE_CHANGE_FAILURE != nullRc)
Comment thread
rekhap2kandhavelan marked this conversation as resolved.
{
GstState current, pending;
gint retryCnt = GST_ELEMENT_GET_STATE_RETRY_CNT_MAX;
do
{
gst_element_get_state(GST_ELEMENT(stream->sinkbin), &current, &pending, 100 * GST_MSECOND);
}
while ((current != GST_STATE_NULL) && (--retryCnt > 0));

if (current != GST_STATE_NULL)
{
MW_LOG_ERR("Sinkbin did not reach NULL (current=%s), forcing", gst_element_state_get_name(current));
gst_element_set_state(GST_ELEMENT(stream->sinkbin), GST_STATE_NULL);
}
}
if (!gst_bin_remove(GST_BIN(privatePlayer->gstPrivateContext->pipeline), GST_ELEMENT(stream->sinkbin)))
{
MW_LOG_INFO("Unable to remove stale sinkbin from pipeline");
}
}

g_clear_object(&stream->sinkbin);
}
if (privatePlayer->gstPrivateContext->subtitle_sink)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,9 +370,17 @@ TEST_F(InterfacePlayerTests, TearDownStreamTest_successvideo)


EXPECT_CALL(*g_mockGStreamer, gst_element_get_state(&gst_sinkbin, _,_,0))
.WillOnce(Return(GST_STATE_CHANGE_SUCCESS));
.WillOnce(DoAll(
SetArgPointee<1>(GST_STATE_PLAYING),
SetArgPointee<2>(GST_STATE_NULL),
Return(GST_STATE_CHANGE_SUCCESS)));
EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_sinkbin, GST_STATE_NULL))
.WillOnce(Return(GST_STATE_CHANGE_SUCCESS));
EXPECT_CALL(*g_mockGStreamer, gst_element_get_state(&gst_sinkbin, _, _, 100 * GST_MSECOND))
.WillOnce(DoAll(
SetArgPointee<1>(GST_STATE_NULL),
SetArgPointee<2>(GST_STATE_VOID_PENDING),
Return(GST_STATE_CHANGE_SUCCESS)));


mInterfaceGstPlayer->TearDownStream(eGST_MEDIATYPE_VIDEO);
Expand Down
Loading