From 86e2ebc1d21dbae4509b2482fa5fd154d3e366ef Mon Sep 17 00:00:00 2001 From: rkandh015 Date: Wed, 8 Jul 2026 13:17:16 +0530 Subject: [PATCH] Revert commit d3486b4430a2c785140d7415d53ccbd1e4dddb52 --- middleware/InterfacePlayerPriv.h | 1 - middleware/InterfacePlayerRDK.cpp | 140 ++---------------- .../InterfacePlayerFunctionTests.cpp | 117 +-------------- 3 files changed, 13 insertions(+), 245 deletions(-) diff --git a/middleware/InterfacePlayerPriv.h b/middleware/InterfacePlayerPriv.h index e796af9cec..5c80a6b7bc 100755 --- a/middleware/InterfacePlayerPriv.h +++ b/middleware/InterfacePlayerPriv.h @@ -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. */ diff --git a/middleware/InterfacePlayerRDK.cpp b/middleware/InterfacePlayerRDK.cpp index 34dcf78f9d..6bfd9c85e5 100644 --- a/middleware/InterfacePlayerRDK.cpp +++ b/middleware/InterfacePlayerRDK.cpp @@ -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"), @@ -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; @@ -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); @@ -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, @@ -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, ¤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; - } + MW_LOG_ERR("InterfacePlayerRDK_Pause - validateStateWithMsTimeout - FAILED GstState %d", nextState); } } else if (GST_STATE_CHANGE_SUCCESS != rc) @@ -4473,17 +4403,7 @@ 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); - /* 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; @@ -4542,20 +4462,6 @@ bool InterfacePlayerRDK::SetPlayBackRate(double rate) bool ret = false; std::vector 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(eGST_MEDIATYPE_SUBTITLE) && interfacePlayerPriv->gstPrivateContext->stream[iTrack].source != nullptr) @@ -4563,9 +4469,7 @@ 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); - + ret = interfacePlayerPriv->socInterface->SetPlaybackRate(sources, interfacePlayerPriv->gstPrivateContext->pipeline, rate, interfacePlayerPriv->gstPrivateContext->video_dec,interfacePlayerPriv->gstPrivateContext->audio_dec); return ret; } @@ -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); @@ -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) diff --git a/middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp b/middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp index 81e9d2a45b..f69b8230c5 100644 --- a/middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp +++ b/middleware/test/utests/tests/InterfacePlayerTests/InterfacePlayerFunctionTests.cpp @@ -29,36 +29,7 @@ #include "MockGstUtils.h" #include #include -#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& 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; @@ -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)); @@ -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); -}