Let only seeks reset Android MSE stream playback completion

media/base/android/media_source_player.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "media/base/android/media_source_player.h"
 
 #include <limits>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
@@ skipping 55 matching lines @@
     scoped_ptr<DemuxerAndroid> demuxer)
     : MediaPlayerAndroid(player_id, manager),
       demuxer_(demuxer.Pass()),
       pending_event_(NO_EVENT_PENDING),
       width_(0),
       height_(0),
       audio_codec_(kUnknownAudioCodec),
       video_codec_(kUnknownVideoCodec),
       num_channels_(0),
       sampling_rate_(0),
-      audio_finished_(true),
-      video_finished_(true),
+      audio_finished_(false),
+      video_finished_(false),
       playing_(false),
       is_audio_encrypted_(false),
       is_video_encrypted_(false),
       volume_(-1.0),
       clock_(&default_tick_clock_),
       next_video_data_is_iframe_(true),
       doing_browser_seek_(false),
       pending_seek_(false),
       reconfig_audio_decoder_(false),
       reconfig_video_decoder_(false),
@@ skipping 223 matching lines @@
   // Create decoder jobs if they are not created
   ConfigureAudioDecoderJob();
   ConfigureVideoDecoderJob();
 
   // If one of the decoder job is not ready, do nothing.
   if ((HasAudio() && !audio_decoder_job_) ||
       (HasVideo() && !video_decoder_job_)) {
     return;
   }
 
-  audio_finished_ = false;
-  video_finished_ = false;
   SetPendingEvent(PREFETCH_REQUEST_EVENT_PENDING);
   ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::OnDemuxerConfigsAvailable(
     const DemuxerConfigs& configs) {
   DVLOG(1) << __FUNCTION__;
   duration_ = base::TimeDelta::FromMilliseconds(configs.duration_ms);
   clock_.SetDuration(duration_);
 
@@ skipping 111 matching lines @@
     // A browser seek must not jump into the past. Ideally, it seeks to the
     // requested time, but it might jump into the future.
     DCHECK(actual_browser_seek_time >= GetCurrentTime());
     DVLOG(1) << __FUNCTION__ << " : setting clock to actual browser seek time: "
              << actual_browser_seek_time.InSecondsF();
     clock_.SetTime(actual_browser_seek_time, actual_browser_seek_time);
     if (audio_timestamp_helper_)
       audio_timestamp_helper_->SetBaseTimestamp(actual_browser_seek_time);
   }
 
+  audio_finished_ = false;
+  video_finished_ = false;
+
   base::TimeDelta current_time = GetCurrentTime();
   // TODO(qinmin): Simplify the logic by using |start_presentation_timestamp_|
   // to preroll media decoder jobs. Currently |start_presentation_timestamp_|
   // is calculated from decoder output, while preroll relies on the access
   // unit's timestamp. There are some differences between the two.
   preroll_timestamp_ = current_time;
   if (audio_decoder_job_)
     audio_decoder_job_->BeginPrerolling(preroll_timestamp_);
   if (video_decoder_job_)
     video_decoder_job_->BeginPrerolling(preroll_timestamp_);
@@ skipping 59 matching lines @@
 
     // Return early if we can't successfully configure a new video decoder job
     // yet, except continue processing other pending events if |surface_| is
     // empty.
     if (HasVideo() && !video_decoder_job_ && !surface_.IsEmpty())
       return;
   }
 
   if (IsEventPending(PREFETCH_REQUEST_EVENT_PENDING)) {
     DVLOG(1) << __FUNCTION__ << " : Handling PREFETCH_REQUEST_EVENT.";
-    int count = (audio_decoder_job_ ? 1 : 0) + (video_decoder_job_ ? 1 : 0);
+    DCHECK(audio_decoder_job_ || AudioFinishedOrNoAudio());
+    DCHECK(video_decoder_job_ || VideoFinishedOrNoVideo());
+
+    int count = ((AudioFinishedOrNoAudio()) ? 0 : 1) +
+        ((VideoFinishedOrNoVideo()) ? 0 : 1);
 
     base::Closure barrier = BarrierClosure(count, base::Bind(
         &MediaSourcePlayer::OnPrefetchDone, weak_this_.GetWeakPtr()));
 
-    if (audio_decoder_job_)
+    if (!AudioFinishedOrNoAudio())
       audio_decoder_job_->Prefetch(barrier);
 
-    if (video_decoder_job_)
+    if (!VideoFinishedOrNoVideo())
       video_decoder_job_->Prefetch(barrier);
 
     SetPendingEvent(PREFETCH_DONE_EVENT_PENDING);
     ClearPendingEvent(PREFETCH_REQUEST_EVENT_PENDING);
     return;
   }
 
   DCHECK_EQ(pending_event_, NO_EVENT_PENDING);
 
   // Now that all pending events have been handled, resume decoding if we are
