Reducing the IPC latency for MSE video decoding

media/base/android/media_source_player.cc

Index: media/base/android/media_source_player.cc
diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc
index 584d50b2385167d44046b41bfb0c8b139c3279e6..f0c122df8b579f4c31809651425be1ae05e8d667 100644
--- a/media/base/android/media_source_player.cc
+++ b/media/base/android/media_source_player.cc
@@ -61,7 +61,9 @@ MediaSourcePlayer::MediaSourcePlayer(
       reconfig_video_decoder_(false),
       weak_this_(this),
       drm_bridge_(NULL),
-      is_waiting_for_key_(false) {
+      is_waiting_for_key_(false),
+      has_pending_audio_data_request_(false),
+      has_pending_video_data_request_(false) {
   demuxer_->Initialize(this);
   clock_.SetMaxTime(base::TimeDelta());
 }
@@ -222,7 +224,7 @@ void MediaSourcePlayer::Release() {
   // Clear all the pending events except seeks and config changes.
   pending_event_ &= (SEEK_EVENT_PENDING | CONFIG_CHANGE_EVENT_PENDING);
   is_surface_in_use_ = false;
-  audio_decoder_job_.reset();
+  ResetAudioDecoderJob();
   ResetVideoDecoderJob();
 
   // Prevent job re-creation attempts in OnDemuxerConfigsAvailable()
@@ -345,6 +347,20 @@ void MediaSourcePlayer::OnDemuxerConfigsAvailable(
 void MediaSourcePlayer::OnDemuxerDataAvailable(const DemuxerData& data) {
   DVLOG(1) << __FUNCTION__ << "(" << data.type << ")";
   DCHECK_LT(0u, data.access_units.size());
+
+  if (has_pending_audio_data_request_ && data.type == DemuxerStream::AUDIO) {

[wolenetz, 2014/03/17 19:51:00]

Do I understand correctly that we drop on the floor the result of the pending
read if the player has been Release()'ed and not yet restarted when the video or
audio data arrives here? Is that the right thing to do, or should we hold onto
the data to feed newly created decoder, or do we always seek to take care of
this case on restart?

[qinmin, 2014/03/18 18:58:58]

Yes, the purpose of this is to drop the request that has been made by a released
player.
We cannot pass the data to a newly created decoder, because the data might be
requested for an inactive chunk, which is not the data the previous decoder is
currently decoding. Additionally, the data may not be an iframe and we cannot
feed it to a newly created decoder.

On 2014/03/17 19:51:00, wolenetz wrote:

+    has_pending_audio_data_request_ = false;
+    ProcessPendingEvents();
+    return;
+  }
+
+  if (has_pending_video_data_request_ && data.type == DemuxerStream::VIDEO) {
+    next_video_data_is_iframe_ = false;
+    has_pending_video_data_request_ = false;
+    ProcessPendingEvents();
+    return;
+  }
+
   if (data.type == DemuxerStream::AUDIO && audio_decoder_job_) {
     audio_decoder_job_->OnDataReceived(data);
   } else if (data.type == DemuxerStream::VIDEO) {
@@ -418,15 +434,16 @@ void MediaSourcePlayer::OnDemuxerSeekDone(
   // I-frame later than the requested one due to data removal or GC. Update
   // player clock to the actual seek target.
   if (doing_browser_seek_) {
-    DCHECK(actual_browser_seek_time != kNoTimestamp());
+    base::TimeDelta seek_time = (actual_browser_seek_time == kNoTimestamp()) ?
+        base::TimeDelta() : actual_browser_seek_time;
     // 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());
+    DCHECK(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);
+             << seek_time.InSecondsF();
+    clock_.SetTime(seek_time, seek_time);
     if (audio_timestamp_helper_)
-      audio_timestamp_helper_->SetBaseTimestamp(actual_browser_seek_time);
+      audio_timestamp_helper_->SetBaseTimestamp(seek_time);
   }
 
   reached_audio_eos_ = false;
@@ -476,6 +493,11 @@ void MediaSourcePlayer::ProcessPendingEvents() {
     return;
   }
 
+  if (has_pending_audio_data_request_ || has_pending_video_data_request_) {
+    DVLOG(1) << __FUNCTION__ << " : has pending data request.";
+    return;
+  }
+
   if (IsEventPending(PREFETCH_DONE_EVENT_PENDING)) {
     DVLOG(1) << __FUNCTION__ << " : PREFETCH_DONE still pending.";
     return;
@@ -510,6 +532,14 @@ void MediaSourcePlayer::ProcessPendingEvents() {
 
   if (IsEventPending(PREFETCH_REQUEST_EVENT_PENDING)) {
     DVLOG(1) << __FUNCTION__ << " : Handling PREFETCH_REQUEST_EVENT.";
+    // If one of the decoder is not initialized, cancel this event as it will be

[wolenetz, 2014/03/17 19:51:00]

Were we hitting the DCHECK's below? In other words, is it possible for
PREFETCH_REQUEST_EVENT_PENDING to become true but the relevant decoder job(s) to
not yet be initialized?

[qinmin, 2014/03/18 18:58:58]

This is possible with the new implementation.
Considering the case that we pass an empty surface to the video decoder: In that
case, the old decoder will be destroyed while requesting some data. Since the
new surface is empty, we will not create the new video decoder. In our previous
implementation, we will early return when processing the SURFACE_CHANGE event.
Now after sometime the data requested by the old decoder arrives, and we start
to process pending events. Since SURFACE_CHANGE event has been processed, we
will hit the PREFETCH_REQUEST pending event and find that the video decoder is
actually null.

When decoder is null, there is no point to handle the prefetch request, that's
why we can clear the pending events here.

On 2014/03/17 19:51:00, wolenetz wrote:

+    // called later when Start() is called again.
+    if ((HasVideo() && !video_decoder_job_) ||
+        (HasAudio() && !audio_decoder_job_)) {
+      ClearPendingEvent(PREFETCH_REQUEST_EVENT_PENDING);
+      return;
+    }
+
     DCHECK(audio_decoder_job_ || AudioFinished());
     DCHECK(video_decoder_job_ || VideoFinished());
 
@@ -747,7 +777,7 @@ bool MediaSourcePlayer::VideoFinished() {
 
 void MediaSourcePlayer::ConfigureAudioDecoderJob() {
   if (!HasAudio()) {
-    audio_decoder_job_.reset();
+    ResetAudioDecoderJob();
     return;
   }
 
@@ -761,8 +791,8 @@ void MediaSourcePlayer::ConfigureAudioDecoderJob() {
 
   DCHECK(!audio_decoder_job_ || !audio_decoder_job_->is_decoding());
 
+  ResetAudioDecoderJob();
   DVLOG(1) << __FUNCTION__ << " : creating new audio decoder job";
-
   audio_decoder_job_.reset(AudioDecoderJob::Create(
       audio_codec_, sampling_rate_, num_channels_, &audio_extra_data_[0],
       audio_extra_data_.size(), media_crypto.obj(),
@@ -777,6 +807,10 @@ void MediaSourcePlayer::ConfigureAudioDecoderJob() {
 }
 
 void MediaSourcePlayer::ResetVideoDecoderJob() {
+  if (video_decoder_job_) {
+    has_pending_video_data_request_ =
+        video_decoder_job_->is_requesting_demuxer_data();
+  }
   video_decoder_job_.reset();
 
   // Any eventual video decoder job re-creation will use the current |surface_|.
@@ -784,6 +818,14 @@ void MediaSourcePlayer::ResetVideoDecoderJob() {
     ClearPendingEvent(SURFACE_CHANGE_EVENT_PENDING);
 }
 
+void MediaSourcePlayer::ResetAudioDecoderJob() {
+  if (audio_decoder_job_) {
+    has_pending_audio_data_request_ =
+        audio_decoder_job_->is_requesting_demuxer_data();
+  }
+  audio_decoder_job_.reset();
+}
+
 void MediaSourcePlayer::ConfigureVideoDecoderJob() {
   if (!HasVideo() || surface_.IsEmpty()) {
     ResetVideoDecoderJob();