Skip to content

RDKEMW-21173 [support/8.5.3.0] - Revert RDKEMW-16484 ticket due to regression issue #1702

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
rekhap2kandhavelan wants to merge 1 commit into
base: support/3.3.0_8.5
Choose a base branch
from
Open

RDKEMW-21173 [support/8.5.3.0] - Revert RDKEMW-16484 ticket due to regression issue #1702

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
1 change: 0 additions & 1 deletion middleware/InterfacePlayerPriv.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ struct GstPlayerPriv
gboolean buffering_in_progress; /**< buffering is in progress */
guint buffering_timeout_cnt; /**< make sure buffering_timeout doesn't get stuck */
GstState buffering_target_state; /**< the target state after buffering */
bool seekPausedState; /** < true when seek with keepPaused is active — guards buffering_timeout from setting PLAYING */
gint64 lastKnownPTS; /**< To store the PTS of last displayed video */
long long ptsUpdatedTimeMS; /**< Timestamp when PTS was last updated */
guint ptsCheckForEosOnUnderflowIdleTaskId; /**< ID of task to ensure video PTS is not moving before notifying EOS on underflow. */
Expand Down
140 changes: 12 additions & 128 deletions middleware/InterfacePlayerRDK.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ firstFrameCallbackIdleTaskId(GST_TASK_ID_INVALID), firstFrameCallbackIdleTaskPen
using_westerossink(false), usingRialtoSink(false), usingClosedCaptionsControl(false), pauseOnStartPlayback(false), eosSignalled(false),
buffering_enabled(FALSE), buffering_in_progress(FALSE), buffering_timeout_cnt(0),
buffering_target_state(GST_STATE_NULL),
seekPausedState(false), lastKnownPTS(0), ptsUpdatedTimeMS(0), ptsCheckForEosOnUnderflowIdleTaskId(GST_TASK_ID_INVALID),
lastKnownPTS(0), ptsUpdatedTimeMS(0), ptsCheckForEosOnUnderflowIdleTaskId(GST_TASK_ID_INVALID),
numberOfVideoBuffersSent(0), segmentStart(0), positionQuery(NULL), durationQuery(NULL),
paused(false), pipelineState(GST_STATE_NULL),
firstVideoFrameDisplayedCallbackTask("FirstVideoFrameDisplayedCallback"),
Expand Down Expand Up @@ -504,39 +504,17 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF
MW_LOG_ERR("InterfacePlayerRDK_Configure GST_STATE_PAUSED failed");
}
interfacePlayerPriv->gstPrivateContext->pendingPlayState = false;
if (!interfacePlayerPriv->gstPrivateContext->seekPausedState)
{
interfacePlayerPriv->gstPrivateContext->paused = false;
}
interfacePlayerPriv->gstPrivateContext->paused = false;
}
else
{
MW_LOG_INFO("Setting state to GST_STATE_PLAYING");
/* If a seek-with-keepPaused is active we must not race into PLAYING.
* Defer the PLAYING transition and leave pipeline in PAUSED until
* an explicit resume (Pause(false)) clears `seekPausedState`.
*/
if (interfacePlayerPriv->gstPrivateContext->seekPausedState)
{
MW_LOG_WARN("seekPausedState active - deferring transition to PLAYING, marking pendingPlayState");
interfacePlayerPriv->gstPrivateContext->buffering_target_state = GST_STATE_PLAYING;
interfacePlayerPriv->gstPrivateContext->pendingPlayState = true;
/* Ensure pipeline is left/returned to PAUSED to avoid accidental play */
if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE)
{
MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PAUSED failed while deferring PLAYING");
}
interfacePlayerPriv->gstPrivateContext->paused = true;
}
else
if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE)
{
if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE)
{
MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PLAYING failed");
}
interfacePlayerPriv->gstPrivateContext->pendingPlayState = false;
interfacePlayerPriv->gstPrivateContext->paused = false;
MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PLAYING failed");
}
interfacePlayerPriv->gstPrivateContext->pendingPlayState = false;
interfacePlayerPriv->gstPrivateContext->paused = false;
}
interfacePlayerPriv->gstPrivateContext->eosSignalled = false;
interfacePlayerPriv->gstPrivateContext->numberOfVideoBuffersSent = 0;
Expand Down Expand Up @@ -1616,32 +1594,14 @@ bool InterfacePlayerRDK::Flush(double position, int rate, bool shouldTearDown, b
interfacePlayerPriv->gstPrivateContext->ptsCheckForEosOnUnderflowIdleTaskId = PLAYER_TASK_ID_INVALID;

}

