Remove dead code paths around PIXEL_STORAGE_TEXTURE in capture pipeline.

media/capture/content/thread_safe_capture_oracle.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 "media/capture/content/thread_safe_capture_oracle.h"
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 23 matching lines @@
     const VideoCaptureParams& params,
     bool enable_auto_throttling)
     : client_(client.Pass()),
       oracle_(base::TimeDelta::FromMicroseconds(static_cast<int64>(
                   1000000.0 / params.requested_format.frame_rate +
                   0.5 /* to round to nearest int */)),
               params.requested_format.frame_size,
               params.resolution_change_policy,
               enable_auto_throttling),
       params_(params) {
 }

[mcasas, 2015/11/13 19:13:57]

What about 

  DCHECK_EQ(PIXEL_FORMAT_I420, params_.requested_format.pixel_format);

I know this is enforced at least in screen_capture_device_core.cc
but well...

[miu, 2015/11/14 03:43:48]

Nothing operates on the pixels here, so I'd prefer to just plumb-through the
configured params_.requested_format.pixel_format.  In the future, both upstream
or downstream code may include more than just I420 support.  See my other
comment for details.

 
 ThreadSafeCaptureOracle::~ThreadSafeCaptureOracle() {
 }
 
 bool ThreadSafeCaptureOracle::ObserveEventAndDecideCapture(
     VideoCaptureOracle::Event event,
     const gfx::Rect& damage_rect,
     base::TimeTicks event_time,
     scoped_refptr<VideoFrame>* storage,
     CaptureFrameCallback* callback) {
   // Grab the current time before waiting to acquire the |lock_|.
   const base::TimeTicks capture_begin_time = base::TimeTicks::Now();
 
   base::AutoLock guard(lock_);
 
   if (!client_)
     return false;  // Capture is stopped.
 
   const bool should_capture =
       oracle_.ObserveEventAndDecideCapture(event, damage_rect, event_time);
   const gfx::Size visible_size = oracle_.capture_size();
   // Always round up the coded size to multiple of 16 pixels.
   // See http://crbug.com/402151.
   const gfx::Size coded_size((visible_size.width() + 15) & ~15,
                              (visible_size.height() + 15) & ~15);

[mcasas, 2015/11/13 19:13:57]

Suggest replacing these interesting bit operations with
#include "base/bits.h"
inline size_t Align(size_t size, size_t alignment);

so the line would read:

  const gfx::Size coded_size(base::Align(visible_size.width(), 16),
                             base::Align(visible_size.height(), 16));

[miu, 2015/11/14 03:43:48]

Done.  Also, re-opened a new bug since the original one was fixed and verified
(but the solution was a hack that should have been revisited).

 
   scoped_ptr<media::VideoCaptureDevice::Client::Buffer> output_buffer(
       client_->ReserveOutputBuffer(coded_size,
-                                   (params_.requested_format.pixel_storage !=
-                                    media::PIXEL_STORAGE_TEXTURE)
-                                       ? media::PIXEL_FORMAT_I420
-                                       : media::PIXEL_FORMAT_ARGB,
+                                   params_.requested_format.pixel_format,
                                    params_.requested_format.pixel_storage));
   // Get the current buffer pool utilization and attenuate it: The utilization
   // reported to the oracle is in terms of a maximum sustainable amount (not the
   // absolute maximum).
   const double attenuated_utilization =
       client_->GetBufferPoolUtilization() *
       (100.0 / kTargetMaxPoolUtilizationPercent);
 
   const char* event_name =
       (event == VideoCaptureOracle::kTimerPoll
@@ skipping 19 matching lines @@
   } else if (!should_capture && !output_buffer.get()) {
     // We decided not to capture, but we wouldn't have been able to if we wanted
     // to because no output buffer was available.
     TRACE_EVENT_INSTANT1("gpu.capture", "NearlyPipelineLimited",
                          TRACE_EVENT_SCOPE_THREAD, "trigger", event_name);
     return false;
   }
   const int frame_number = oracle_.RecordCapture(attenuated_utilization);
   TRACE_EVENT_ASYNC_BEGIN2("gpu.capture", "Capture", output_buffer.get(),
                            "frame_number", frame_number, "trigger", event_name);
-  // Texture frames wrap a texture mailbox, which we don't have at the moment.
-  // We do not construct those frames.
-  if (params_.requested_format.pixel_storage != media::PIXEL_STORAGE_TEXTURE) {
-    *storage = VideoFrame::WrapExternalData(
-        media::PIXEL_FORMAT_I420, coded_size, gfx::Rect(visible_size),
-        visible_size, static_cast<uint8*>(output_buffer->data()),
-        output_buffer->mapped_size(), base::TimeDelta());
-    DCHECK(*storage);
-  }
+  *storage = VideoFrame::WrapExternalData(
+      params_.requested_format.pixel_format, coded_size,
+      gfx::Rect(visible_size), visible_size,
+      static_cast<uint8*>(output_buffer->data()), output_buffer->mapped_size(),
+      base::TimeDelta());
+  DCHECK(*storage);
   *callback =
       base::Bind(&ThreadSafeCaptureOracle::DidCaptureFrame, this, frame_number,
                  base::Passed(&output_buffer), capture_begin_time,
                  oracle_.estimated_frame_duration());
   return true;
 }
 
 gfx::Size ThreadSafeCaptureOracle::GetCaptureSize() const {
   base::AutoLock guard(lock_);
   return oracle_.capture_size();
@@ skipping 61 matching lines @@
   // destructor.  |metadata| is still valid for read-access at this point.
   double utilization = -1.0;
   if (metadata->GetDouble(media::VideoFrameMetadata::RESOURCE_UTILIZATION,
                           &utilization)) {
     base::AutoLock guard(lock_);
     oracle_.RecordConsumerFeedback(frame_number, utilization);
   }
 }
 
 }  // namespace media