Fix VideoDecoderShim to return correct decode_id for all codecs.

content/renderer/pepper/video_decoder_shim.cc

 // Copyright 2014 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 "content/renderer/pepper/video_decoder_shim.h"
 
 #include <GLES2/gl2.h>
 #include <GLES2/gl2ext.h>
 #include <GLES2/gl2extchromium.h>
 
@@ skipping 78 matching lines @@
   ~DecoderImpl();
 
   void Initialize(media::VideoDecoderConfig config);
   void Decode(uint32_t decode_id, scoped_refptr<media::DecoderBuffer> buffer);
   void Reset();
   void Stop();
 
  private:
   void OnPipelineStatus(media::PipelineStatus status);
   void DoDecode();
-  void OnDecodeComplete(uint32_t decode_id, media::VideoDecoder::Status status);
+  void OnDecodeComplete(media::VideoDecoder::Status status);
   void OnOutputComplete(const scoped_refptr<media::VideoFrame>& frame);
   void OnResetComplete();
 
   // WeakPtr is bound to main_message_loop_. Use only in shim callbacks.
   base::WeakPtr<VideoDecoderShim> shim_;
   scoped_ptr<media::VideoDecoder> decoder_;
   scoped_refptr<base::MessageLoopProxy> main_message_loop_;
   // Queue of decodes waiting for the decoder.
   typedef std::queue<PendingDecode> PendingDecodeQueue;
   PendingDecodeQueue pending_decodes_;
-  int max_decodes_at_decoder_;
-  int num_decodes_at_decoder_;
+  bool waiting_decoder_;

[bbudge, 2015/01/15 21:21:00]

nit: s/waiting/awaiting

[Sergey Ulanov, 2015/01/16 00:46:39]

Done.

   // VideoDecoder returns pictures without information about the decode buffer
-  // that generated it. Save the decode_id from the last decode that completed,
-  // which is close for most decoders, which only decode one buffer at a time.
+  // that generated it, but software decoder always generate corresponding

[bbudge, 2015/01/15 21:21:00]

nit: s/decoder/decoders

[Sergey Ulanov, 2015/01/16 00:46:39]

Done.

+  // frames before decode is finished.
   uint32_t decode_id_;
 };
 
 VideoDecoderShim::DecoderImpl::DecoderImpl(
     const base::WeakPtr<VideoDecoderShim>& proxy)
     : shim_(proxy),
       main_message_loop_(base::MessageLoopProxy::current()),
-      max_decodes_at_decoder_(0),
-      num_decodes_at_decoder_(0),
+      waiting_decoder_(0),

[bbudge, 2015/01/15 21:21:00]

s/0/false

[Sergey Ulanov, 2015/01/16 00:46:39]

Done.

       decode_id_(0) {
 }
 
 VideoDecoderShim::DecoderImpl::~DecoderImpl() {
   DCHECK(pending_decodes_.empty());
 }
 
 void VideoDecoderShim::DecoderImpl::Initialize(
     media::VideoDecoderConfig config) {
   DCHECK(!decoder_);
 #if !defined(MEDIA_DISABLE_LIBVPX)
   if (config.codec() == media::kCodecVP9) {
     decoder_.reset(
         new media::VpxVideoDecoder(base::MessageLoopProxy::current()));
   } else
 #endif
   {
     scoped_ptr<media::FFmpegVideoDecoder> ffmpeg_video_decoder(
         new media::FFmpegVideoDecoder(base::MessageLoopProxy::current()));
     ffmpeg_video_decoder->set_decode_nalus(true);
     decoder_ = ffmpeg_video_decoder.Pass();
   }
-  max_decodes_at_decoder_ = decoder_->GetMaxDecodeRequests();
+
+  // VpxVideoDecoder and FFmpegVideoDecoder support only one pending Decode()
+  // request.
+  DCHECK_EQ(decoder_->GetMaxDecodeRequests(), 1);
+
   // We can use base::Unretained() safely in decoder callbacks because
   // |decoder_| is owned by DecoderImpl. During Stop(), the |decoder_| will be
   // destroyed and all outstanding callbacks will be fired.
   decoder_->Initialize(
       config,
       true /* low_delay */,
       base::Bind(&VideoDecoderShim::DecoderImpl::OnPipelineStatus,
                  base::Unretained(this)),
       base::Bind(&VideoDecoderShim::DecoderImpl::OnOutputComplete,
                  base::Unretained(this)));
@@ skipping 43 matching lines @@
       break;
     case media::DECODER_ERROR_NOT_SUPPORTED:
       result = PP_ERROR_NOTSUPPORTED;
       break;
     default:
       result = PP_ERROR_FAILED;
       break;
   }
 
   // Calculate how many textures the shim should create.
