Motown: Change player state machine to support preroll

services/media/factory_service/media_player_impl.cc

Index: services/media/factory_service/media_player_impl.cc
diff --git a/services/media/factory_service/media_player_impl.cc b/services/media/factory_service/media_player_impl.cc
index e174c2f90dcd219f930ab7b4d656e67b04682e2a..74c242830d5b72cd57df2accc1d8f0fb56357c1a 100644
--- a/services/media/factory_service/media_player_impl.cc
+++ b/services/media/factory_service/media_player_impl.cc
@@ -83,7 +83,7 @@ MediaPlayerImpl::MediaPlayerImpl(InterfaceHandle<SeekingReader> reader,
     callback_joiner->WhenJoined([this]() {
       // The enabled streams are prepared.
       factory_.reset();
-      state_ = State::kPaused;
+      state_ = State::kFlushed;
       Update();
     });
   });
@@ -92,75 +92,150 @@ MediaPlayerImpl::MediaPlayerImpl(InterfaceHandle<SeekingReader> reader,
 MediaPlayerImpl::~MediaPlayerImpl() {}
 
 void MediaPlayerImpl::Update() {
+  // This method is called whenever we might want to take action based on the
+  // current state and recent events. The current state is in |state_|. Recent
+  // events are recorded in |target_state_|, which indicates what state we'd
+  // like to transition to, |target_position_|, which can indicate a position
+  // we'd like to stream to, and |end_of_stream_| which tells us we've reached
+  // end of stream.
+  //
+  // The states are as follows:
+  //
+  // |kWaiting| - Indicates that we've done something asynchronous, and no
+  //              further action should be taken by the state machine until that
+  //              something completes (at which point the callback will change
+  //              the state and call |Update|).
+  // |kFlushed| - Indicates that presentation time is not progressing and that
+  //              the pipeline is not primed with packets. This is the initial
+  //              state and the state we transition to in preparation for
+  //              seeking. A seek is currently only done when when the pipeline
+  //              is clear of packets.
+  // |kPrimed| -  Indicates that presentation time is progressing and that the

[kulakowski, 2016/06/21 01:22:54]

not progressing?

[dalesat, 2016/06/21 16:57:35]

Done.

+  //              pipeline is primed with packets. We transition to this state
+  //              when the client calls |Pause|, either from |kFlushed| or
+  //              |kPlaying| state.
+  // |kPlaying| - Indicates that presentation time is progressing and there are
+  //              packets in the pipeline. We transition to this state when the
+  //              client calls |Play|. If we're in |kFlushed| when |Play| is
+  //              called, we transition through |kPrimed| state.
+  //
+  // The while loop that surrounds all the logic below is there because, after
+  // taking some action and transitioning to a new state, we may want to check
+  // to see if there's more to do in the new state. You'll also notice that
+  // the callback lambdas generally call |Update|.
   while (true) {
     switch (state_) {
-      case State::kPaused:
+      case State::kFlushed:
+        // Presentation time is not progressing, and the pipeline is clear of
+        // packets.
         if (target_position_ != kUnspecifiedTime) {
-          WhenPausedAndSeeking();
-          break;
-        }
+          // We want to seek. Enter |kWaiting| state until the operation is
+          // complete.
+          state_ = State::kWaiting;
+          demux_->Seek(target_position_, [this]() {
+            transform_subject_time_ = target_position_;
+            target_position_ = kUnspecifiedTime;
+            state_ = State::kFlushed;
+            // Back in |kFlushed|. Call |Update| to see if there's further
+            // action to be taken.
+            Update();
+          });
 
-        if (target_state_ == State::kPlaying) {
-          if (!flushed_) {
-            SetTimelineTransform(1, 1);
-            state_ = State::kWaiting;
-            break;
-          }
+          // Done for now. We're in kWaiting, and the callback will call Update
+          // when the Seek call is complete.
+          return;
+        }
 
-          flushed_ = false;
+        if (target_state_ == State::kPlaying ||
+            target_state_ == State::kPrimed) {
+          // We want to transition to |kPrimed| or to |kPlaying|, for which
+          // |kPrimed| is a prerequisite. We enter |kWaiting| state, issue the
+          // |Prime| request and transition to |kPrimed| when the operation is
+          // complete.
           state_ = State::kWaiting;
           demux_->Prime([this]() {
-            SetTimelineTransform(1, 1);
-            state_ = State::kWaiting;
+            state_ = State::kPrimed;

[kulakowski, 2016/06/21 01:22:54]

Ok cool this is much easier for me to follow now. Eg this chunk primes. If the
target state was playing and not just primed, there'll be more work to do after
the async work.

[dalesat, 2016/06/21 16:57:34]

Acknowledged.

+            // Now we're in |kPrimed|. Call |Update| to see if there's further
+            // action to be taken.
             Update();
           });
+
+          // Done for now. We're in |kWaiting|, and the callback will call
+          // |Update| when the prime is complete.
+          return;
         }
+
+        // No interesting events to respond to. Done for now.
+        return;
+
+      case State::kPrimed:
+        // Presentation time is not progressing, and the pipeline is primed with
+        // packets.
+        if (target_position_ != kUnspecifiedTime ||
+            target_state_ == State::kFlushed) {
+          // Either we want to seek or just want to transition to |kFlushed|.
+          // We transition to |kWaiting|, issue the |Flush| request and
+          // transition to |kFlushed| when the operation is complete.
+          state_ = State::kWaiting;
+          demux_->Flush([this]() {
+            state_ = State::kFlushed;
+            // Now we're in |kFlushed|. Call |Update| to see if there's further
+            // action to be taken.
+            Update();
+          });
+
+          // Done for now. We're in |kWaiting|, and the callback will call
+          // |Update| when the flush is complete.
+          return;
+        }
+
+        if (target_state_ == State::kPlaying) {
+          // We want to transition to |kPlaying|. Start the presentation
+          // timeline and enter |kWaiting|. |SetTimelineTransform| will

[kulakowski, 2016/06/21 01:22:54]

Ah cool, this was one other big missing piece when I was staring at this
earlier. That SetTimelineTransform can transition.

(Actually, that could use a comment in the header perhaps.)

[dalesat, 2016/06/21 16:57:35]

I noticed this before but left it alone. The way I'm calling
SetTimelineTransform here is inconsistent with the way other async calls are
done from this method. To fix that, I've gotten rid of the SetTimelineTransform
method and made the call to to TimelineConsumer directly from Update. That puts
the callback here...consistent with other async operations.

+          // transition to |kPlaying| and call |Update| when the operation
+          // completes.
+          SetTimelineTransform(1, 1);
+          state_ = State::kWaiting;
+          return;
+        }
+
+        // No interesting events to respond to. Done for now.
         return;
 
       case State::kPlaying:
+        // Presentation time is progressing, and packets are moving through
+        // the pipeline.
         if (target_position_ != kUnspecifiedTime ||
-            target_state_ == State::kPaused) {
+            target_state_ == State::kFlushed ||
+            target_state_ == State::kPrimed) {
+          // Either we want to seek or we want to stop playback. In either case,
+          // we need to stop the presentation timeline and enter kWaiting.
+          // |SetTimelineTransform| will transition to |kPrimed| and call
+          // |Update| when the operation completes.
           SetTimelineTransform(1, 0);

[kulakowski, 2016/06/21 01:22:54]

ditto here, and it indeed makes sense that 1, 1 would kPlay and 1, 0 would
kPrime.

[dalesat, 2016/06/21 16:57:35]

Acknowledged.

           state_ = State::kWaiting;
-          break;
+          return;
         }
 
         if (end_of_stream_) {
-          target_state_ = State::kPaused;
-          state_ = State::kPaused;
+          // We've reached end of stream. The presentation timeline stops by
+          // itself, so we just need to transition to |kPrimed|.
+          target_state_ = State::kPrimed;
+          state_ = State::kPrimed;
+          // Loop around to check if there's more work to do.
           break;
         }
+
+        // No interesting events to respond to. Done for now.
         return;
 
       case State::kWaiting:
+        // Waiting for some async operation. Nothing to do until it completes.
         return;
     }
   }
 }
 
-void MediaPlayerImpl::WhenPausedAndSeeking() {
-  if (!flushed_) {
-    state_ = State::kWaiting;
-    demux_->Flush([this]() {
-      flushed_ = true;
-      WhenFlushedAndSeeking();
-    });
-  } else {
-    WhenFlushedAndSeeking();
-  }
-}
-
-void MediaPlayerImpl::WhenFlushedAndSeeking() {
-  state_ = State::kWaiting;
-  DCHECK(target_position_ != kUnspecifiedTime);
-  demux_->Seek(target_position_, [this]() {
-    transform_subject_time_ = target_position_;
-    target_position_ = kUnspecifiedTime;
-    state_ = State::kPaused;
-    Update();
-  });
-}
-
 void MediaPlayerImpl::SetTimelineTransform(uint32_t reference_delta,
                                            uint32_t subject_delta) {
   timeline_consumer_->SetTimelineTransform(
@@ -170,13 +245,15 @@ void MediaPlayerImpl::SetTimelineTransform(uint32_t reference_delta,
         RCHECK(state_ == State::kWaiting);
 
         if (subject_delta == 0) {
-          state_ = State::kPaused;
+          state_ = State::kPrimed;
         } else {
           state_ = State::kPlaying;
         }
 
         Update();
       });
+
+  transform_subject_time_ = kUnspecifiedTime;
 }
 
 void MediaPlayerImpl::GetStatus(uint64_t version_last_seen,
@@ -190,7 +267,7 @@ void MediaPlayerImpl::Play() {
 }
 
 void MediaPlayerImpl::Pause() {
-  target_state_ = State::kPaused;
+  target_state_ = State::kPrimed;
   Update();
 }