Remove dead code paths around PIXEL_STORAGE_TEXTURE in capture pipeline.

content/browser/renderer_host/media/video_capture_buffer_pool_unittest.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.
 
 // Unit test for VideoCaptureBufferPool.
 
 #include "content/browser/renderer_host/media/video_capture_buffer_pool.h"
 
 #include "base/bind.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop/message_loop.h"
 #include "cc/test/test_context_provider.h"
 #include "cc/test/test_web_graphics_context_3d.h"

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

Probably remove these two includes.

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

They seem to be used for testing GMBs (see lines 145-152).

 #include "content/browser/compositor/buffer_queue.h"
 #include "content/browser/gpu/browser_gpu_memory_buffer_manager.h"
 #include "content/browser/renderer_host/media/video_capture_controller.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"

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

Probably unused too.

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

Done.

 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 
 struct PixelFormatAndStorage {
   media::VideoPixelFormat pixel_format;
   media::VideoPixelStorage pixel_storage;
 };
 
 static const PixelFormatAndStorage kCapturePixelFormatAndStorages[] = {
     {media::PIXEL_FORMAT_I420, media::PIXEL_STORAGE_CPU},
     {media::PIXEL_FORMAT_ARGB, media::PIXEL_STORAGE_CPU},

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

At the level of VCBP,  CPU storage should support all formats,
whereas GMB only is intended for I420. ARGB was explicited
here for Texture. I wonder if we could reflect that in these test
parameters?  Either that, or add provisions in 
VCBP::SharedMemTracker::Init() to limit it to I420.

Unclear to me, since now we support more than what we use
(VideoCaptureDeviceClient should only use I420 now).

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

Good questions.  A few points:

1. There's nothing extra needed for the buffer pool to support ANY pixel format
(for the SHMEM case).  So, it's probably too pedantic to purposely exclude any.

2. Code that uses the buffer pool already stringently checks for I420 format, so
there's no risk of unintended use.

3. We should consider whether I420 should be the only supported format.  I know
of efforts where people would, for example, love to have I444 support to
eliminate color bleeding problems for certain applications.

So, I'll leave the ARGB+STORAGE_CPU case in-place for these tests for now.

-    {media::PIXEL_FORMAT_ARGB, media::PIXEL_STORAGE_TEXTURE},
 #if !defined(OS_ANDROID)
     {media::PIXEL_FORMAT_I420,
      media::PIXEL_STORAGE_GPUMEMORYBUFFER},
 #endif
 };
 
 static const int kTestBufferPoolSize = 3;
 
 class VideoCaptureBufferPoolTest
     : public testing::TestWithParam<PixelFormatAndStorage> {
@@ skipping 174 matching lines @@
   ASSERT_NE(nullptr, buffer3.get());
   ASSERT_EQ(3.0 / kTestBufferPoolSize, pool_->GetBufferPoolUtilization());
 
   // GMB backed frames have their platform and format specific allocations.
   if (GetParam().pixel_storage != media::PIXEL_STORAGE_GPUMEMORYBUFFER) {
     ASSERT_LE(format_lo.ImageAllocationSize(), buffer1->mapped_size());
     ASSERT_LE(format_lo.ImageAllocationSize(), buffer2->mapped_size());
     ASSERT_LE(format_lo.ImageAllocationSize(), buffer3->mapped_size());
   }
 
-  // Texture backed Frames cannot be manipulated via mapping.
-  if (GetParam().pixel_storage != media::PIXEL_STORAGE_TEXTURE) {
-    ASSERT_NE(nullptr, buffer1->data());
-    ASSERT_NE(nullptr, buffer2->data());
-    ASSERT_NE(nullptr, buffer3->data());
-  }
+  ASSERT_NE(nullptr, buffer1->data());
+  ASSERT_NE(nullptr, buffer2->data());
+  ASSERT_NE(nullptr, buffer3->data());
+
   // Touch the memory.
   if (buffer1->data() != nullptr)
     memset(buffer1->data(), 0x11, buffer1->mapped_size());
   if (buffer2->data() != nullptr)
     memset(buffer2->data(), 0x44, buffer2->mapped_size());
   if (buffer3->data() != nullptr)
     memset(buffer3->data(), 0x77, buffer3->mapped_size());
 
   // Fourth buffer should fail.  Buffer pool utilization should be at 100%.
   ASSERT_FALSE(ReserveBuffer(size_lo, GetParam())) << "Pool should be empty";
@@ skipping 120 matching lines @@
   if (buffer4->data() != nullptr)
     memset(buffer4->data(), 0x77, buffer4->mapped_size());
   buffer4.reset();
 }
 
 INSTANTIATE_TEST_CASE_P(,
                         VideoCaptureBufferPoolTest,
                         testing::ValuesIn(kCapturePixelFormatAndStorages));
 
 } // namespace content