-  uint32_t shim_texture_pool_size =
-      max_decodes_at_decoder_ + media::limits::kMaxVideoFrames;
+  uint32_t shim_texture_pool_size = media::limits::kMaxVideoFrames + 1;
   main_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(&VideoDecoderShim::OnInitializeComplete,
                  shim_,
                  result,
                  shim_texture_pool_size));
 }
 
 void VideoDecoderShim::DecoderImpl::DoDecode() {
-  while (!pending_decodes_.empty() &&
-         num_decodes_at_decoder_ < max_decodes_at_decoder_) {
-    num_decodes_at_decoder_++;
-    const PendingDecode& decode = pending_decodes_.front();
-    decoder_->Decode(
-        decode.buffer,
-        base::Bind(&VideoDecoderShim::DecoderImpl::OnDecodeComplete,
-                   base::Unretained(this),
-                   decode.decode_id));
-    pending_decodes_.pop();
-  }
+  if (pending_decodes_.empty() || waiting_decoder_)
+    return;
+
+  waiting_decoder_ = true;
+  const PendingDecode& decode = pending_decodes_.front();
+  decode_id_ = decode.decode_id;
+  decoder_->Decode(decode.buffer,
+                   base::Bind(&VideoDecoderShim::DecoderImpl::OnDecodeComplete,
+                              base::Unretained(this)));
+  pending_decodes_.pop();
 }
 
 void VideoDecoderShim::DecoderImpl::OnDecodeComplete(
-    uint32_t decode_id,
     media::VideoDecoder::Status status) {
-  num_decodes_at_decoder_--;
-  decode_id_ = decode_id;
+  DCHECK(waiting_decoder_);
+  waiting_decoder_ = false;
 
   int32_t result;
   switch (status) {
     case media::VideoDecoder::kOk:
     case media::VideoDecoder::kAborted:
       result = PP_OK;
       break;
     case media::VideoDecoder::kDecodeError:
       result = PP_ERROR_RESOURCE_FAILED;
       break;
     default:
       NOTREACHED();
       result = PP_ERROR_FAILED;
       break;
   }
 
   main_message_loop_->PostTask(
       FROM_HERE,
       base::Bind(
-          &VideoDecoderShim::OnDecodeComplete, shim_, result, decode_id));
+          &VideoDecoderShim::OnDecodeComplete, shim_, result, decode_id_));
 
   DoDecode();
 }
 
 void VideoDecoderShim::DecoderImpl::OnOutputComplete(
     const scoped_refptr<media::VideoFrame>& frame) {
+  // Software decoders are expected to generated frames only when Decode() call

[bbudge, 2015/01/15 21:21:00]

s/generated/generate
s/Decode()/a Decode()

[Sergey Ulanov, 2015/01/16 00:46:39]

Done.

+  // is pending.
+  DCHECK(waiting_decoder_);
+
   scoped_ptr<PendingFrame> pending_frame;
   if (!frame->end_of_stream()) {
     pending_frame.reset(new PendingFrame(
         decode_id_, frame->coded_size(), frame->visible_rect()));
     // Convert the VideoFrame pixels to ABGR to match VideoDecodeAccelerator.
     libyuv::I420ToABGR(frame->data(media::VideoFrame::kYPlane),
                        frame->stride(media::VideoFrame::kYPlane),
                        frame->data(media::VideoFrame::kUPlane),
                        frame->stride(media::VideoFrame::kUPlane),
                        frame->data(media::VideoFrame::kVPlane),
@@ skipping 313 matching lines @@
 void VideoDecoderShim::DeleteTexture(uint32_t texture_id) {
   gpu::gles2::GLES2Interface* gles2 = context_provider_->ContextGL();
   gles2->DeleteTextures(1, &texture_id);
 }
 
 void VideoDecoderShim::FlushCommandBuffer() {
   context_provider_->ContextGL()->Flush();
 }
 
 }  // namespace content