/* If pipeline is paused (seek with keepPaused), mark seekPausedState
* so that when ConfigurePipeline restarts buffering, the buffering_timeout callback
* won't race to set PLAYING before Pause(1) arrives */
if (interfacePlayerPriv->gstPrivateContext->paused)
{
interfacePlayerPriv->gstPrivateContext->seekPausedState = true;
MW_LOG_MIL("InterfacePlayerRDK: Flush with paused state — setting seekPausedState");
}
if (interfacePlayerPriv->gstPrivateContext->bufferingTimeoutTimerId)
{
MW_LOG_MIL("InterfacePlayerRDK: Remove bufferingTimeoutTimerId %d", interfacePlayerPriv->gstPrivateContext->bufferingTimeoutTimerId);
g_source_remove(interfacePlayerPriv->gstPrivateContext->bufferingTimeoutTimerId);
interfacePlayerPriv->gstPrivateContext->bufferingTimeoutTimerId = PLAYER_TASK_ID_INVALID;
// Reset buffering state to prevent stale timeout_cnt from triggering error after seek
interfacePlayerPriv->gstPrivateContext->buffering_in_progress = false;
interfacePlayerPriv->gstPrivateContext->buffering_timeout_cnt = DEFAULT_BUFFERING_MAX_CNT;


}
// If pipeline is not paused (normal playback seek), clear seekPausedState
if (!interfacePlayerPriv->gstPrivateContext->paused)
{
interfacePlayerPriv->gstPrivateContext->seekPausedState = false;
MW_LOG_MIL("InterfacePlayerRDK: rate indicates playback, clearing seekPausedState");
}

// If the pipeline is not setup, we will cache the value for later
SetSeekPosition(position);

Expand Down Expand Up @@ -3389,12 +3349,6 @@ bool InterfacePlayerRDK::Pause(bool pause , bool forceStopGstreamerPreBuffering)
GstState nextState = pause ? GST_STATE_PAUSED : GST_STATE_PLAYING;
interfacePlayerPriv->gstPrivateContext->buffering_target_state = nextState;

/* Clear seekPausedState when explicitly resuming playback */
if (!pause)
{
interfacePlayerPriv->gstPrivateContext->seekPausedState = false;
}

if (GST_STATE_PAUSED == nextState && forceStopGstreamerPreBuffering)
{
/* maybe in a timing case during the playback start,
Expand All @@ -3404,41 +3358,17 @@ bool InterfacePlayerRDK::Pause(bool pause , bool forceStopGstreamerPreBuffering)
* and the resume play will be handled from StopBuffering once after getting enough buffer/frames.
*/
interfacePlayerPriv->gstPrivateContext->buffering_in_progress = false;
}
}

GstStateChangeReturn rc = SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, nextState);

