Don't require VDAs to return all PictureBuffers at once.

media/filters/gpu_video_decoder.cc

 // Copyright (c) 2012 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/filters/gpu_video_decoder.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ skipping 50 matching lines @@
 GpuVideoDecoder::BufferData::~BufferData() {}
 
 GpuVideoDecoder::GpuVideoDecoder(GpuVideoAcceleratorFactories* factories)
     : needs_bitstream_conversion_(false),
       factories_(factories),
       state_(kNormal),
       decoder_texture_target_(0),
       next_picture_buffer_id_(0),
       next_bitstream_buffer_id_(0),
       available_pictures_(0),
+      can_stall_anytime_(false),
       weak_factory_(this) {
   DCHECK(factories_);
 }
 
 void GpuVideoDecoder::Reset(const base::Closure& closure)  {
   DVLOG(3) << "Reset()";
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
 
   if (state_ == kDrainingDecoder) {
     base::MessageLoop::current()->PostTask(
@@ skipping 66 matching lines @@
            << config.AsHumanReadableString();
 
   // TODO(posciak): destroy and create a new VDA on codec/profile change
   // (http://crbug.com/260224).
   if (previously_initialized && (config_.profile() != config.profile())) {
     DVLOG(1) << "Codec or profile changed, cannot reinitialize.";
     bound_init_cb.Run(false);
     return;
   }
 
-  if (!IsProfileSupported(config.profile(), config.coded_size())) {
+  VideoDecodeAccelerator::Capabilities capabilities =
+      factories_->GetVideoDecodeAcceleratorCapabilities();
+  if (!IsProfileSupported(capabilities, config.profile(),
+                          config.coded_size())) {
     DVLOG(1) << "Profile " << config.profile() << " or coded size "
              << config.coded_size().ToString() << " not supported.";
     bound_init_cb.Run(false);
     return;
   }
 
   config_ = config;
+  can_stall_anytime_ = capabilities.flags &
+                       VideoDecodeAccelerator::Capabilities::CAN_STALL_ANY_TIME;
   needs_bitstream_conversion_ = (config.codec() == kCodecH264);
   output_cb_ = BindToCurrentLoop(output_cb);
 
   if (previously_initialized) {
     // Reinitialization with a different config (but same codec and profile).
     // VDA should handle it by detecting this in-stream by itself,
     // no need to notify it.
     bound_init_cb.Run(true);
     return;
   }
@@ skipping 169 matching lines @@
   NOTREACHED() << "Missing bitstreambuffer id: " << id;
 }
 
 bool GpuVideoDecoder::NeedsBitstreamConversion() const {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   return needs_bitstream_conversion_;
 }
 
 bool GpuVideoDecoder::CanReadWithoutStalling() const {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
-  return
-      next_picture_buffer_id_ == 0 ||  // Decode() will ProvidePictureBuffers().
-      available_pictures_ > 0;
+  return next_picture_buffer_id_ ==
+             0 ||  // Decode() will ProvidePictureBuffers().
+         (!can_stall_anytime_ && available_pictures_ > 0);

[Pawel Osciak, 2015/12/05 00:18:55]

The indents here seem wrong...

[liberato (no reviews please), 2015/12/07 19:04:39]

not sure, but it's what cl format does.

[Pawel Osciak, 2015/12/09 01:31:48]

Maybe it gets confused by the inline comment? I think this warrants a more
detailed comment than the existing one anyway though, so perhaps we could have a
comment explaining the whole condition above the return please?

 }
 
 int GpuVideoDecoder::GetMaxDecodeRequests() const {
   return kMaxInFlightDecodes;
 }
 
 void GpuVideoDecoder::ProvidePictureBuffers(uint32 count,
                                             const gfx::Size& size,
                                             uint32 texture_target) {
   DVLOG(3) << "ProvidePictureBuffers(" << count << ", "
@@ skipping 266 matching lines @@
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
   if (!vda_)
     return;
 
   state_ = kError;
 
   DLOG(ERROR) << "VDA Error: " << error;
   DestroyVDA();
 }
 
-bool GpuVideoDecoder::IsProfileSupported(VideoCodecProfile profile,
-                                         const gfx::Size& coded_size) {
+bool GpuVideoDecoder::IsProfileSupported(
+    const VideoDecodeAccelerator::Capabilities& capabilities,
+    VideoCodecProfile profile,
+    const gfx::Size& coded_size) {
   DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent();
-  VideoDecodeAccelerator::SupportedProfiles supported_profiles =
-      factories_->GetVideoDecodeAcceleratorSupportedProfiles();
+  const VideoDecodeAccelerator::SupportedProfiles& supported_profiles =
+      capabilities.supported_profiles;
   for (const auto& supported_profile : supported_profiles) {

[Pawel Osciak, 2015/12/05 00:18:55]

Nit: Personally I'd just s/supported_profiles/capabilities.supported_profiles/
and remove l.654-655, but up to you.

[liberato (no reviews please), 2015/12/07 19:04:39]

Done.

     if (profile == supported_profile.profile) {
       return IsCodedSizeSupported(coded_size,
                                   supported_profile.min_resolution,
                                   supported_profile.max_resolution);
     }
   }
   return false;
 }
 
 void GpuVideoDecoder::DCheckGpuVideoAcceleratorFactoriesTaskRunnerIsCurrent()
     const {
   DCHECK(factories_->GetTaskRunner()->BelongsToCurrentThread());
 }
 
 }  // namespace media