Allow multiple concurrent Decode() requests in VideoDecoder interface.

media/filters/gpu_video_decoder.cc

Index: media/filters/gpu_video_decoder.cc
diff --git a/media/filters/gpu_video_decoder.cc b/media/filters/gpu_video_decoder.cc
index 1ce5ba0fbaba5e5929fd1642f13fb438f0681e31..5cadaab794645228d2d870bef9c474944dd0d9a5 100644
--- a/media/filters/gpu_video_decoder.cc
+++ b/media/filters/gpu_video_decoder.cc
@@ -80,23 +80,17 @@ void GpuVideoDecoder::Reset(const base::Closure& closure)  {
         FROM_HERE,
         base::Bind(
             &GpuVideoDecoder::Reset, weak_factory_.GetWeakPtr(), closure));
-    // NOTE: if we're deferring Reset() until a Flush() completes, return
-    // queued pictures to the VDA so they can be used to finish that Flush().
-    if (pending_decode_cb_.is_null())
-      ready_video_frames_.clear();
     return;
   }
 
-  // Throw away any already-decoded, not-yet-delivered frames.
-  ready_video_frames_.clear();
-
   if (!vda_) {
     base::MessageLoop::current()->PostTask(FROM_HERE, closure);
     return;
   }
 
-  if (!pending_decode_cb_.is_null())
-    EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEOSFrame());
+  while (!pending_decode_callbacks_.empty()) {
+    DeliverFrame(VideoFrame::CreateEOSFrame());
+  }
 
   DCHECK(pending_reset_cb_.is_null());
   pending_reset_cb_ = BindToCurrentLoop(closure);
@@ -108,8 +102,9 @@ void GpuVideoDecoder::Stop() {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   if (vda_)
     DestroyVDA();
-  if (!pending_decode_cb_.is_null())
-    EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEOSFrame());
+  while (!pending_decode_callbacks_.empty()) {
+    DeliverFrame(VideoFrame::CreateEOSFrame());
+  }
   if (!pending_reset_cb_.is_null())
     base::ResetAndReturn(&pending_reset_cb_).Run();
 }