if (GST_STATE_CHANGE_ASYNC == rc)
{
/* CID:330433 Waiting while holding lock. Sleep introduced in validateStateWithMsTimeout to prevent continuous polling when synchronizing pipeline state.
* Too risky to remove mutex lock. It may be replaced if approach is redesigned in future */
/* wait a bit longer for the state change to conclude */
if (nextState != validateStateWithMsTimeout(this, nextState, 100))
if (nextState != validateStateWithMsTimeout(this,nextState, 100))
{
GstState current, pending;
MW_LOG_INFO("InterfacePlayerRDK_Pause - validateStateWithMsTimeout - FAILED expected %s", gst_element_state_get_name(nextState));

/* Recovery: retry the state change once before reporting failure */
MW_LOG_INFO("InterfacePlayerRDK_Pause - retrying state change to GstState %d", nextState);

// Wait for any in-flight transition to settle
gst_element_get_state(interfacePlayerPriv->gstPrivateContext->pipeline, &current, &pending, 0);

// Single retry — no destructive NULL reset
GstStateChangeReturn rcRetry = SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, nextState);
if (GST_STATE_CHANGE_ASYNC == rcRetry)
{
if (nextState != validateStateWithMsTimeout(this, nextState, 100))
{
MW_LOG_ERR("Retry also failed — reporting error");
retValue = false;
}
}
else if (GST_STATE_CHANGE_SUCCESS != rcRetry)
{
MW_LOG_ERR("Retry failed immediately with rc %d — reporting error", rcRetry);
retValue = false;
}
MW_LOG_ERR("InterfacePlayerRDK_Pause - validateStateWithMsTimeout - FAILED GstState %d", nextState);
}
}
else if (GST_STATE_CHANGE_SUCCESS != rc)
Expand Down Expand Up @@ -4473,17 +4403,7 @@ static gboolean bus_message(GstBus * bus, GstMessage * msg, InterfacePlayerRDK *
if(eGST_MEDIAFORMAT_DASH != static_cast<GstMediaFormat>(pInterfacePlayerRDK->m_gstConfigParam->media))
{
SetStateWithWarnings(privatePlayer->gstPrivateContext->pipeline, GST_STATE_PAUSED);
/* Avoid forcing PLAYING if a seek-with-keepPaused is active */
if (!privatePlayer->gstPrivateContext->seekPausedState)
{
SetStateWithWarnings(privatePlayer->gstPrivateContext->pipeline, GST_STATE_PLAYING);
}
else
{
MW_LOG_WARN("GST_MESSAGE_CLOCK_LOST: seekPausedState active - skipping PLAYING");
privatePlayer->gstPrivateContext->pendingPlayState = true;
privatePlayer->gstPrivateContext->buffering_target_state = GST_STATE_PLAYING;
}
SetStateWithWarnings(privatePlayer->gstPrivateContext->pipeline, GST_STATE_PLAYING);
}
break;

Expand Down Expand Up @@ -4542,30 +4462,14 @@ bool InterfacePlayerRDK::SetPlayBackRate(double rate)
bool ret = false;
std::vector<GstElement*> sources;
MW_LOG_TRACE("InterfacePlayerRDK: gst_event_new_instant_rate_change: %f ...V6", rate);

/* Guard: do not process rate change when rate is 0 or pipeline is paused */
if (rate == 0.0)
{
MW_LOG_WARN("SetPlayBackRate: rate==0 — skipping rate change");
return false;
}

if (interfacePlayerPriv->gstPrivateContext->paused)
{
MW_LOG_WARN("SetPlayBackRate: pipeline is paused — skipping rate change");
return false;
}

for (int iTrack = 0; iTrack < GST_TRACK_COUNT; iTrack++)
{
if (iTrack != static_cast<int>(eGST_MEDIATYPE_SUBTITLE) && interfacePlayerPriv->gstPrivateContext->stream[iTrack].source != nullptr)
{
sources.push_back(interfacePlayerPriv->gstPrivateContext->stream[iTrack].source);
}
}

ret = interfacePlayerPriv->socInterface->SetPlaybackRate(sources, interfacePlayerPriv->gstPrivateContext->pipeline, rate, interfacePlayerPriv->gstPrivateContext->video_dec, interfacePlayerPriv->gstPrivateContext->audio_dec);

ret = interfacePlayerPriv->socInterface->SetPlaybackRate(sources, interfacePlayerPriv->gstPrivateContext->pipeline, rate, interfacePlayerPriv->gstPrivateContext->video_dec,interfacePlayerPriv->gstPrivateContext->audio_dec);
return ret;
}