@@ skipping 31 matching lines @@
   if (is_clock_manager)
     decoder_starvation_callback_.Cancel();
 
   if (status == MEDIA_CODEC_ERROR) {
     DVLOG(1) << __FUNCTION__ << " : decode error";
     Release();
     manager()->OnError(player_id(), MEDIA_ERROR_DECODE);
     return;
   }
 
+  if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM) {
+    PlaybackCompleted(is_audio);
+    return;
+  }
+
   if (pending_event_ != NO_EVENT_PENDING) {
     ProcessPendingEvents();
     return;
   }
 
-  if (status == MEDIA_CODEC_OUTPUT_END_OF_STREAM) {
-    PlaybackCompleted(is_audio);
-    return;
-  }
-
   if (status == MEDIA_CODEC_OK && is_clock_manager &&
       presentation_timestamp != kNoTimestamp()) {
     UpdateTimestamps(presentation_timestamp, audio_output_bytes);
   }
 
   if (!playing_) {
     if (is_clock_manager)
       clock_.Pause();
     return;
   }
@@ skipping 23 matching lines @@
     DecodeMoreAudio();
     return;
   }
 
   DecodeMoreVideo();
 }
 
 void MediaSourcePlayer::DecodeMoreAudio() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(!audio_decoder_job_->is_decoding());
