diff --git a/InterfacePlayerPriv.h b/InterfacePlayerPriv.h index 296c5431..97798d78 100755 --- a/InterfacePlayerPriv.h +++ b/InterfacePlayerPriv.h @@ -206,6 +206,7 @@ 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. */ diff --git a/InterfacePlayerRDK.cpp b/InterfacePlayerRDK.cpp index f648fa50..b02c8d7a 100644 --- a/InterfacePlayerRDK.cpp +++ b/InterfacePlayerRDK.cpp @@ -148,7 +148,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), -lastKnownPTS(0), ptsUpdatedTimeMS(0), ptsCheckForEosOnUnderflowIdleTaskId(GST_TASK_ID_INVALID), +seekPausedState(0),lastKnownPTS(0), ptsUpdatedTimeMS(0), ptsCheckForEosOnUnderflowIdleTaskId(GST_TASK_ID_INVALID), numberOfVideoBuffersSent(0), segmentStart(0), positionQuery(NULL), paused(false), pipelineState(GST_STATE_NULL), firstVideoFrameDisplayedCallbackTask("FirstVideoFrameDisplayedCallback"), @@ -512,6 +512,13 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF interfacePlayerPriv->gstPrivateContext->buffering_in_progress = true; interfacePlayerPriv->gstPrivateContext->buffering_timeout_cnt = DEFAULT_BUFFERING_MAX_CNT; + // buffering_timeout will handle the PLAYING transition, so seekPausedState must not block it. + if (interfacePlayerPriv->gstPrivateContext->seekPausedState) + { + MW_LOG_WARN("ConfigurePipeline: clearing seekPausedState — buffering will drive PLAYING transition"); + interfacePlayerPriv->gstPrivateContext->seekPausedState = false; + } + if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PAUSED) == GST_STATE_CHANGE_FAILURE) { MW_LOG_ERR("InterfacePlayerRDK_Configure GST_STATE_PAUSED failed"); @@ -522,6 +529,32 @@ void InterfacePlayerRDK::ConfigurePipeline(int format, int audioFormat, int subF 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) + { + MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PLAYING failed"); + } + interfacePlayerPriv->gstPrivateContext->pendingPlayState = false; + interfacePlayerPriv->gstPrivateContext->paused = false; + } + if (SetStateWithWarnings(interfacePlayerPriv->gstPrivateContext->pipeline, GST_STATE_PLAYING) == GST_STATE_CHANGE_FAILURE) { MW_LOG_ERR("InterfacePlayerRDK: GST_STATE_PLAYING failed"); @@ -1597,12 +1630,30 @@ 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 rate indicates playback (not paused seek), clear seekPausedState + if (rate > 0 && !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 @@ -1680,8 +1731,22 @@ bool InterfacePlayerRDK::Flush(double position, int rate, bool shouldTearDown, b { if ((interfacePlayerPriv->socInterface->IsSimulatorSink() || interfacePlayerPriv->gstPrivateContext->usingRialtoSink) && rate != GST_NORMAL_PLAY_RATE) { - MW_LOG_INFO("Resetting seek position to zero"); - position = 0; + const bool isTrickplay = (rate != GST_NORMAL_PLAY_RATE); + const bool isRialtoOrSimulator = (interfacePlayerPriv->socInterface->IsSimulatorSink() || interfacePlayerPriv->gstPrivateContext->usingRialtoSink); + const bool isLiveMedia = (static_cast(m_gstConfigParam->media) == eGST_MEDIAFORMAT_OTA); + + if (isRialtoOrSimulator && isTrickplay) + { + if (isLiveMedia) + { + MW_LOG_WARN("Live trickplay active - preserving flush seek position %f", position); + } + else + { + MW_LOG_INFO("Resetting seek position to zero"); + position = 0; + } + } } } if (!gst_element_seek(interfacePlayerPriv->gstPrivateContext->pipeline, playRate, GST_FORMAT_TIME, GST_SEEK_FLAG_FLUSH, GST_SEEK_TYPE_SET, @@ -3402,9 +3467,32 @@ bool InterfacePlayerRDK::Pause(bool pause , bool forceStopGstreamerPreBuffering) /* 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)) { - MW_LOG_ERR("InterfacePlayerRDK_Pause - validateStateWithMsTimeout - FAILED GstState %d", nextState); + 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, ¤t, &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; + } } } else if (GST_STATE_CHANGE_SUCCESS != rc) @@ -4500,7 +4588,17 @@ static gboolean bus_message(GstBus * bus, GstMessage * msg, InterfacePlayerRDK * if(eGST_MEDIAFORMAT_DASH != static_cast(pInterfacePlayerRDK->m_gstConfigParam->media)) { SetStateWithWarnings(privatePlayer->gstPrivateContext->pipeline, GST_STATE_PAUSED); - SetStateWithWarnings(privatePlayer->gstPrivateContext->pipeline, GST_STATE_PLAYING); + /* 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; + } } break; @@ -4566,7 +4664,26 @@ bool InterfacePlayerRDK::SetPlayBackRate(double rate) 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); + + /* If application requested resume via rate change but middleware's + * seek-paused protection left the pipeline in PAUSED, ensure we clear + * 'seekPausedState` here at middleware level. This handles cases where + * higher-level callers may retry or skip setting rate — forcing an + * explicit resume in the middleware prevents the pipeline from being + * stuck in PAUSED.*/ + if (rate != 0.0 && interfacePlayerPriv->gstPrivateContext->seekPausedState && interfacePlayerPriv->gstPrivateContext->paused) + { + MW_LOG_WARN("InterfacePlayerRDK: SetPlayBackRate detected resume while seekPausedState active — forcing resume"); + /* Pause(false) clears seekPausedState in Pause implementation. */ + Pause(false, false); + interfacePlayerPriv->gstPrivateContext->seekPausedState = false; + interfacePlayerPriv->gstPrivateContext->pendingPlayState = false; + /* After explicit resume we consider operation successful */ + ret = true; + } + return ret; } @@ -4686,6 +4803,19 @@ static gboolean buffering_timeout (gpointer data) } else if (frames == -1 || frames >= pInterfacePlayerRDK->m_gstConfigParam->framesToQueue || (privatePlayer->gstPrivateContext->buffering_timeout_cnt > 0 && --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 (cnt %u, frames %d)", privatePlayer->gstPrivateContext->buffering_timeout_cnt, frames); + if (privatePlayer->gstPrivateContext->buffering_timeout_cnt == 0) + { + MW_LOG_ERR("buffering_timeout: seekPausedState still active after timeout exhausted — clearing to unblock"); + privatePlayer->gstPrivateContext->seekPausedState = false; + } + return privatePlayer->gstPrivateContext->buffering_in_progress; + } 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); @@ -5097,6 +5227,16 @@ 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) diff --git a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index 95585061..62151444 100644 --- a/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -1985,10 +1985,12 @@ TEST_F(InterfacePlayerTests, Pause_Success) mPlayerContext->pipeline = &gst_element_pipeline; EXPECT_CALL(*g_mockGStreamer, gst_element_get_state(_, NotNull(), NotNull(), _)) - .WillRepeatedly(Return(GST_STATE_CHANGE_SUCCESS)); + .WillOnce(DoAll(SetArgPointee<1>(GST_STATE_READY), SetArgPointee<2>(GST_STATE_READY), Return(GST_STATE_CHANGE_SUCCESS))) + .WillRepeatedly(DoAll(SetArgPointee<1>(GST_STATE_PAUSED), SetArgPointee<2>(GST_STATE_PAUSED), Return(GST_STATE_CHANGE_SUCCESS))); EXPECT_CALL(*g_mockGStreamer, gst_element_set_state(_, GST_STATE_PAUSED)) - .WillOnce(Return(GST_STATE_CHANGE_ASYNC)); + .Times(::testing::AnyNumber()) + .WillRepeatedly(Return(GST_STATE_CHANGE_SUCCESS)); bool result = mInterfaceGstPlayer->Pause(pause, forceStopGstreamerPreBuffering); @@ -3298,3 +3300,49 @@ TEST_F(InterfacePlayerTests, GetVideoPTS_PropertyProbeOnceAtCreation) EXPECT_EQ(mInterfaceGstPlayer->GetVideoPTS(), ptsValue); EXPECT_EQ(mInterfaceGstPlayer->GetVideoPTS(), ptsValue); } + +/** + * @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 = nullptr; + + // Act: Request a resume rate + bool result = mInterfaceGstPlayer->SetPlayBackRate(1.0); + + // Assert: Middleware should clear seekPausedState and pendingPlayState, and return true + EXPECT_TRUE(result); + EXPECT_EQ(mPlayerContext->seekPausedState, false); + EXPECT_EQ(mPlayerContext->pendingPlayState, false); +}