Introduce a client minimum picture pool size

ppapi/tests/test_video_decoder.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 "ppapi/tests/test_video_decoder.h"
 
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/cpp/video_decoder.h"
 #include "ppapi/lib/gl/gles2/gl2ext_ppapi.h"
 #include "ppapi/tests/testing_instance.h"
 
+const uint32_t kMaxRequestedPictureCount = 100;

[bbudge, 2015/08/12 17:47:09]

This could break if the constant ever changes in the resource. Why not make it
much bigger?

[lpique, 2015/08/12 22:48:54]

I wrote the tests to test the boundary case. Allocating the maximum allowed
should be possible, and allocating one more should fail. It seems like a
reasonable test to have.

What I would have like to have done was include
ppapi/proxy/video-decoder_resource.h, and use the "kMaximumPictureCount"
constant that was defined there. Unfortunately I ran into issues compiling the
code (the included headers required C++11 support when the tests are built
without the flag enabling it).

I considered defining an enum constant in the .idl, but that didn't seem like
the right thing to do. I also didn't observe any other place in the ppapi tree
where I could add a quick internal implementation constant/header file to be
shared between implementation and test.

The only other place that made sense was to add it to media/base/limits.h. But
then its a maximum that needs to be enforced in a few other places in the code,
and not just in the PPAPI host implementation. Also what would be the difference
between the new constant and the existing kMaxVideoFrames constant there?

For now I've left the value, but changed the name, and added comments both here
and in video_decoder_resource.cc that the constants should match. If someone
ignores the comment and changes one and not the other, the test will fail.

I'm open to suggestions for doing it in some other way.

[bbudge, 2015/08/12 23:39:22]

A comment to connect the two is fine. Thanks.

+
 REGISTER_TEST_CASE(VideoDecoder);
 
 bool TestVideoDecoder::Init() {
-  video_decoder_interface_ = static_cast<const PPB_VideoDecoder_0_1*>(
-      pp::Module::Get()->GetBrowserInterface(PPB_VIDEODECODER_INTERFACE_0_1));
   const int width = 16;
   const int height = 16;
   const int32_t attribs[] = {PP_GRAPHICS3DATTRIB_WIDTH, width,
                              PP_GRAPHICS3DATTRIB_HEIGHT, height,
                              PP_GRAPHICS3DATTRIB_NONE};
   graphics_3d_ = pp::Graphics3D(instance_, attribs);
-  return video_decoder_interface_ && CheckTestingInterface();
+  return (pp::Module::Get()->GetBrowserInterface(PPB_VIDEODECODER_INTERFACE) &&
+          CheckTestingInterface());
 }
 
 void TestVideoDecoder::RunTests(const std::string& filter) {
   RUN_CALLBACK_TEST(TestVideoDecoder, Create, filter);
 }
 
 std::string TestVideoDecoder::TestCreate() {
   // Test that Initialize fails with a bad Graphics3D resource.
   {
     pp::VideoDecoder video_decoder(instance_);
     ASSERT_FALSE(video_decoder.is_null());
 
     TestCompletionCallback callback(instance_->pp_instance(), callback_type());
     pp::Graphics3D null_graphics_3d;
     callback.WaitForResult(
         video_decoder.Initialize(null_graphics_3d,
                                  PP_VIDEOPROFILE_VP8_ANY,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_ERROR_BADRESOURCE, callback.result());
   }
   // Test that Initialize fails with a bad profile enum value.
   {
     pp::VideoDecoder video_decoder(instance_);
     TestCompletionCallback callback(instance_->pp_instance(), callback_type());
     const PP_VideoProfile kInvalidProfile = static_cast<PP_VideoProfile>(-1);
     callback.WaitForResult(
         video_decoder.Initialize(graphics_3d_,
                                  kInvalidProfile,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_ERROR_BADARGUMENT, callback.result());
   }
   // Test that Initialize succeeds if we can create a Graphics3D resources and
   // if we allow software fallback to VP8, which should always be supported.
   if (!graphics_3d_.is_null()) {
     pp::VideoDecoder video_decoder(instance_);
     TestCompletionCallback callback(instance_->pp_instance(), callback_type());
     callback.WaitForResult(
         video_decoder.Initialize(graphics_3d_,
                                  PP_VIDEOPROFILE_VP8_ANY,
                                  PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 0,
                                  callback.GetCallback()));
     ASSERT_EQ(PP_OK, callback.result());
   }
+  // Test that Initialize succeeds with a larger than normal number of requested
+  // picture buffers, if we can create a Graphics3D resources and if we allow

[bbudge, 2015/08/12 17:47:09]

nit s/resources/resource

[lpique, 2015/08/12 22:48:54]

Done.

+  // software fallback to VP8, which should always be supported.
+  if (!graphics_3d_.is_null()) {
+    pp::VideoDecoder video_decoder(instance_);
+    TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+    callback.WaitForResult(
+        video_decoder.Initialize(graphics_3d_,
+                                 PP_VIDEOPROFILE_VP8_ANY,
+                                 PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 kMaxRequestedPictureCount,
+                                 callback.GetCallback()));
+    ASSERT_EQ(PP_OK, callback.result());
+  }
+  // Test that Initialize fails if we request an unreasonable number of picture
+  // buffers.
+  if (!graphics_3d_.is_null()) {
+    pp::VideoDecoder video_decoder(instance_);
+    TestCompletionCallback callback(instance_->pp_instance(), callback_type());
+    callback.WaitForResult(
+        video_decoder.Initialize(graphics_3d_,
+                                 PP_VIDEOPROFILE_VP8_ANY,
+                                 PP_HARDWAREACCELERATION_WITHFALLBACK,
+                                 kMaxRequestedPictureCount + 1,
+                                 callback.GetCallback()));
+    ASSERT_EQ(PP_ERROR_BADARGUMENT, callback.result());
+  }
 
   PASS();
 }