Changes to the way flushing is handled

direct-rialto/AampPlayerStateMachine.cpp

@@ -41,7 +41,8 @@ class SourcesAttachedState;
 class PlayingState;
 class PausedState;
 class FlushingState;
-class StoppedState;
+class StoppedState;  // forward decl retained; Stop() calls WaitForFlushToComplete
+					  // so onStop() should never be dispatched from FLUSHING
 class ErrorState;
 
 // ============================================================================
@@ -55,6 +56,7 @@ class IdleState final : public IPlayerState
 	const char   *name() const override { return "IDLE"; }
 
 	std::unique_ptr<IPlayerState> onPipelineLoaded() override;
+	std::unique_ptr<IPlayerState> onReconfigure()    override; // first-tune reset
 };
 
 class PipelineCreatedState final : public IPlayerState
@@ -64,6 +66,9 @@ class PipelineCreatedState final : public IPlayerState
 	const char   *name() const override { return "PIPELINE_CREATED"; }
 
 	std::unique_ptr<IPlayerState> onSourceAttaching() override;
+	std::unique_ptr<IPlayerState> onStop()            override;
+	std::unique_ptr<IPlayerState> onError()           override;
+	std::unique_ptr<IPlayerState> onReconfigure()     override;
 };
 
 class SourcesAttachingState final : public IPlayerState
@@ -73,6 +78,9 @@ class SourcesAttachingState final : public IPlayerState
 	const char   *name() const override { return "SOURCES_ATTACHING"; }
 
 	std::unique_ptr<IPlayerState> onAllSourcesAttached() override;
+	std::unique_ptr<IPlayerState> onStop()               override;
+	std::unique_ptr<IPlayerState> onError()              override;
+	std::unique_ptr<IPlayerState> onReconfigure()        override;
 };
 
 class SourcesAttachedState final : public IPlayerState
@@ -83,6 +91,9 @@ class SourcesAttachedState final : public IPlayerState
 
 	std::unique_ptr<IPlayerState> onPlaybackStarted() override;
 	std::unique_ptr<IPlayerState> onFlush()           override;
+	std::unique_ptr<IPlayerState> onStop()            override;
+	std::unique_ptr<IPlayerState> onError()           override;
+	std::unique_ptr<IPlayerState> onReconfigure()     override;
 };
 
 class PlayingState final : public IPlayerState
@@ -93,8 +104,12 @@ class PlayingState final : public IPlayerState
 
 	std::unique_ptr<IPlayerState> onPlaybackPaused()  override;
 	std::unique_ptr<IPlayerState> onFlush()           override;
-	// Tolerate a duplicate PLAYING notification without transitioning.
-	std::unique_ptr<IPlayerState> onPlaybackStarted() override { return nullptr; }
+	std::unique_ptr<IPlayerState> onStop()            override;
+	std::unique_ptr<IPlayerState> onError()           override;
+	std::unique_ptr<IPlayerState> onReconfigure()     override;
+	// Duplicate onPlaybackStarted while already PLAYING is handled at the
+	// PlayerStateMachine level (see onPlaybackStarted() below) so dispatch()
+	// never sees an unexpected null for this event.
 };
 
 class PausedState final : public IPlayerState
@@ -105,6 +120,9 @@ class PausedState final : public IPlayerState
 
 	std::unique_ptr<IPlayerState> onPlaybackStarted() override;
 	std::unique_ptr<IPlayerState> onFlush()           override;
+	std::unique_ptr<IPlayerState> onStop()            override;
+	std::unique_ptr<IPlayerState> onError()           override;
+	std::unique_ptr<IPlayerState> onReconfigure()     override;
 };
 
 class FlushingState final : public IPlayerState
@@ -113,50 +131,47 @@ class FlushingState final : public IPlayerState
 	PlayerStateId id()   const override { return PlayerStateId::FLUSHING; }
 	const char   *name() const override { return "FLUSHING"; }
 