Expand Down Expand Up @@ -4685,16 +4589,6 @@ static gboolean buffering_timeout (gpointer data)
}
else if (frames == -1 || frames >= pInterfacePlayerRDK->m_gstConfigParam->framesToQueue || privatePlayer->gstPrivateContext->buffering_timeout_cnt-- == 0)
{
/* Do not set PLAYING if a seek-with-keepPaused is in progress.
* The buffering_timeout timer may fire after ConfigurePipeline restarts buffering
* but BEFORE the Pause(1) from keepPaused logic arrives — causing a race. */
if (privatePlayer->gstPrivateContext->seekPausedState)
{
MW_LOG_WARN("buffering_timeout: skipping PLAYING — seekPausedState active");
privatePlayer->gstPrivateContext->buffering_in_progress = false;
return false; //stop the timer
}

uint32_t original_buffering_timeout_cnt = privatePlayer->gstPrivateContext->buffering_timeout_cnt;
MW_LOG_MIL("Set pipeline state to %s - buffering_timeout_cnt %u frames %i",
gst_element_state_get_name(privatePlayer->gstPrivateContext->buffering_target_state), original_buffering_timeout_cnt, frames);
Expand Down Expand Up @@ -5102,16 +4996,6 @@ void InterfacePlayerRDK::NotifyFragmentCachingComplete()
{
if(interfacePlayerPriv->gstPrivateContext->pendingPlayState)
{
/* If a seek-with-keepPaused is active, do not transition to PLAYING here.
* Leave pendingPlayState set so the explicit resume will perform the transition.
*/
if (interfacePlayerPriv->gstPrivateContext->seekPausedState)
{
MW_LOG_WARN("NotifyFragmentCachingComplete: seekPausedState active - deferring PLAYING");
interfacePlayerPriv->gstPrivateContext->buffering_target_state = GST_STATE_PLAYING;
return;
}

MW_LOG_MIL("InterfacePlayer: Setting pipeline to PLAYING state ");
interfacePlayerPriv->gstPrivateContext->buffering_target_state = GST_STATE_PLAYING;
if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE)
Expand Down
117 changes: 1 addition & 116 deletions middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,36 +29,7 @@
#include "MockGstUtils.h"
#include <gst/gstplugin.h>
#include <gst/gstpluginfeature.h>
#include "SocInterface.h"

/**
* Minimal SocInterface mock for GetVideoPTS tests.
* Only GetVideoPts is mocked; all other methods delegate to the base fake.
*/
class MockSocInterfaceForPts : public SocInterface
{
public:
MOCK_METHOD(long long, GetVideoPts,
(GstElement *video_sink, GstElement *video_dec, bool isWesteros),
(override));
/* Pure virtual stubs required to make this class instantiable. */
MOCK_METHOD(bool, SetPlaybackRate,
(const std::vector<GstElement*>& sources, GstElement *pipeline,
double rate, GstElement *video_dec, GstElement *audio_dec), (override));
MOCK_METHOD(bool, SetRateCorrection, (), (override));
MOCK_METHOD(bool, IsVideoSink, (const char* name), (override));
MOCK_METHOD(bool, IsAudioSinkOrAudioDecoder, (const char* name), (override));
MOCK_METHOD(bool, IsVideoDecoder, (const char* name), (override));
MOCK_METHOD(bool, ConfigureAudioSink,
(GstElement **audio_sink, GstObject *src, bool decStreamSync), (override));
MOCK_METHOD(bool, IsAudioOrVideoDecoder, (const char* name), (override));
MOCK_METHOD(void, GetCCDecoderHandle,
(gpointer *dec_handle, GstElement *video_dec), (override));
MOCK_METHOD(bool, IsVideoMaster, (GstElement *videoSink), (override));
MOCK_METHOD(void, SetAudioProperty,
(const char * &volume, const char * &mute, bool& isSinkBinVolume), (override));
MOCK_METHOD(void, SetPlaybackFlags, (gint &flags, bool isSub), (override));
};

using ::testing::NiceMock;
using ::testing::StrictMock;
Expand Down Expand Up @@ -1983,10 +1954,7 @@ TEST_F(InterfacePlayerTests, Pause_Success)
mPlayerContext->pipeline = &gst_element_pipeline;

EXPECT_CALL(*g_mockGStreamer, gst_element_get_state(_, NotNull(), NotNull(), _))
.WillRepeatedly(DoAll(
SetArgPointee<1>(GST_STATE_PAUSED),
SetArgPointee<2>(GST_STATE_VOID_PENDING),
Return(GST_STATE_CHANGE_SUCCESS)));
.WillRepeatedly(Return(GST_STATE_CHANGE_SUCCESS));

EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(_, GST_STATE_PAUSED))
.WillOnce(Return(GST_STATE_CHANGE_ASYNC));
Expand Down Expand Up @@ -3085,86 +3053,3 @@ TEST_F(InterfacePlayerTests, SetStreamCaps_EncryptedAudioCodecFormat)

delete g_mockGstUtils;
}

