Bug 1090991 - Unify MediaDecoderStateMachine::On{DecodeError,AudioEOS,VideoEOS} and eliminate duplicated logic. r=cpearce

We take this opportunity to align the behavior of Finish() calls between audio
and video EOS, invoking them unconditionally for both cases. Currently both
cases always call Finish() immediately, with the exception of:

(A) Video in seeking mode, where we may push mFirstVideoFrameAfterSeek before
    doing so, and
(B) Video in the |default:| case.

Push() and Finish() seem like orthogonal operations on MediaQueue, but we
nonetheless preserve the old order just in case. There doesn't seem to be a good
reason for (B).

dom/media/MediaDecoderStateMachine.cpp

@@ -689,45 +689,6 @@ MediaDecoderStateMachine::IsVideoSeekComplete()
      (VideoQueue().IsFinished() || VideoQueue().GetSize() > 0));
 }
 
-void
-MediaDecoderStateMachine::OnAudioEOS()
-{
-  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
-  SAMPLE_LOG("OnAudioEOS");
-  mAudioRequestPending = false;
-  AudioQueue().Finish();
-  switch (mState) {
-    case DECODER_STATE_DECODING_METADATA: {
-      MaybeFinishDecodeMetadata();
-      return;
-    }
-    case DECODER_STATE_BUFFERING:
-    case DECODER_STATE_DECODING: {
-      CheckIfDecodeComplete();
-      SendStreamData();
-      // The ready state can change when we've decoded data, so update the
-      // ready state, so that DOM events can fire.
-      UpdateReadyState();
-      mDecoder->GetReentrantMonitor().NotifyAll();
-      return;
-    }
-
-    case DECODER_STATE_SEEKING: {
-      if (!mCurrentSeekTarget.IsValid()) {
-        // We've received an EOS from a previous decode. Discard it.
-        return;
-      }
-      mDropAudioUntilNextDiscontinuity = false;
-      CheckIfSeekComplete();
-      return;
-    }
-    default: {
-      // Ignore other cases.
-      return;
-    }
-  }
-}
-
 void
 MediaDecoderStateMachine::OnAudioDecoded(AudioData* aAudioSample)
 {
@@ -839,22 +800,75 @@ void
 MediaDecoderStateMachine::OnNotDecoded(MediaData::Type aType,
                                        RequestSampleCallback::NotDecodedReason aReason)
 {
+  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
+  SAMPLE_LOG("OnNotDecoded (aType=%u, aReason=%u)", aType, aReason);
+  bool isAudio = aType == MediaData::AUDIO_DATA;
+  MOZ_ASSERT_IF(!isAudio, aType == MediaData::VIDEO_DATA);
+
+  // This callback means that the pending request is dead.
+  if (isAudio) {
+    mAudioRequestPending = false;
+  } else {
+    mVideoRequestPending = false;
+  }
+
+  // If this is a decode error, delegate to the generic error path.
   if (aReason == RequestSampleCallback::DECODE_ERROR) {
-    OnDecodeError();
+    DecodeError();
     return;
   }
 
+  // This is an EOS. Finish off the queue, and then handle things based on our
+  // state.
   MOZ_ASSERT(aReason == RequestSampleCallback::END_OF_STREAM);
-  if (aType == MediaData::AUDIO_DATA) {
-    OnAudioEOS();
-  } else {
-    MOZ_ASSERT(aType == MediaData::VIDEO_DATA);
-    OnVideoEOS();
+  if (!isAudio && mState == DECODER_STATE_SEEKING &&
+      mCurrentSeekTarget.IsValid() && mFirstVideoFrameAfterSeek) {
+    // Null sample. Hit end of stream. If we have decoded a frame,
+    // insert it into the queue so that we have something to display.
+    // We make sure to do this before invoking VideoQueue().Finish()
+    // below.
+    VideoQueue().Push(mFirstVideoFrameAfterSeek.forget());
+  }
+  isAudio ? AudioQueue().Finish() : VideoQueue().Finish();
+  switch (mState) {
+    case DECODER_STATE_DECODING_METADATA: {
+      MaybeFinishDecodeMetadata();
+      return;
+    }
+
+    case DECODER_STATE_BUFFERING:
+    case DECODER_STATE_DECODING: {
+      CheckIfDecodeComplete();
+      SendStreamData();
+      // The ready state can change when we've decoded data, so update the
+      // ready state, so that DOM events can fire.
+      UpdateReadyState();
+      mDecoder->GetReentrantMonitor().NotifyAll();
+      return;
+    }
+    case DECODER_STATE_SEEKING: {
+      if (!mCurrentSeekTarget.IsValid()) {
+        // We've received a sample from a previous decode. Discard it.
+        return;
+      }
+
+      if (isAudio) {
+        mDropAudioUntilNextDiscontinuity = false;
+      } else {
+        mDropVideoUntilNextDiscontinuity = false;
+      }
+
+      CheckIfSeekComplete();
+      return;
+    }
+    default: {
+      return;
+    }
   }
 }
 
 void
-MediaDecoderStateMachine::OnDecodeError()
+MediaDecoderStateMachine::AcquireMonitorAndInvokeDecodeError()
 {
   ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
   DecodeError();
@@ -873,52 +887,6 @@ MediaDecoderStateMachine::MaybeFinishDecodeMetadata()
   }
 }
 
-void
-MediaDecoderStateMachine::OnVideoEOS()
-{
-  ReentrantMonitorAutoEnter mon(mDecoder->GetReentrantMonitor());
-  SAMPLE_LOG("OnVideoEOS");
-  mVideoRequestPending = false;
-  switch (mState) {
-    case DECODER_STATE_DECODING_METADATA: {
-      VideoQueue().Finish();
-      MaybeFinishDecodeMetadata();
-      return;
-    }
-
-    case DECODER_STATE_BUFFERING:
-    case DECODER_STATE_DECODING: {
-      VideoQueue().Finish();
-      CheckIfDecodeComplete();
-      SendStreamData();
-      // The ready state can change when we've decoded data, so update the
-      // ready state, so that DOM events can fire.
-      UpdateReadyState();
-      mDecoder->GetReentrantMonitor().NotifyAll();
-      return;
-    }
-    case DECODER_STATE_SEEKING: {
-      if (!mCurrentSeekTarget.IsValid()) {
-        // We've received a sample from a previous decode. Discard it.
-        return;
-      }
-      // Null sample. Hit end of stream. If we have decoded a frame,
-      // insert it into the queue so that we have something to display.
-      if (mFirstVideoFrameAfterSeek) {
-        VideoQueue().Push(mFirstVideoFrameAfterSeek.forget());
-      }
-      VideoQueue().Finish();
-      mDropVideoUntilNextDiscontinuity = false;
-      CheckIfSeekComplete();
-      return;
-    }
-    default: {
-      // Ignore other cases.
-      return;
-    }
-  }
-}
-
 void
 MediaDecoderStateMachine::OnVideoDecoded(VideoData* aVideoSample)
 {
@@ -3185,10 +3153,10 @@ void MediaDecoderStateMachine::OnAudioSinkError()
   // Otherwise notify media decoder/element about this error for it makes
   // no sense to play an audio-only file without sound output.
   RefPtr<nsIRunnable> task(
-    NS_NewRunnableMethod(this, &MediaDecoderStateMachine::OnDecodeError));
+    NS_NewRunnableMethod(this, &MediaDecoderStateMachine::AcquireMonitorAndInvokeDecodeError));
   nsresult rv = mDecodeTaskQueue->Dispatch(task);
   if (NS_FAILED(rv)) {
-    DECODER_WARN("Failed to dispatch OnDecodeError");
+    DECODER_WARN("Failed to dispatch AcquireMonitorAndInvokeDecodeError");
   }
 }
 

dom/media/MediaDecoderStateMachine.h

@@ -344,11 +344,7 @@ public:
   void OnNotDecoded(MediaData::Type aType, RequestSampleCallback::NotDecodedReason aReason);
 
 private:
-  // XXXbholley - We leave these as helper functions to keep this patch small.
-  // They go away in the next patch.
-  void OnAudioEOS();
-  void OnVideoEOS();
-  void OnDecodeError();
+  void AcquireMonitorAndInvokeDecodeError();
 
 protected:
   virtual ~MediaDecoderStateMachine();