-	/// After a flush new init fragments arrive, restarting source attachment.
-	std::unique_ptr<IPlayerState> onSourceAttaching() override;
-
-	/// Rialto sends PLAYING while flushing (e.g. after seek completes).
-	std::unique_ptr<IPlayerState> onPlaybackStarted() override;
-
-	/// Rialto sends PAUSED while flushing (e.g. seek with keepPaused=1).
-	std::unique_ptr<IPlayerState> onPlaybackPaused() override;
+	std::unique_ptr<IPlayerState> onError() override;
+
+	// onPlaybackStarted() and onPlaybackPaused() are intentionally NOT
+	// overridden here.  When Rialto sends PLAYING or PAUSED during a flush
+	// (delayed ack for a play() that was in-flight when Flush() started),
+	// PlayerStateMachine::onPlaybackStarted/Paused() intercepts the call,
+	// updates m_preFlushStateId, and returns without transitioning.  The
+	// machine stays in FLUSHING until onFlushComplete() fires — keeping
+	// WaitForFlushToComplete() correctly blocked until all sources confirm
+	// flushed.  See PlayerStateMachine::onPlaybackStarted/Paused() below.
+	//
+	// onStop() is not overridden: Stop() calls WaitForFlushToComplete()
+	// before dispatching onStop(), so FLUSHING is never the current state
+	// when onStop is dispatched.
+	//
+	// onReconfigure() is not overridden: Configure() calls Stop() first,
+	// which waits for flush to complete before onReconfigure() is fired.
 };
 
+// StoppedState is removed; onStop() now transitions directly to IDLE.
+// The forward decl above is kept so the compiler does not complain if any
+// stale reference exists during incremental builds.
 class StoppedState final : public IPlayerState
 {
 public:
 	PlayerStateId id()   const override { return PlayerStateId::STOPPED; }
 	const char   *name() const override { return "STOPPED"; }
-	// Re-configure (re-tune) resets to IDLE; handled by base onReconfigure().
 };
 
 class ErrorState final : public IPlayerState
 {
 public:
 	PlayerStateId id()   const override { return PlayerStateId::ERROR; }
 	const char   *name() const override { return "ERROR"; }
-	// Re-configure after error resets to IDLE; handled by base onReconfigure().
-};
-
-// ============================================================================
-// IPlayerState base implementations for cross-state events
-// ============================================================================
-
-std::unique_ptr<IPlayerState> IPlayerState::onStop()
-{
-	return std::make_unique<StoppedState>();
-}
-
-std::unique_ptr<IPlayerState> IPlayerState::onError()
-{
-	return std::make_unique<ErrorState>();
-}
 
-std::unique_ptr<IPlayerState> IPlayerState::onReconfigure()
-{
-	return std::make_unique<IdleState>();
-}
+	std::unique_ptr<IPlayerState> onStop()        override;
+	std::unique_ptr<IPlayerState> onReconfigure() override;
+	// onError() is not overridden: already in ERROR; a second fatal
+	// notification would produce a no-op (dispatch warns) rather than
+	// a spurious self-transition.
+};
 
 // ============================================================================
 // Concrete state transition bodies
@@ -207,20 +222,34 @@ std::unique_ptr<IPlayerState> PausedState::onFlush()
 	return std::make_unique<FlushingState>();
 }
 
