Add multiple video encoders/decoders to video encoder/decoder unittest

content/common/gpu/media/video_decode_accelerator_unittest.cc

Index: content/common/gpu/media/video_decode_accelerator_unittest.cc
diff --git a/content/common/gpu/media/video_decode_accelerator_unittest.cc b/content/common/gpu/media/video_decode_accelerator_unittest.cc
index 3e1a9fd6c32a6c0f35632e8f2b85a475c763f1ef..198b8950f0296f6b76b5e01c486814af3238a344 100644
--- a/content/common/gpu/media/video_decode_accelerator_unittest.cc
+++ b/content/common/gpu/media/video_decode_accelerator_unittest.cc
@@ -55,12 +55,16 @@
 
 #if defined(OS_WIN)
 #include "content/common/gpu/media/dxva_video_decode_accelerator.h"
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
+#elif defined(OS_CHROMEOS)
+#if defined(ARCH_CPU_ARMEL) || (defined(USE_OZONE) && defined(USE_V4L2_CODEC))
 #include "content/common/gpu/media/v4l2_video_decode_accelerator.h"
 #include "content/common/gpu/media/v4l2_video_device.h"
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
+// defined(ARCH_CPU_ARMEL) || (defined(USE_OZONE) && defined(USE_V4L2_CODEC))

[Pawel Osciak, 2015/01/09 13:09:11]

Maybe just remove this please, it's actually confusing I think.

[henryhsu, 2015/01/09 14:29:23]

Done.

+#endif
+#if defined(ARCH_CPU_X86_FAMILY)
 #include "content/common/gpu/media/vaapi_video_decode_accelerator.h"
 #include "content/common/gpu/media/vaapi_wrapper.h"
+#endif  // defined(ARCH_CPU_X86_FAMILY)
 #else
 #error The VideoAccelerator tests are not supported on this platform.
 #endif  // OS_WIN
@@ -282,6 +286,10 @@ class GLRenderingVDAClient
  private:
   typedef std::map<int32, scoped_refptr<TextureRef>> TextureRefMap;
 
+  scoped_ptr<media::VideoDecodeAccelerator> CreateDXVAVDA();

[Pawel Osciak, 2015/01/09 13:09:11]

Could these be static and maybe even outside of this class?

[henryhsu, 2015/01/09 14:29:23]

I think no. First, we have multiple instances of GLRenderingVDAClient in several
test cases. Each instance should have their own VDA. Second, static or global
variables of class type are forbidden.

+  scoped_ptr<media::VideoDecodeAccelerator> CreateV4L2VDA();
+  scoped_ptr<media::VideoDecodeAccelerator> CreateVaapiVDA();
+
   void SetState(ClientState new_state);
   void FinishInitialization();
   void ReturnPicture(int32 picture_buffer_id);
@@ -411,40 +419,75 @@ GLRenderingVDAClient::~GLRenderingVDAClient() {
 
 static bool DoNothingReturnTrue() { return true; }
 
+scoped_ptr<media::VideoDecodeAccelerator>
+GLRenderingVDAClient::CreateDXVAVDA() {
+  scoped_ptr<media::VideoDecodeAccelerator> decoder;
+#if defined(OS_WIN)
+  if (base::win::GetVersion() >= base::win::VERSION_WIN7)
+    decoder.reset(
+        new DXVAVideoDecodeAccelerator(base::Bind(&DoNothingReturnTrue)));
+#endif
+  return decoder.Pass();
+}
+
+scoped_ptr<media::VideoDecodeAccelerator>
+GLRenderingVDAClient::CreateV4L2VDA() {
+  scoped_ptr<media::VideoDecodeAccelerator> decoder;
+#if defined(OS_CHROMEOS) && (defined(ARCH_CPU_ARMEL) || \
+    (defined(USE_OZONE) && defined(USE_V4L2_CODEC)))
+  scoped_ptr<V4L2Device> device = V4L2Device::Create(V4L2Device::kDecoder);
+  if (device.get()) {
+    base::WeakPtr<VideoDecodeAccelerator::Client> weak_client = AsWeakPtr();
+    decoder.reset(new V4L2VideoDecodeAccelerator(
+        static_cast<EGLDisplay>(rendering_helper_->GetGLDisplay()),
+        static_cast<EGLContext>(rendering_helper_->GetGLContextHandle()),
+        weak_client,
+        base::Bind(&DoNothingReturnTrue),
+        device.Pass(),
+        base::MessageLoopProxy::current()));
+  }
+#endif
+  return decoder.Pass();
+}
+
+scoped_ptr<media::VideoDecodeAccelerator>
+GLRenderingVDAClient::CreateVaapiVDA() {
+  scoped_ptr<media::VideoDecodeAccelerator> decoder;
+#if defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
+  decoder.reset(
+      new VaapiVideoDecodeAccelerator(base::Bind(&DoNothingReturnTrue)));
+#endif
+  return decoder.Pass();
+}
+
 void GLRenderingVDAClient::CreateAndStartDecoder() {
   CHECK(decoder_deleted());
   CHECK(!decoder_.get());
 
   VideoDecodeAccelerator::Client* client = this;
-  base::WeakPtr<VideoDecodeAccelerator::Client> weak_client = AsWeakPtr();
-#if defined(OS_WIN)
-  decoder_.reset(
-      new DXVAVideoDecodeAccelerator(base::Bind(&DoNothingReturnTrue)));
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_ARMEL)
 