/**
* @brief Test that NotifyFragmentCachingComplete defers PLAYING while seekPausedState is active.
*
* This matches the middleware behavior required for seek-with-keepPaused flows,
* where fragment caching completion should not force playback transition until
* explicit resume is requested.
*/
TEST_F(InterfacePlayerTests, NotifyFragmentCachingComplete_DefersPlayingWhenSeekPaused)
{
// Arrange: Create a pending play state and protect it with seekPausedState
mPlayerContext->pendingPlayState = true;
mPlayerContext->seekPausedState = true;
mPlayerContext->buffering_target_state = GST_STATE_PAUSED;

// Act: Notify fragment caching complete
mInterfaceGstPlayer->NotifyFragmentCachingComplete();


// Assert: Pending state should remain until explicit resume, and target stays PLAYING
EXPECT_EQ(mPlayerContext->pendingPlayState, true);
EXPECT_EQ(mPlayerContext->seekPausedState, true);
EXPECT_EQ(mPlayerContext->buffering_target_state, GST_STATE_PLAYING);
}

/**
* @brief Test that SetPlayBackRate clears seekPausedState when resuming from a seek-paused state.
*
* When a non-zero rate arrives while seekPausedState is active and the pipeline is still paused,
* the middleware should force resume and clear the protection state.
*/
TEST_F(InterfacePlayerTests, SetPlayBackRate_ForceResumeClearsSeekPausedState)
{
// Arrange: Simulate a paused pipeline in seek-paused state
mPlayerContext->paused = true;
mPlayerContext->seekPausedState = true;
mPlayerContext->pendingPlayState = true;
mPlayerContext->pipeline = &gst_element_pipeline;

// Act: Request a rate change while paused
bool result = mInterfaceGstPlayer->SetPlayBackRate(1.0);

// Assert: SetPlayBackRate should return false and NOT clear seekPausedState
// (clearing seekPausedState is the responsibility of Pause(false))
EXPECT_FALSE(result);
EXPECT_EQ(mPlayerContext->seekPausedState, true);
EXPECT_EQ(mPlayerContext->pendingPlayState, true);
EXPECT_EQ(mPlayerContext->paused, true);
}

/**
* @brief Test that explicit resume via Pause(false) clears seekPausedState
* and transitions pipeline to PLAYING — this is the actual "force resume" path.
*/
TEST_F(InterfacePlayerTests, PauseFalse_ClearsSeekPausedStateAndResumes)
{
// Arrange: Simulate a paused pipeline in seek-paused state
mPlayerContext->paused = true;
mPlayerContext->seekPausedState = true;
mPlayerContext->pendingPlayState = true;
mPlayerContext->pipeline = &gst_element_pipeline;

// Mock: gst_element_set_state to PLAYING succeeds asynchronously
EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(&gst_element_pipeline, GST_STATE_PLAYING))
.WillOnce(Return(GST_STATE_CHANGE_ASYNC));

// Mock: validateStateWithMsTimeout succeeds — pipeline reaches PLAYING
EXPECT_CALL(*g_mockGStreamer, gst_element_get_state(&gst_element_pipeline, NotNull(), NotNull(), _))
.WillRepeatedly(DoAll(
SetArgPointee<1>(GST_STATE_PLAYING),
SetArgPointee<2>(GST_STATE_VOID_PENDING),
Return(GST_STATE_CHANGE_SUCCESS)));

// Act: Resume playback
bool result = mInterfaceGstPlayer->Pause(false, false);

// Assert: seekPausedState cleared, pipeline marked as playing
EXPECT_TRUE(result);
EXPECT_EQ(mPlayerContext->seekPausedState, false);
EXPECT_EQ(mPlayerContext->paused, false);
EXPECT_EQ(mPlayerContext->pendingPlayState, false);
EXPECT_EQ(mPlayerContext->buffering_target_state, GST_STATE_PLAYING);
}