Move video encode accelerator IPC messages to GPU IO thread

media/video/video_encode_accelerator.h

Index: media/video/video_encode_accelerator.h
diff --git a/media/video/video_encode_accelerator.h b/media/video/video_encode_accelerator.h
index ac1ffd56dfa8a1f51a54c5e7d7284a7584c174fc..e822d5d9e7b654781a6bee8099f45f431d6b4f37 100644
--- a/media/video/video_encode_accelerator.h
+++ b/media/video/video_encode_accelerator.h
@@ -12,6 +12,8 @@
 #include <vector>
 
 #include "base/memory/ref_counted.h"
+#include "base/memory/weak_ptr.h"
+#include "base/single_thread_task_runner.h"
 #include "media/base/bitstream_buffer.h"
 #include "media/base/media_export.h"
 #include "media/base/video_decoder_config.h"
@@ -147,11 +149,28 @@ class MEDIA_EXPORT VideoEncodeAccelerator {
 
   // Destroys the encoder: all pending inputs and outputs are dropped
   // immediately and the component is freed.  This call may asynchronously free
-  // system resources, but its client-visible effects are synchronous.  After
-  // this method returns no more callbacks will be made on the client.  Deletes
+  // system resources, but its client-visible effects are synchronous. After
+  // this method returns no more callbacks will be made on the client. Deletes
   // |this| unconditionally, so make sure to drop all pointers to it!
   virtual void Destroy() = 0;
 
+  // Encode tasks include all calls, except Initialize() and Destroy(), that are

[Pawel Osciak, 2016/10/25 01:44:05]

Could we instead document the complete set of calls that are called on IO
instead of wording it this way please? If we say all except X() and Y(), if we
add additional methods later on, we may forget to update this doc.

Also, I think we shouldn't probably need RequireBitstreamBuffers and NotifyError
callbacks to be on IO as well?

[emircan, 2016/10/25 21:57:51]

Sgtm. I moved RequireBitstreamBuffers and NotifyError back to main and updated
the comment.

+  // used frequently during the encode session. If the Client can support
+  // running these calls on a separate thread, it may call this method to
+  // try to set up the VEA implementation to do so.
+  //
+  // If the VEA can support this as well, return true, otherwise return false.
+  // If true is returned, the client may submit each of these calls on
+  // |encode_task_runner|, and then expect Client methods to be called on
+  // |encode_task_runner| as well; called on |encode_client|, instead of
+  // |client| provided to Initialize().
+  //
+  // One application of this is offloading the GPU main thread. This helps
+  // reduce latency by avoiding the wait and reduces jitter.
+  virtual bool TryToSetupEncodeOnSeparateThread(

[Pawel Osciak, 2016/10/25 01:44:05]

Could you add this call to video_encode_accelerator_unittest as well please?

[emircan, 2016/10/25 21:57:51]

VEAUnittest is not running on Win bots right now. I will leave a TODO. I am
going to enable it once I stabilize WebRTC perf and quality browser tests, and
H264 problems that come up before[0].

[0] https://codereview.chromium.org/2205623002/#msg86

[Pawel Osciak, 2016/10/27 01:45:19]

The test is currently run on CrOS bots on CrOS devices however. Would it be
possible to spawn a dummy thread in the test with a threadchecker, and only use
it for checking the expected calls go to it and post back to main immediately
afterwards please?

[emircan, 2016/10/27 17:53:30]

I added a dummy IO thread where calls run to the test. However, it will first
call this method which returns false on CrOS VAAPI and V4L2. Therefore, all the
calls will still go through main thread, and new thread wouldn't be tested until
Win.

+      const base::WeakPtr<Client>& encode_client,
+      const scoped_refptr<base::SingleThreadTaskRunner>& encode_task_runner);
+
  protected:
   // Do not delete directly; use Destroy() or own it with a scoped_ptr, which
   // will Destroy() it properly by default.