Rewrite media::Pipeline state transition machinery and simplify shutdown.

media/base/pipeline.h

Index: media/base/pipeline.h
diff --git a/media/base/pipeline.h b/media/base/pipeline.h
index 9325f1ab5c199a7ac6985c87d28cc205a64b5729..01ec09a603221c713e904cb9db2e647b66366613 100644
--- a/media/base/pipeline.h
+++ b/media/base/pipeline.h
@@ -232,9 +232,8 @@ class MEDIA_EXPORT Pipeline
     kInitAudioDecoder,
     kInitAudioRenderer,
     kInitVideoRenderer,
-    kPausing,
+    kInitPrerolling,
     kSeeking,
-    kFlushing,
     kStarting,
     kStarted,
     kStopping,
@@ -244,23 +243,13 @@ class MEDIA_EXPORT Pipeline
   // Updates |state_|. All state transitions should use this call.
   void SetState(State next_state);
 
-  // Simple method used to make sure the pipeline is running normally.
-  bool IsPipelineOk();
-
-  // Helper method to tell whether we are in transition to seek state.
-  bool IsPipelineSeeking();
+  static const char* GetStateString(State state);
+  State GetNextState() const;
 
   // Helper method that runs & resets |seek_cb_| and resets |seek_timestamp_|
   // and |seek_pending_|.
   void FinishSeek();
 
-  // Returns true if the given state is one that transitions to a new state
-  // after iterating through each filter.
-  static bool TransientState(State state);
-
-  // Given the current state, returns the next state.
-  State FindNextState(State current);
-
   // DataSourceHost (by way of DemuxerHost) implementation.
   virtual void SetTotalBytes(int64 total_bytes) OVERRIDE;
   virtual void AddBufferedByteRange(int64 start, int64 end) OVERRIDE;
@@ -283,16 +272,6 @@ class MEDIA_EXPORT Pipeline
   void OnAudioRendererEnded();
   void OnVideoRendererEnded();
 
-  // Callbacks executed by filters upon completing initialization.
-  void OnFilterInitialize(PipelineStatus status);
-
-  // Callback executed by filters upon completing Play(), Pause(), Flush(),
-  // Seek() or Stop().
-  void OnFilterStateTransition(PipelineStatus status);
-
-  // Callback executed by filters when completing teardown operations.
-  void OnTeardownStateTransition(PipelineStatus status);
-
   // Callback executed by filters to update statistics.
   void OnUpdateStatistics(const PipelineStatistics& stats);
 
@@ -314,13 +293,6 @@ class MEDIA_EXPORT Pipeline
                  const PipelineStatusCB& seek_cb,
                  const BufferingStateCB& buffering_state_cb);
 
-  // InitializeTask() performs initialization in multiple passes. It is executed
-  // as a result of calling Start() or InitializationComplete() that advances
-  // initialization to the next state. It works as a hub of state transition for
-  // initialization.  One stage communicates its status to the next through
-  // |last_stage_status|.
-  void InitializeTask(PipelineStatus last_stage_status);
-
   // Stops and destroys all filters, placing the pipeline in the kStopped state.
   void StopTask(const base::Closure& stop_cb);
 
@@ -345,38 +317,12 @@ class MEDIA_EXPORT Pipeline
   // Carries out disabling the audio renderer.
   void AudioDisabledTask();
 
-  // Carries out advancing to the next filter during Play()/Pause()/Seek().
-  void FilterStateTransitionTask();
-
-  // Carries out advancing to the next teardown operation.
-  void TeardownStateTransitionTask();
-
-  // Carries out stopping filter threads, deleting filters, running
-  // appropriate callbacks, and setting the appropriate pipeline state
-  // depending on whether we performing Stop() or SetError().
-  // Called after all filters have been stopped.
-  void FinishDestroyingFiltersTask();
-
-  // Internal methods used in the implementation of the pipeline thread.  All
-  // of these methods are only called on the pipeline thread.
-
-  // The following initialize methods are used to select a specific type of
-  // object from FilterCollection and initialize it asynchronously.
-  void InitializeDemuxer();
-  void OnDemuxerInitialized(PipelineStatus status);
-
-  // Returns true if the asynchronous action of creating decoder has started.
-  // Returns false if this method did nothing because the corresponding
-  // audio stream does not exist.
-  bool InitializeAudioDecoder(const scoped_refptr<Demuxer>& demuxer);
-
-  // Initializes a renderer and connects it with decoder. Returns true if the
-  // asynchronous action of creating renderer has started. Returns
-  // false if this method did nothing because the corresponding audio/video
-  // stream does not exist.
-  bool InitializeAudioRenderer(const scoped_refptr<AudioDecoder>& decoder);
-  bool InitializeVideoRenderer(
-      const scoped_refptr<DemuxerStream>& stream);
+  // Kicks off initialization for each media object, executing |done_cb| with
+  // the result when completed.
+  void InitializeDemuxer(const PipelineStatusCB& done_cb);
+  void InitializeAudioDecoder(const PipelineStatusCB& done_cb);
+  void InitializeAudioRenderer(const PipelineStatusCB& done_cb);
+  void InitializeVideoRenderer(const PipelineStatusCB& done_cb);
 
   // Kicks off destroying filters. Called by StopTask() and ErrorChangedTask().
   // When we start to tear down the pipeline, we will consider two cases:
@@ -390,30 +336,36 @@ class MEDIA_EXPORT Pipeline
   // Compute the time corresponding to a byte offset.
   base::TimeDelta TimeForByteOffset_Locked(int64 byte_offset) const;
 
