Add media::VideoEncodeAccelerator with WebRTC integration

media/video/video_encode_accelerator.h

+// 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.
+  struct SupportedProfile {
+    VideoCodecProfile profile;
+    gfx::Size max_resolution;
+    struct {
+      uint32 numerator;
+      uint32 denominator;
+    } max_framerate;

[Pawel Osciak, 2013/07/26 02:26:43]

I'm not sure about this. Is this because of something specific in our HW or
required by webrtc? Max framerate may depend on selected codec, profile,
resolution, bitrate, etc...

[sheu, 2013/07/26 19:09:07]

Right, this struct is used by the VEA to export a supported configuration.  We'd
get a list of these.

[Pawel Osciak, 2013/07/27 08:35:48]

Ok, but how does max_framerate relate to max_resolution here? This structure
should provide a list of resolutions with supported framerates for each
resolution (or at least one max_framerate for each resolution).

[sheu, 2013/07/29 18:24:52]

I suppose we could have a vector of supported max resolutions/framerates, but
until we have hardware and a usecase for different limits per resolution I'd
just like to keep it this way?

(Basically I don't want to make SupportedProfile too complicated... yet.)

[Pawel Osciak, 2013/08/01 03:21:39]

How will this be used now then? Is there anything using it now? There are no
calls in the API that return it that I can see...

+  };
+
+  // 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.
+    // Examples of such failures include GPU hardware failures, GPU driver
+    // failures, GPU library failures, browser programming errors, and so on.
+    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 client what size of buffers to provide for input and

[Pawel Osciak, 2013/07/26 02:26:43]

s/tell client/tell the client/

[sheu, 2013/07/26 19:09:07]

Done.

+    // output.  The VEA disclaims use or ownership of all previously provided

[Pawel Osciak, 2013/07/26 02:26:43]

So this means that we can have one set of input/output buffers at a time and we
switch between sets via RequireBitstreamBuffers. Who is responsible for freeing
them? I assume after RequireBitstreamBuffers is called, the client is expected
to free the old set?

[sheu, 2013/07/26 19:09:07]

Right, that's the "disclaims use or _ownership_" part of things.

+    // buffers once this callback is made.
+    // 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.
+    //  |input_dimensions| is the logical dimensions of the input frames to

[Pawel Osciak, 2013/07/26 02:26:43]

s/is/are perhaps?

I understand the dimensions part, but I'd say explicitly it's still in pixels.

[sheu, 2013/07/26 19:09:07]

Done.

+    //  encode.  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 |nput_dimensions|.

[Pawel Osciak, 2013/07/26 02:26:43]

s/nput_/input_/

[sheu, 2013/07/26 19:09:07]

Done.

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

[Pawel Osciak, 2013/07/26 02:26:43]

in bytes

[sheu, 2013/07/26 19:09:07]

Done.

+    virtual void RequireBitstreamBuffers(int input_count,
+                                         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;
+
+    // Callback to deliver encoded bitstream buffers.  The VEA disclaims use or
+    // ownership of the specified buffer once this callback is made.
+    // |bitstream_buffer_id| is the id of the buffer that is ready.
+    // |size| is the byte size of the used portion of the buffer.
+    virtual void BitstreamBufferReady(int32 bitstream_buffer_id,
+                                      size_t size,

[Pawel Osciak, 2013/07/26 02:26:43]

s/size/payload_size ?

[sheu, 2013/07/26 19:09:07]

Done.  Now to run through all the rest of the source files and rename :-P

+                                      bool key_frame) = 0;
+
+    // Error notification callback.
+    virtual void NotifyError(Error error) = 0;
+
+   protected:
+    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.
+  //  |output_profile| is the codec profile of the encoded output stream.
+  //  |initial_bitrate| is the initial bitrate of the encoded output stream,
+  //  in bits/s.
+  virtual void Initialize(media::VideoFrame::Format input_format,
+                          const gfx::Size& input_resolution,
+                          VideoCodecProfile output_profile,
+                          int32 initial_bitrate) = 0;

[Pawel Osciak, 2013/07/26 02:26:43]

Perhaps we could drop initial_bitrate and simply call
RequestEncodingParameterChange() before the first Encode() ?

[sheu, 2013/07/26 19:09:07]

Thought about this.  fischman@'s initial VEA spec also had bitrate in the
Initialize() call, which I found kind of odd, but in the end I reasoned that
encoders that don't support midstream bitrate changing should be able to just
stub that call out.

[Pawel Osciak, 2013/07/27 08:35:48]

Not following you. I agree bitrate change or any other request to change
parameters may not be supported and it's ok not to do anything, but how does
that relate to having initial_bitrate here?

[sheu, 2013/07/29 18:24:52]

You need an initial bitrate to start encoding; can't just let the HW guess what
you want.  I'd consider it an initial parameter just like input_Format or
input_resolution.

[Pawel Osciak, 2013/08/01 03:21:39]

Or you could let encoder choose a default. I don't see a problem with that and
encoders must have something set as default. I feel it's still better than
having this one-off here in Initialize() and also that this is too tied to
current use case. And when we add more knobs, will we only allow setting bitrate
initially? Or ask for them to be set via RequestEncodingParameterChange before
first Encode?

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

FTR I don't care about this question (I can easily accept either way).

+
+  // 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()).
+  //  |force_keyframe| forces the encoding of a keyframe for this frame.
+  virtual void Encode(const BitstreamBuffer& buffer, bool force_keyframe) = 0;

[Pawel Osciak, 2013/07/26 02:26:43]

Btw, why are we using BitstreamBuffer for inputs? For SHM?

[sheu, 2013/07/26 19:09:07]

Yep.  They also hold a size and ID nicely.

[Pawel Osciak, 2013/07/27 08:35:48]

Well, it's going to be less useful when we start using dmabufs or something. Can
we use something else that also doesn't imply they hold bitstream? VideoFrame?
Weren't you using it before? It also has SHM.

[sheu, 2013/07/29 18:24:52]

DMABUFs are also FDs, which can be dup()ed and IPCed just like the typical
anonymous shared memory backing we have for current SharedMemory buffers.  So I
don't see a need for changing this when DMABUFs weigh in, which is the reason I
chose to go this route.

[Pawel Osciak, 2013/08/01 03:21:39]

Well, I won't push too hard. I just find unpleasantly asymmetric for VDA to do
BitstreamBuffer->VideoFrame while VEA doing BitstreamBuffer->BitstreamBuffer.
Looks like Ami agrees with me on PS3.

+
+  // 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 in the encoding parameters.  This is only a request,

[Pawel Osciak, 2013/07/26 02:26:43]

s/in/to/ ?

[sheu, 2013/07/26 19:09:07]

Not a big deal, but sure :-P

+  // fulfilled on a best-effort basis.

[Pawel Osciak, 2013/07/26 02:26:43]

If the change is possible, is there a requirement when will it happen (on next
Encode()d frame, after X frames)? When can we try to call this again? I think we
may need a notification whether the request could be fulfilled or not, otherwise
the client may wait forever for it, may not know when it's ok to try a different
change, etc.

[sheu, 2013/07/26 19:09:07]

I was hoping this to be fire-and-forget thing for the client.  A more advisory
call, I guess.

There's a problem with the current MFC encoding parameters change bit where we
can't actually specify exactly which frame a parameters change starts to take
effect from, since all the V4L2 API allows us to do is to queue up buffers, and
the parameter change ioctl on the userspace process thread is asynchronous with
respect to the IRQ handler thread that actually schedules the buffer to the
hardware.  So I'm can't make a guarantee on when it has to happen.

The above could be fixed in the driver (probably by attaching the encode
parameters per-buffer), but that's something to talk to SLSI about, while they
actually implement the parameter changing.

[Pawel Osciak, 2013/07/27 08:35:48]

Ok, I guess it's ok not to have it frame-precise atm, but I'd still be
advocating for at least a success/failure callback, even if we can't exactly
tell when, for the other reasons stated above.

[sheu, 2013/07/29 18:24:52]

I'll let fischman@ weigh in on this one, since the VEA design doc was
fire-and-forget as well.

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

I don't see the value in having the callback indicate success/failure today.
The client can detect success/failure by observing the encoded bitrate (since it
has information of timestamps as well as emitted bitstream buffers and their
sizes).

[Pawel Osciak, 2013/08/01 03:21:39]

But the client can't tell when the change in bitrate will happen. How long is it
supposed to wait for it to happen or not? Should it try a different value when
it fails, because minimum bitrate for example depends on resolution and
sometimes it cannot go lower?
I still feel callbacks may become necessary for other knobs added later, but I'm
ok with not adding them now just for bitrate.

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

Generally I want to minimize the amount of conversation the VEA and its client
have about capabilities and fallbacks (because I think that way lies madness and
also because I believe we don't need anywhere near that much flexibility for our
use-cases).

I'm fine revisiting this when more knobs are added.

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

Ok. I give up, you are right, too much detail for as of yet nonexisting
requirements.

+  // Parameters:
+  //  |bitrate| is the requested new bitrate.

[Pawel Osciak, 2013/07/26 02:26:43]

in kbps, in bps?

[sheu, 2013/07/26 19:09:07]

Done.

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