Move video encode accelerator IPC messages to GPU IO thread

media/video/video_encode_accelerator.h

 // Copyright 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.
 
 #ifndef MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_
 #define MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #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"
 #include "media/base/video_frame.h"
 
 namespace media {
 
 class BitstreamBuffer;
 class VideoFrame;
 
@@ skipping 115 matching lines @@
   // Request a change to the encoding parameters.  This is only a request,
   // fulfilled on a best-effort basis.
   // Parameters:
   //  |bitrate| is the requested new bitrate, in bits per second.
   //  |framerate| is the requested new framerate, in frames per second.
   virtual void RequestEncodingParametersChange(uint32_t bitrate,
                                                uint32_t framerate) = 0;
 
   // 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.
   virtual ~VideoEncodeAccelerator();
 };
 
 }  // namespace media
 
 namespace std {
 
 // Specialize std::default_delete so that
 // std::unique_ptr<VideoEncodeAccelerator> uses "Destroy()" instead of trying to
 // use the destructor.
 template <>
 struct MEDIA_EXPORT default_delete<media::VideoEncodeAccelerator> {
   void operator()(media::VideoEncodeAccelerator* vea) const;
 };
 
 }  // namespace std
 
 #endif  // MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_