-  scoped_ptr<V4L2Device> device = V4L2Device::Create(V4L2Device::kDecoder);
-  if (!device.get()) {
-    NotifyError(media::VideoDecodeAccelerator::PLATFORM_FAILURE);
-    return;
+  scoped_ptr<media::VideoDecodeAccelerator> decoders[] = {
+    CreateDXVAVDA(),
+    CreateV4L2VDA(),
+    CreateVaapiVDA()
+  };
+  size_t num_decoders = sizeof(decoders) /

[Pawel Osciak, 2015/01/09 13:09:11]

Please use arraysize() and move to the for loop.

[henryhsu, 2015/01/09 14:29:23]

Done.

+      sizeof(scoped_ptr<media::VideoDecodeAccelerator>);
+
+  for (size_t i = 0; i < num_decoders; ++i) {
+    if (!decoders[i]) continue;

[Pawel Osciak, 2015/01/09 13:09:11]

New line please.

[henryhsu, 2015/01/09 14:29:23]

Done.

+    decoder_ = decoders[i].Pass();
+    weak_decoder_factory_.reset(
+        new base::WeakPtrFactory<VideoDecodeAccelerator>(decoder_.get()));
+    SetState(CS_DECODER_SET);
+    if (decoder_->Initialize(profile_, client)) {
+      FinishInitialization();
+      return;
+    }
   }
-  decoder_.reset(new V4L2VideoDecodeAccelerator(
-      static_cast<EGLDisplay>(rendering_helper_->GetGLDisplay()),
-      static_cast<EGLContext>(rendering_helper_->GetGLContextHandle()),
-      weak_client, base::Bind(&DoNothingReturnTrue), device.Pass(),
-      base::MessageLoopProxy::current()));
-#elif defined(OS_CHROMEOS) && defined(ARCH_CPU_X86_FAMILY)
-  decoder_.reset(
-      new VaapiVideoDecodeAccelerator(base::Bind(&DoNothingReturnTrue)));
-#endif  // OS_WIN
   CHECK(decoder_.get());

[Pawel Osciak, 2015/01/09 13:09:11]

Perhaps have decoder_.reset() at the end of each iteration and have this as the
failing check instead of these two lines below?

[henryhsu, 2015/01/09 14:29:23]

We don't have to reset decoder_ at the end of each iteration because it will be
replaced by next decoders[i].
And I think CHECK(decoder_.get()) is useless. Once this CHECK is hit, we still
don't know which decoder initial failed.
But CHECK(false) is required to raise error when all decoders cannot be
initialized.

-  weak_decoder_factory_.reset(
-      new base::WeakPtrFactory<VideoDecodeAccelerator>(decoder_.get()));
-  SetState(CS_DECODER_SET);
-  if (decoder_deleted())
-    return;
-
-  CHECK(decoder_->Initialize(profile_, client));
-  FinishInitialization();
+  // Decoders are all initialize failed.
+  CHECK(false);
 }
 
 void GLRenderingVDAClient::ProvidePictureBuffers(