Moving the validation of bitstream_buffer into VDA implementations.

content/common/gpu/media/android_video_decode_accelerator.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 "content/common/gpu/media/android_video_decode_accelerator.h"
 
 #include <stddef.h>
 
 #include "base/android/build_info.h"
 #include "base/auto_reset.h"
@@ skipping 600 matching lines @@
   const bool allow_overlay = strategy_->ArePicturesOverlayable();
   base::MessageLoop::current()->PostTask(
       FROM_HERE, base::Bind(&AndroidVideoDecodeAccelerator::NotifyPictureReady,
                             weak_this_factory_.GetWeakPtr(),
                             media::Picture(picture_buffer_id, bitstream_id,
                                            gfx::Rect(size_), allow_overlay)));
 }
 
 void AndroidVideoDecodeAccelerator::Decode(
     const media::BitstreamBuffer& bitstream_buffer) {
+  // We use id = -1 as a special bitstream_buffer internally, which indicating

[Pawel Osciak, 2016/02/19 07:48:44]

This feels like an opportunity to remove the check at l.637:

Decode() {
  if (id > 0 && size != 0) {
    DecodeBuffer(bitstream_buffer);
    return;
  }

  if (handle valid) {
    CloseHandle();
  }

  if (id < 0) {
    POST_ERROR
  } else {
    NEOBB()
  }
}

DecodeBuffer() {
   pending.push();
   DoIOTask();
}

and call DecodeBuffer from Flush() ?

[Owen Lin, 2016/02/23 09:06:38]

Done.

+  // flush the decoder. The client should not passing any id < 0 when call
+  // Decode().
+  if (bitstream_buffer.id() < 0) {
+    POST_ERROR(INVALID_ARGUMENT,
+               "Invalid bistream_buffer, id: " << bitstream_buffer.id());
+    if (base::SharedMemory::IsHandleValid(bitstream_buffer.handle()))
+      base::SharedMemory::CloseHandle(bitstream_buffer.handle());
+    return;
+  }
+  DecodeInternal(bitstream_buffer);
+}
+
+void AndroidVideoDecodeAccelerator::DecodeInternal(
+    const media::BitstreamBuffer& bitstream_buffer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   if (bitstream_buffer.id() != -1 && bitstream_buffer.size() == 0) {
     base::MessageLoop::current()->PostTask(
         FROM_HERE,
         base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
                    weak_this_factory_.GetWeakPtr(), bitstream_buffer.id()));

[Pawel Osciak, 2016/02/19 07:48:44]

We need to CloseHandle here as well, but please see above.

     return;
   }
 
   pending_bitstream_buffers_.push(
       std::make_pair(bitstream_buffer, base::Time::Now()));
   TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount",
                  pending_bitstream_buffers_.size());
 
   DoIOTask();
 }
@@ skipping 60 matching lines @@
   }
 
   strategy_->ReuseOnePictureBuffer(i->second);
 
   DoIOTask();
 }
 
 void AndroidVideoDecodeAccelerator::Flush() {
   DCHECK(thread_checker_.CalledOnValidThread());
 
-  Decode(media::BitstreamBuffer(-1, base::SharedMemoryHandle(), 0));
+  DecodeInternal(media::BitstreamBuffer(-1, base::SharedMemoryHandle(), 0));
 }
 
 bool AndroidVideoDecodeAccelerator::ConfigureMediaCodec() {
   DCHECK(thread_checker_.CalledOnValidThread());
   TRACE_EVENT0("media", "AVDA::ConfigureMediaCodec");
 
   jobject media_crypto = media_crypto_ ? media_crypto_->obj() : nullptr;
 
   // |needs_protected_surface_| implies encrypted stream.
   DCHECK(!needs_protected_surface_ || media_crypto);
@@ skipping 89 matching lines @@
   TRACE_EVENT0("media", "AVDA::Reset");
 
   while (!pending_bitstream_buffers_.empty()) {
     int32_t bitstream_buffer_id = pending_bitstream_buffers_.front().first.id();
     pending_bitstream_buffers_.pop();
 
     if (bitstream_buffer_id != -1) {
       base::MessageLoop::current()->PostTask(
           FROM_HERE,
           base::Bind(&AndroidVideoDecodeAccelerator::NotifyEndOfBitstreamBuffer,
                      weak_this_factory_.GetWeakPtr(), bitstream_buffer_id));

[Pawel Osciak, 2016/02/19 07:48:44]

Here we should also be closing the handle? Perhaps pending_bitstream_buffers
should just store SharedMemory to simplify things.

[Owen Lin, 2016/02/23 09:06:38]

Sounds good. But let me prepare the change after the SharedMemoryRegion is in.

     }
   }
   TRACE_COUNTER1("media", "AVDA::PendingBitstreamBufferCount", 0);
   bitstreams_notified_in_advance_.clear();
 
   // Any error that is waiting to post can be ignored.
   error_sequence_token_++;
 
   ResetCodecState();
 
@@ skipping 186 matching lines @@
 
   if (UseDeferredRenderingStrategy()) {
     capabilities.flags = media::VideoDecodeAccelerator::Capabilities::
         NEEDS_ALL_PICTURE_BUFFERS_TO_DECODE;
   }
 
   return capabilities;
 }
 
 }  // namespace content