+  DCHECK(!AudioFinishedOrNoAudio());
 
   if (audio_decoder_job_->Decode(
           start_time_ticks_, start_presentation_timestamp_, base::Bind(
               &MediaSourcePlayer::MediaDecoderCallback,
               weak_this_.GetWeakPtr(), true))) {
     TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreAudio",
                              audio_decoder_job_.get());
     return;
   }
 
   // Failed to start the next decode.
   // Wait for demuxer ready message.
   DCHECK(!reconfig_audio_decoder_);
   reconfig_audio_decoder_ = true;
 
   // Config change may have just been detected on the other stream. If so,
   // don't send a duplicate demuxer config request.
   if (IsEventPending(CONFIG_CHANGE_EVENT_PENDING)) {
     DCHECK(reconfig_video_decoder_);
     return;
   }
 
   SetPendingEvent(CONFIG_CHANGE_EVENT_PENDING);
   ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::DecodeMoreVideo() {
   DVLOG(1) << __FUNCTION__;
   DCHECK(!video_decoder_job_->is_decoding());
+  DCHECK(!VideoFinishedOrNoVideo());
 
   if (video_decoder_job_->Decode(
           start_time_ticks_, start_presentation_timestamp_, base::Bind(
               &MediaSourcePlayer::MediaDecoderCallback,
               weak_this_.GetWeakPtr(), false))) {
     TRACE_EVENT_ASYNC_BEGIN0("media", "MediaSourcePlayer::DecodeMoreVideo",
                              video_decoder_job_.get());
     return;
   }
 
@@ skipping 18 matching lines @@
   ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::PlaybackCompleted(bool is_audio) {
   DVLOG(1) << __FUNCTION__ << "(" << is_audio << ")";
   if (is_audio)
     audio_finished_ = true;
   else
     video_finished_ = true;
 
-  if ((!HasAudio() || audio_finished_) && (!HasVideo() || video_finished_)) {
+  if (AudioFinishedOrNoAudio() && VideoFinishedOrNoVideo()) {
     playing_ = false;
     clock_.Pause();
     start_time_ticks_ = base::TimeTicks();
     manager()->OnPlaybackComplete(player_id());
   }
+
+  if (pending_event_ != NO_EVENT_PENDING)

[qinmin, 2013/12/04 00:20:16]

i think you should process the pending events first.
For example, playing_ may not be changed if the pending event is seek.

That's why i think my other comments is better: rather than switch the order of
complete event and pending event, just do a check to see if the stream is
already reaching the end before sending out prefetch request.

[wolenetz, 2013/12/05 01:03:28]

From our chats/analyses, MediaDecoderCallback() can protect against completing
playback if there was a seek that became pending during EOS decode by checking
for that exact condition. All other pending events are unaffected by decode of
EOS triggering playback completion (except PREFETCH_DONE_EVENT_PENDING, which is
illegal during decode). We need to retain PREFETCH_REQUEST processing for any
remaining unfinished stream, even if one of the potentially multiple streams
decodes EOS. Patch set 5 includes these adjustments and associated new unit
tests.

We also discussed adding defense in depth by having MDJ reply with output_eos if
it has had no intervening flush since original decoding output_eos. I've added a
DCHECK+elevated output_eos_encountered to a member instead of hiding what would
be an incorrect usage of MDJ.

+    ProcessPendingEvents();
 }
 
 void MediaSourcePlayer::ClearDecodingData() {
   DVLOG(1) << __FUNCTION__;
   if (audio_decoder_job_)
     audio_decoder_job_->Flush();
   if (video_decoder_job_)
     video_decoder_job_->Flush();
   start_time_ticks_ = base::TimeTicks();
 }
 
 bool MediaSourcePlayer::HasVideo() {
   return kUnknownVideoCodec != video_codec_;
 }
 
 bool MediaSourcePlayer::HasAudio() {
   return kUnknownAudioCodec != audio_codec_;
 }
 
+bool MediaSourcePlayer::AudioFinishedOrNoAudio() {
+  return audio_finished_ || !HasAudio();
+}
+
+bool MediaSourcePlayer::VideoFinishedOrNoVideo() {
+  return video_finished_ || !HasVideo();
+}
+
 void MediaSourcePlayer::ConfigureAudioDecoderJob() {
   if (!HasAudio()) {
     audio_decoder_job_.reset();
     return;
   }
 
   // Create audio decoder job only if config changes.
   if (audio_decoder_job_ && !reconfig_audio_decoder_)
     return;
 
@@ skipping 161 matching lines @@
   if (pending_event_ != NO_EVENT_PENDING) {
     ProcessPendingEvents();
     return;
   }
 
   start_time_ticks_ = base::TimeTicks::Now();
   start_presentation_timestamp_ = GetCurrentTime();
   if (!clock_.IsPlaying())
     clock_.Play();
 
-  if (audio_decoder_job_)
+  if (!AudioFinishedOrNoAudio()) {
+    DCHECK(audio_decoder_job_);
     DecodeMoreAudio();
-  if (video_decoder_job_)
+  }
+
+  if (!VideoFinishedOrNoVideo()) {
+    DCHECK(video_decoder_job_);
     DecodeMoreVideo();
+  }
 }
 
 const char* MediaSourcePlayer::GetEventName(PendingEventFlags event) {
   static const char* kPendingEventNames[] = {
     "SEEK",
     "SURFACE_CHANGE",
     "CONFIG_CHANGE",
     "PREFETCH_REQUEST",
     "PREFETCH_DONE",
   };
@@ skipping 21 matching lines @@
 
 void MediaSourcePlayer::ClearPendingEvent(PendingEventFlags event) {
   DVLOG(1) << __FUNCTION__ << "(" << GetEventName(event) << ")";
   DCHECK_NE(event, NO_EVENT_PENDING);
   DCHECK(IsEventPending(event)) << GetEventName(event);
 
   pending_event_ &= ~event;
 }
 
 }  // namespace media