Add media::VideoEncodeAccelerator with WebRTC integration

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
new file mode 100644
index 0000000000000000000000000000000000000000..370e555e68e9526d920da7fae3e3b04e1644f165
--- /dev/null
+++ b/media/video/video_encode_accelerator.h
@@ -0,0 +1,136 @@
+// 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.
+
+#ifndef MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_
+#define MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_
+
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/memory/ref_counted.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;
+
+// Video encoder interface.
+class MEDIA_EXPORT VideoEncodeAccelerator {
+ public:
+  virtual ~VideoEncodeAccelerator();
+
+  // Specification of an encoding profile supported by an encoder.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

Did you mean to add a query/response API pair here and corresponding IPC
messages?

[sheu, 2013/08/03 01:31:03]

Not at the moment; this is statically queried.  (See the hullabaloo around
GpuVEA/GpuVEAHost.

+  struct SupportedProfile {
+    VideoCodecProfile profile;
+    gfx::Size max_resolution;
+    struct {
+      uint32 numerator;
+      uint32 denominator;
+    } max_framerate;
+  };
+
+  // Enumeration of potential errors generated by the API.
+  enum Error {
+    // An operation was attempted during an incompatible encoder state.
+    kIllegalStateError,
+    // Invalid argument was passed to an API method.
+    kInvalidArgumentError,
+    // A failure occurred at the browser layer or one of its dependencies.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

s/browser layer/GPU process/

[sheu, 2013/08/03 01:31:03]

Done.

+    // Examples of such failures include GPU hardware failures, GPU driver
+    // failures, GPU library failures, browser programming errors, and so on.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

s/browser/GPU process/

[sheu, 2013/08/03 01:31:03]

Done.

+    kPlatformFailureError,
+  };
+
+  // Interface for clients that use VideoEncodeAccelerator.
+  class MEDIA_EXPORT Client {
+   public:
+    // Callback to notify client that encoder has been successfully initialized.
+    virtual void NotifyInitializeDone() = 0;
+
+    // Callback to tell the client what size of buffers to provide for input and
+    // output.  The VEA disclaims use or ownership of all previously provided
+    // buffers once this callback is made.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

This seems unfortunate in that it makes it higher-latency-than-necessary to
increase the number of buffers in use.
Why not have a VEA::Client::DismissBitstreamBuffer call, instead?

[sheu, 2013/08/03 01:31:03]

VDA::Client::DismissBitstreamBuffer ended up being either ignored or just
batch-used on all buffers in our VDA implementations.

In fact, all the client needs to do is to provide at least this many outstanding
buffers -- there's no restriction on the client providing more.  So increasing
the number of buffers in use is a non-issue.

[Pawel Osciak, 2013/08/03 01:57:47]

Not ignored anymore, since we have support for MSE/EME.
WebRTC/hangout might be switching resolutions/settings as well and possibly
quite often? 

It might make sense then to request/dismiss some pictures in the working set of
VideoFrames (I'm assuming after switch to VideoFrame there will be a constant
set of pre-alloced/inited VideoFrames, but not BitstreamBuffers, like in VDA
model and VEA::NotifyInputDone() would be equivalent to a Dismiss?). That said,
this seems like something that could be added later.

On the other hand, we can't have a direct parallel to VDA here with output
BitstreamBufs, on config/knob change we might need to request larger output
buffers to be provided to us from that point?

[Ami GONE FROM CHROMIUM, 2013/08/05 18:44:38]

Pawel I think this comment was part of your confusion about our earlier
conversation.  If you have a proposal/point here still can you restate it?

[Pawel Osciak, 2013/08/06 06:47:33]

Sorry, my point was how do we want to handle change of resolution initiated by
client. This should be a common Hangouts scenario, where WebRTC switches res and
requests a new one from capturer and needs to update encoder as well. Unless we
want to have a destroy/create/reinit cycle (which I'm not strongly saying is not
ok), DPB() would be useful.

[sheu, 2013/08/07 09:25:03]

Until we have actual HW support for resolution changes, can we punt this?  At
the moment all we can do anyways is destroy/create/reinit, and that will happen
at the RTCVideoEncoder level.

+    // Parameters:
+    //  |input_count| is the number of input frames required for encoding.  The
+    //  client should provide at least this many frames, since the encoder may
+    //  need to hold onto some subset of inputs as reference pictures.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

The method name deals in bitstreambuffers but here you say frames.
I believe this comment is misguided and the param really means "number of
bitstream buffers to provide to the VEA".

[sheu, 2013/08/03 01:31:03]

moving to media::VideoFrame.

+    //  |input_dimensions| are the logical dimensions of the input frames to
+    //  encode, in pixels.  The encoder may have hardware alignment requirements
+    //  that make this different from |input_resolution|, as requested in
+    //  Initialize(), in which case the input buffer should be padded to

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

s/input buffer/input to VEA::Encode()/

[sheu, 2013/08/03 01:31:03]

Done.

+    //  |input_dimensions|.
+    //  |output_size| is the required size of output buffers for this encoder,

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

s/output_size/buffer_size/

[sheu, 2013/08/03 01:31:03]

output_buffer_size_, then.  I'd like to make it clear this is an output.

+    //  in bytes.
+    virtual void RequireBitstreamBuffers(int input_count,

[Pawel Osciak, 2013/08/01 09:00:42]

I think we are missing a Flush() or something similar after all. This API
requires the client to provide at least input_count input buffers at all times,
otherwise the encoder may not return output buffers at all until it does. But at
the end of the stream we need to force return of the rest of the stream without
providing any more input buffers. Unless I'm missing something?

[Ami GONE FROM CHROMIUM, 2013/08/01 20:49:22]

We dropped Flush() from go/vea (see resolved comment thread from "3:53 PM Mar
20") when we realized it was unnecessary for real-time encode purposes.
If/when a high-latency encode use-case is introduced then we can add
Flush/NotifyFlushDone.

(if you still think we need Flush now can you describe the use-case in terms of
a hangout?)

[Pawel Osciak, 2013/08/02 00:25:10]

In terms of a hangout, no. This came up while writing/planning
testing/validation for encoders. Of course we could queue another set of
input_count of empty frames to get more or less the same effect (although we
don't really specify if there is a 1:1 relationship between input frames and
output bitstream buffers either). But ok, ok, I know I'm (slightly) overthinking
it.
I guess this is not the place for the discussion on how to test the encoder at
the end of the stream. I withdraw :)

+                                         const gfx::Size& input_dimensions,
+                                         size_t output_size) = 0;
+
+    // Callback to notify that encoder has finished with an input frame.
+    virtual void NotifyInputDone(int32 bitstream_buffer_id) = 0;

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:12]

s/Input/Encode/ (and in the comment)

[sheu, 2013/08/03 01:31:03]

I renamed it to Input a while back.  I wanted to avoid implying that this
callback notifies that an frame has been encoded: it doesn't, it just informs
the client that a particular input can be dispensed with, no guarantees on
whether its frame contents have actually been encoded yet.

Actually, since we're moving to media::VideoFrame, and they're all
reference-counted wrappers around const memory -- I'll just drop this entry
point.

+
+    // Callback to deliver encoded bitstream buffers.  The VEA disclaims use or
+    // ownership of the specified buffer once this callback is made.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

"disclaims use and ownership" is blort.
What is true is that the VEA promises not to use the BB from the time this
method is called until VEA::UOBB is called again.  I think the comment could do
a better job of communicating this.

+    // |bitstream_buffer_id| is the id of the buffer that is ready.
+    // |size| is the byte size of the used portion of the buffer.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

missing "payload_" from param name

[sheu, 2013/08/03 01:31:03]

Done.

+    virtual void BitstreamBufferReady(int32 bitstream_buffer_id,
+                                      size_t payload_size,
+                                      bool key_frame) = 0;

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

doco param

[sheu, 2013/08/03 01:31:03]

Done.

+
+    // Error notification callback.
+    virtual void NotifyError(Error error) = 0;
+
+   protected:

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

Why protected the dtor?

[sheu, 2013/08/03 01:31:03]

Because VDA::Client does the same?

Not sure if this should be private or protected, but generally we shouldn't be
deleting VEA::Client through any pointers held to a Client instance.

+    virtual ~Client() {}
+  };
+
+  // Video encoder functions.
+
+  // Initialize the video encoder with a specific configuration.  Called once
+  // per encoder construction.
+  // Parameters:
+  //  |input_format| is the frame format of the input stream.
+  //  |input_resolution| is the resolution of the input stream.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

It is unfortunate that this needs to be specified at the head of the stream for
variable-resolution sources (such as tab capture when the tab is resized,
someday soon).  Can this become dynamic?

[sheu, 2013/08/03 01:31:03]

Unfortunately, we'd best tear down and reinstantiate a VEA at this point. 
Exynos HW does not support resolution changes midstream.

The webrtc::VideoEncoder interface also doesn't provide for resolution changing,
so I think we're good here.

+  //  |output_profile| is the codec profile of the encoded output stream.
+  //  |initial_bitrate| is the initial bitrate of the encoded output stream,
+  //  in bits per second.
+  virtual void Initialize(media::VideoFrame::Format input_format,
+                          const gfx::Size& input_resolution,
+                          VideoCodecProfile output_profile,
+                          int32 initial_bitrate) = 0;
+
+  // Encodes the given frame.
+  // Parameters:
+  //  |buffer| is the buffer holding the frame that is to be encoded (with a
+  //  buffer logical size/width as specified in RequireBitstreamBuffers(), and
+  //  visible size/width as specified in Initialize()).

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

This should be a VideoFrame, not a BitstreamBuffer.

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

Please make sure (DCHECK) the requirements on visible & physical size/width are
obeyed.

[sheu, 2013/08/03 01:31:03]

Alright, retargeting for media::VideoFrame.

+  //  |force_keyframe| forces the encoding of a keyframe for this frame.
+  virtual void Encode(const BitstreamBuffer& buffer, bool force_keyframe) = 0;
+
+  // Send a bitstream buffer to the encoder to be used for storing future
+  // encoded output.
+  // Parameters:
+  //  |buffer| is the bitstream buffer to use for output.
+  virtual void UseOutputBitstreamBuffer(const BitstreamBuffer& buffer) = 0;
+
+  // 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.
+  virtual void RequestEncodingParameterChange(int32 bitrate) = 0;
+
+  // Destroys the encoder: all pending inputs are dropped immediately and the

[Ami GONE FROM CHROMIUM, 2013/07/31 23:01:13]

s/inputs/inputs and outputs/

[sheu, 2013/08/03 01:31:03]

Done.

+  // 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 |this|
+  // unconditionally, so make sure to drop all pointers to it!
+  virtual void Destroy() = 0;
+};
+
+}  // namespace media
+
+#endif  // MEDIA_VIDEO_VIDEO_ENCODE_ACCELERATOR_H_