-std::unique_ptr<IPlayerState> FlushingState::onSourceAttaching()
-{
-	return std::make_unique<SourcesAttachingState>();
-}
-
-std::unique_ptr<IPlayerState> FlushingState::onPlaybackStarted()
-{
-	return std::make_unique<PlayingState>();
-}
-
-std::unique_ptr<IPlayerState> FlushingState::onPlaybackPaused()
-{
-	return std::make_unique<PausedState>();
-}
+// onStop — valid from any active state (except FLUSHING; Stop waits for
+// flush before dispatching, and IDLE has nothing to stop).
+std::unique_ptr<IPlayerState> PipelineCreatedState::onStop()      { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachingState::onStop()     { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachedState::onStop()      { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> PlayingState::onStop()              { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> PausedState::onStop()               { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> ErrorState::onStop()                { return std::make_unique<IdleState>(); }
+
+// onError — valid from any state in which a Rialto FAILURE notification
+// can arrive (FLUSHING included; IDLE and ERROR itself are excluded).
+std::unique_ptr<IPlayerState> PipelineCreatedState::onError()     { return std::make_unique<ErrorState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachingState::onError()    { return std::make_unique<ErrorState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachedState::onError()     { return std::make_unique<ErrorState>(); }
+std::unique_ptr<IPlayerState> PlayingState::onError()             { return std::make_unique<ErrorState>(); }
+std::unique_ptr<IPlayerState> PausedState::onError()              { return std::make_unique<ErrorState>(); }
+std::unique_ptr<IPlayerState> FlushingState::onError()            { return std::make_unique<ErrorState>(); }
+
+// onReconfigure — Configure() fires this on every tune, including first tune
+// from IDLE, retune from any active state, and recovery after ERROR.
+// Not valid from FLUSHING (Configure calls Stop first, which waits for flush).
+std::unique_ptr<IPlayerState> IdleState::onReconfigure()          { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> PipelineCreatedState::onReconfigure()   { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachingState::onReconfigure()  { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> SourcesAttachedState::onReconfigure()   { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> PlayingState::onReconfigure()       { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> PausedState::onReconfigure()        { return std::make_unique<IdleState>(); }
+std::unique_ptr<IPlayerState> ErrorState::onReconfigure()         { return std::make_unique<IdleState>(); }
 
 // ============================================================================
 // PlayerStateMachine
@@ -246,7 +275,8 @@ const char *PlayerStateMachine::currentStateName() const
 }
 
 void PlayerStateMachine::dispatch(
-	std::unique_ptr<IPlayerState> (IPlayerState::*handler)())
+	std::unique_ptr<IPlayerState> (IPlayerState::*handler)(),
+	const char *eventName)
 {
 	std::lock_guard<std::mutex> lock(m_mutex);
 	auto next = (m_state.get()->*handler)();
@@ -256,49 +286,140 @@ void PlayerStateMachine::dispatch(
 			m_state->name(), next->name());
 		m_state = std::move(next);
 	}
+	else
+	{
+		AAMPLOG_WARN("PlayerState: event '%s' has no transition from state '%s' - ignored",
+			eventName, m_state->name());
+	}
 }
 
 void PlayerStateMachine::onPipelineLoaded()
 {
-	dispatch(&IPlayerState::onPipelineLoaded);
+	dispatch(&IPlayerState::onPipelineLoaded, "onPipelineLoaded");
 }
 
 void PlayerStateMachine::onSourceAttaching()
 {
-	dispatch(&IPlayerState::onSourceAttaching);
+	dispatch(&IPlayerState::onSourceAttaching, "onSourceAttaching");
 }
 
 void PlayerStateMachine::onAllSourcesAttached()
 {
-	dispatch(&IPlayerState::onAllSourcesAttached);
+	dispatch(&IPlayerState::onAllSourcesAttached, "onAllSourcesAttached");
 }
 
 void PlayerStateMachine::onPlaybackStarted()
 {
-	dispatch(&IPlayerState::onPlaybackStarted);
+	{
+		// Intercept two cases before calling dispatch() so it never receives
+		// an expected null return:
+		//
+		// 1. FLUSHING: delayed PLAYING ack — update pre-flush state only.
+		// 2. PLAYING: duplicate PLAYING notification — silently tolerated.
+		std::lock_guard<std::mutex> lock(m_mutex);
+		if (m_state->id() == PlayerStateId::FLUSHING)
+		{
+			AAMPLOG_INFO("PlayerState: onPlaybackStarted during FLUSHING - "
+				"updating pre-flush state to PLAYING, "
+				"state remains FLUSHING until all sources flush");
+			m_preFlushStateId = PlayerStateId::PLAYING;
+			return;
+		}
+		if (m_state->id() == PlayerStateId::PLAYING)
+		{
+			AAMPLOG_INFO("PlayerState: duplicate onPlaybackStarted in PLAYING - ignored");
+			return;
+		}
+	}
+	dispatch(&IPlayerState::onPlaybackStarted, "onPlaybackStarted");
 }
 
 void PlayerStateMachine::onPlaybackPaused()
 {
-	dispatch(&IPlayerState::onPlaybackPaused);
+	{
+		// Edge-case race: same reasoning as onPlaybackStarted above.
+		std::lock_guard<std::mutex> lock(m_mutex);
+		if (m_state->id() == PlayerStateId::FLUSHING)
+		{
+			AAMPLOG_INFO("PlayerState: onPlaybackPaused during FLUSHING - "
+				"updating pre-flush state to PAUSED, "
+				"state remains FLUSHING until all sources flush");
+			m_preFlushStateId = PlayerStateId::PAUSED;
+			return;
+		}
+	}
+	dispatch(&IPlayerState::onPlaybackPaused, "onPlaybackPaused");
 }
 
 void PlayerStateMachine::onFlush()
 {
-	dispatch(&IPlayerState::onFlush);
+	// Save the current state before entering FLUSHING so that
+	// onFlushComplete() can restore it when all sources report flushed.
+	std::lock_guard<std::mutex> lock(m_mutex);
+	m_preFlushStateId = m_state->id();
+	AAMPLOG_INFO("PlayerState: saving pre-flush state '%s' (id=%d)",
+		m_state->name(), static_cast<int>(m_preFlushStateId));
+	auto next = m_state->onFlush();
+	if (next)
+	{
+		AAMPLOG_MIL("PlayerState: %s -> %s",
+			m_state->name(), next->name());
+		m_state = std::move(next);
+	}
+}
+
+void PlayerStateMachine::onFlushComplete()
+{
+	std::lock_guard<std::mutex> lock(m_mutex);
+
+	if (m_state->id() != PlayerStateId::FLUSHING)
+	{
+		// The edge-case race: onPlaybackStarted/Paused already exited
+		// FLUSHING before SEEK_DONE arrived.  This call is a no-op —
+		// the correct post-flush state was already applied.
+		AAMPLOG_INFO("PlayerState: onFlushComplete ignored - current state is '"
+			"%s', not FLUSHING (edge-case race already resolved)",
+			m_state->name());
+		return;
+	}
+
+	std::unique_ptr<IPlayerState> restored;
+	switch (m_preFlushStateId)
+	{
+	case PlayerStateId::PLAYING:
+		restored = std::make_unique<PlayingState>();
+		break;
+	case PlayerStateId::PAUSED:
+		restored = std::make_unique<PausedState>();
+		break;
+	case PlayerStateId::SOURCES_ATTACHED:
+		restored = std::make_unique<SourcesAttachedState>();
+		break;
+	default:
+		AAMPLOG_WARN(
+			"PlayerState: onFlushComplete - unexpected pre-flush state %d,"
+			" restoring SOURCES_ATTACHED as safe fallback",
+			static_cast<int>(m_preFlushStateId));
+		restored = std::make_unique<SourcesAttachedState>();
+		break;
+	}
+
+	AAMPLOG_MIL("PlayerState: %s -> %s (flush complete; pre-flush state restored)",
+		m_state->name(), restored->name());
+	m_state = std::move(restored);
 }
 
 void PlayerStateMachine::onStop()
 {
-	dispatch(&IPlayerState::onStop);
+	dispatch(&IPlayerState::onStop, "onStop");
 }
 
 void PlayerStateMachine::onError()
 {
-	dispatch(&IPlayerState::onError);
+	dispatch(&IPlayerState::onError, "onError");
 }
 
 void PlayerStateMachine::onReconfigure()
 {
-	dispatch(&IPlayerState::onReconfigure);
+	dispatch(&IPlayerState::onReconfigure, "onReconfigure");
 }