@@ -222,21 +217,14 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
                              const DecodeCB& decode_cb) {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   DCHECK(pending_reset_cb_.is_null());
-  DCHECK(pending_decode_cb_.is_null());
-
-  pending_decode_cb_ = BindToCurrentLoop(decode_cb);
 
   if (state_ == kError || !vda_) {
-    base::ResetAndReturn(&pending_decode_cb_).Run(kDecodeError, NULL);
+    decode_cb.Run(kDecodeError, NULL);
     return;
   }
 
   switch (state_) {
     case kDecoderDrained:
-      if (!ready_video_frames_.empty()) {
-        EnqueueFrameAndTriggerFrameDelivery(NULL);
-        return;
-      }
       state_ = kNormal;
       // Fall-through.
     case kNormal:
@@ -254,12 +242,10 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   if (buffer->end_of_stream()) {
     if (state_ == kNormal) {
       state_ = kDrainingDecoder;
+      pending_decode_callbacks_.push_back(BindToCurrentLoop(decode_cb));
+      DCHECK_LE(static_cast<int>(pending_decode_callbacks_.size()),
+                kMaxInFlightDecodes);
       vda_->Flush();
-      // If we have ready frames, go ahead and process them to ensure that the
-      // Flush operation does not block in the VDA due to lack of picture
-      // buffers.
-      if (!ready_video_frames_.empty())
-        EnqueueFrameAndTriggerFrameDelivery(NULL);
     }
     return;
   }
@@ -267,10 +253,14 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   size_t size = buffer->data_size();
   SHMBuffer* shm_buffer = GetSHM(size);
   if (!shm_buffer) {
-    base::ResetAndReturn(&pending_decode_cb_).Run(kDecodeError, NULL);
+    decode_cb.Run(kDecodeError, NULL);
     return;
   }
 
+  pending_decode_callbacks_.push_back(BindToCurrentLoop(decode_cb));
+  DCHECK_LE(static_cast<int>(pending_decode_callbacks_.size()),
+            kMaxInFlightDecodes);
+
   memcpy(shm_buffer->shm->memory(), buffer->data(), size);
   BitstreamBuffer bitstream_buffer(
       next_bitstream_buffer_id_, shm_buffer->shm->handle(), size);
@@ -282,18 +272,6 @@ void GpuVideoDecoder::Decode(const scoped_refptr<DecoderBuffer>& buffer,
   RecordBufferData(bitstream_buffer, *buffer.get());
 
   vda_->Decode(bitstream_buffer);
-
-  if (!ready_video_frames_.empty()) {
-    EnqueueFrameAndTriggerFrameDelivery(NULL);
-    return;
-  }
-
-  if (CanMoreDecodeWorkBeDone())
-    base::ResetAndReturn(&pending_decode_cb_).Run(kNotEnoughData, NULL);
-}
-
-bool GpuVideoDecoder::CanMoreDecodeWorkBeDone() {
-  return bitstream_buffers_in_decoder_.size() < kMaxInFlightDecodes;
 }
 
 void GpuVideoDecoder::RecordBufferData(const BitstreamBuffer& bitstream_buffer,
@@ -342,7 +320,11 @@ bool GpuVideoDecoder::CanReadWithoutStalling() const {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   return
       next_picture_buffer_id_ == 0 ||  // Decode() will ProvidePictureBuffers().
-      available_pictures_ > 0 || !ready_video_frames_.empty();
+      available_pictures_ > 0;
+}
+
+int GpuVideoDecoder::GetMaxDecodeRequests() const {
+  return kMaxInFlightDecodes;

[xhwang, 2014/04/17 01:06:47]

[not related to this CL]

fischman: Do you know what's the decoding delay of VDAs? Are they guaranteed to
be < kMaxInFlightDecodes? Up until this CL, we always schedule
kMaxInFlightDecodes decodes upon the first DS::Read() call. This seems to work.
But if we have a VDA that has decoding delay > kMaxInFlightDecodes, then we
won't get any decoded frames after kMaxInFlightDecodes decode calls, and DS will
not issue new decode calls, hence a deadlock.

[Sergey Ulanov, 2014/04/23 02:44:21]

per e-mail thread VDA decoder shouldn't have any delay.

 }
 
 void GpuVideoDecoder::NotifyInitializeDone() {
@@ -479,10 +461,10 @@ void GpuVideoDecoder::PictureReady(const media::Picture& picture) {
                                              pb.texture_id())).second;
   DCHECK(inserted);
 
-  EnqueueFrameAndTriggerFrameDelivery(frame);
+  DeliverFrame(frame);
 }
 
-void GpuVideoDecoder::EnqueueFrameAndTriggerFrameDelivery(
+void GpuVideoDecoder::DeliverFrame(
     const scoped_refptr<VideoFrame>& frame) {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
 
@@ -491,17 +473,12 @@ void GpuVideoDecoder::EnqueueFrameAndTriggerFrameDelivery(
   if (!pending_reset_cb_.is_null())
     return;
 
-  if (frame.get())
-    ready_video_frames_.push_back(frame);
-  else
-    DCHECK(!ready_video_frames_.empty());
+  CHECK(!pending_decode_callbacks_.empty());

[xhwang, 2014/04/17 01:06:47]

This is catching a coding error, use DCHECK?

[Sergey Ulanov, 2014/04/23 02:44:21]

Done.

 
-  if (pending_decode_cb_.is_null())
-    return;
+  DecodeCB decode_cb = pending_decode_callbacks_.front();
+  pending_decode_callbacks_.pop_front();
 
-  base::ResetAndReturn(&pending_decode_cb_)
-      .Run(kOk, ready_video_frames_.front());
-  ready_video_frames_.pop_front();
+  decode_cb.Run(kOk, frame);
 }
 
 // static
@@ -582,18 +559,13 @@ void GpuVideoDecoder::NotifyEndOfBitstreamBuffer(int32 id) {
 
   PutSHM(it->second.shm_buffer);
   bitstream_buffers_in_decoder_.erase(it);
-
-  if (pending_reset_cb_.is_null() && state_ != kDrainingDecoder &&
-      CanMoreDecodeWorkBeDone() && !pending_decode_cb_.is_null()) {
-    base::ResetAndReturn(&pending_decode_cb_).Run(kNotEnoughData, NULL);
-  }

[xhwang, 2014/04/17 01:06:47]

Kudos for removing this logic!

 }
 
 GpuVideoDecoder::~GpuVideoDecoder() {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   // Stop should have been already called.
   DCHECK(!vda_.get() && assigned_picture_buffers_.empty());
-  DCHECK(pending_decode_cb_.is_null());
+  DCHECK(pending_decode_callbacks_.empty());
   for (size_t i = 0; i < available_shm_segments_.size(); ++i) {
     available_shm_segments_[i]->shm->Close();
     delete available_shm_segments_[i];
@@ -612,13 +584,13 @@ void GpuVideoDecoder::NotifyFlushDone() {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   DCHECK_EQ(state_, kDrainingDecoder);
   state_ = kDecoderDrained;
-  EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEOSFrame());
+  DeliverFrame(VideoFrame::CreateEOSFrame());

[xhwang, 2014/04/17 01:06:47]

I suspect that after FlushDone, we'll still have unsatisfied decode_cb's. (See
comments above about decoding delay.)

[Sergey Ulanov, 2014/04/23 02:44:21]

Added DCHECK_EQ(pending_decode_callbacks_.size(), 1U);

 }
 
 void GpuVideoDecoder::NotifyResetDone() {
   DVLOG(3) << "NotifyResetDone()";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
-  DCHECK(ready_video_frames_.empty());
+  DCHECK(pending_decode_callbacks_.empty());
 
   // This needs to happen after the Reset() on vda_ is done to ensure pictures
   // delivered during the reset can find their time data.
@@ -626,9 +598,6 @@ void GpuVideoDecoder::NotifyResetDone() {
 
   if (!pending_reset_cb_.is_null())
     base::ResetAndReturn(&pending_reset_cb_).Run();
-
-  if (!pending_decode_cb_.is_null())
-    EnqueueFrameAndTriggerFrameDelivery(VideoFrame::CreateEOSFrame());
 }
 
 void GpuVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
@@ -641,9 +610,9 @@ void GpuVideoDecoder::NotifyError(media::VideoDecodeAccelerator::Error error) {
 
   state_ = kError;
 
-  if (!pending_decode_cb_.is_null()) {
-    base::ResetAndReturn(&pending_decode_cb_).Run(kDecodeError, NULL);
-    return;
+  while (!pending_decode_callbacks_.empty()) {
+    pending_decode_callbacks_.front().Run(kDecodeError, NULL);
+    pending_decode_callbacks_.pop_front();
   }
 }