-  // Initiates an asynchronous Pause/Seek/Play/Stop() call sequence executing
-  // |done_cb| when completed.
-  void DoPause(const PipelineStatusCB& done_cb);
-  void DoFlush(const PipelineStatusCB& done_cb);
-  void DoPlay(const PipelineStatusCB& done_cb);
-  void DoStop(const PipelineStatusCB& done_cb);
+  void OnStateTransition(PipelineStatus status);
+  void DoStateTransition(PipelineStatus status);
 
-  // Initiates an asynchronous Seek() and preroll call sequence executing
-  // |done_cb| with the final status when completed. If |skip_demuxer_seek| is
-  // true then only renderers will attempt to preroll.
+  // Initiates an asynchronous preroll call sequence executing |done_cb|
+  // with the final status when completed.
+  void DoInitialPreroll(const base::TimeDelta& seek_timestamp,

[Ami GONE FROM CHROMIUM, 2012/09/02 20:29:29]

"initial" and "seek_timestamp" seem at odds to me.
Drop the param and just use demuxer_->GetStartTime() inside?

[scherkus (not reviewing), 2012/09/05 15:04:17]

Done.

+                        const PipelineStatusCB& done_cb);
+
+  // Initiates an asynchronous pause-flush-seek-preroll call sequence
+  // executing |done_cb| with the final status when completed.
   //
   // TODO(scherkus): Prerolling should be separate from seeking so we can report
   // finer grained ready states (HAVE_CURRENT_DATA vs. HAVE_FUTURE_DATA)
   // indepentent from seeking.
-  void DoSeek(base::TimeDelta seek_timestamp, bool skip_demuxer_seek,
+  void DoSeek(const base::TimeDelta& seek_timestamp,

[Ami GONE FROM CHROMIUM, 2012/09/02 20:29:29]

this is an int64; why pass-by-ref?

[scherkus (not reviewing), 2012/09/05 15:04:17]

Habit with non-POD-types, I suppose. Done.

               const PipelineStatusCB& done_cb);
 
+  // Updates playback rate and volume and initiates an asynchronous play call
+  // sequence executing |done_cb| with the final status when completed.
+  void DoPlay(const PipelineStatusCB& done_cb);
+
+  // Initiates an asynchronous pause-flush-stop call sequence executing
+  // |done_cb| when completed.
+  void DoStop(const PipelineStatusCB& done_cb);
+  void OnStopCompleted(PipelineStatus status);
+
   void OnAudioUnderflow();
 
   void StartClockIfWaitingForTimeUpdate_Locked();
 
-  // Report pipeline |status| through |cb| avoiding duplicate error reporting.
-  void ReportStatus(const PipelineStatusCB& cb, PipelineStatus status);

[Ami GONE FROM CHROMIUM, 2012/09/02 20:29:29]

I fear this removal, but I don't think there's a bug in this CL.

[scherkus (not reviewing), 2012/09/05 15:04:17]

There isn't. Promise ;)

-
   // Message loop used to execute pipeline tasks.
   scoped_refptr<base::MessageLoopProxy> message_loop_;
 
@@ -426,15 +378,6 @@ class MEDIA_EXPORT Pipeline
   // Whether or not the pipeline is running.
   bool running_;
 
-  // Whether or not the pipeline is in transition for a seek operation.
-  bool seek_pending_;
-
-  // Whether or not the pipeline is perform a stop operation.
-  bool tearing_down_;
-
-  // Whether or not a playback rate change should be done once seeking is done.
-  bool playback_rate_change_pending_;
-
   // Amount of available buffered data.  Set by filters.
   Ranges<int64> buffered_byte_ranges_;
   Ranges<base::TimeDelta> buffered_time_ranges_;
@@ -459,9 +402,6 @@ class MEDIA_EXPORT Pipeline
   // the filters.
   float playback_rate_;
 
-  // Playback rate to set when the current seek has finished.
-  float pending_playback_rate_;
-
   // Reference clock.  Keeps track of current playback time.  Uses system
   // clock and linear interpolation, but can have its time manually set
   // by filters.
@@ -491,10 +431,6 @@ class MEDIA_EXPORT Pipeline
   // Member that tracks the current state.
   State state_;
 
-  // For kSeeking we need to remember where we're seeking between filter
-  // replies.
-  base::TimeDelta seek_timestamp_;
-
   // Whether we've received the audio/video ended events.
   bool audio_ended_;
   bool video_ended_;
@@ -502,12 +438,15 @@ class MEDIA_EXPORT Pipeline
   // Set to true in DisableAudioRendererTask().
   bool audio_disabled_;
 
-  // Filter collection as passed in by Start().
   scoped_ptr<FilterCollection> filter_collection_;
 
-  // Callbacks for various pipeline operations.
+  // Temporary callback used for Start() and Seek().
   PipelineStatusCB seek_cb_;
+
+  // Temporary callback used for Stop().
   base::Closure stop_cb_;
+
+  // Permanent callbacks passed in via Start().
   PipelineStatusCB ended_cb_;
   PipelineStatusCB error_cb_;
   BufferingStateCB buffering_state_cb_;
@@ -527,13 +466,14 @@ class MEDIA_EXPORT Pipeline
   // Demuxer reference used for setting the preload value.
   scoped_refptr<Demuxer> demuxer_;
 
-  // Helper class that stores filter references during pipeline
-  // initialization.
-  struct PipelineInitState;
-  scoped_ptr<PipelineInitState> pipeline_init_state_;
+  // Audio decoder reference used during initialization.
+  //
+  // TODO(scherkus): Remove after renderers do initialization, see
+  // http://crbug.com/145635
+  scoped_refptr<AudioDecoder> audio_decoder_;
 
-  // Statistics.
   PipelineStatistics statistics_;
+
   // Time of pipeline creation; is non-zero only until the pipeline first
   // reaches "kStarted", at which point it is used & zeroed out.
   base::Time creation_time_;