Add media::VideoEncodeAccelerator with WebRTC integration

content/renderer/media/rtc_video_encoder.h

Index: content/renderer/media/rtc_video_encoder.h
diff --git a/content/renderer/media/rtc_video_encoder.h b/content/renderer/media/rtc_video_encoder.h
new file mode 100644
index 0000000000000000000000000000000000000000..b7600cefe8d63b7b37ba7bdef2153c7223573e0e
--- /dev/null
+++ b/content/renderer/media/rtc_video_encoder.h
@@ -0,0 +1,130 @@
+// 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 CONTENT_RENDERER_MEDIA_RTC_VIDEO_ENCODER_H_
+#define CONTENT_RENDERER_MEDIA_RTC_VIDEO_ENCODER_H_
+
+#include <vector>
+
+#include "base/memory/scoped_vector.h"
+#include "base/memory/weak_ptr.h"
+#include "base/threading/thread_checker.h"
+#include "content/common/content_export.h"
+#include "media/base/video_decoder_config.h"
+#include "third_party/webrtc/modules/video_coding/codecs/interface/video_codec_interface.h"
+#include "ui/gfx/size.h"
+
+namespace base {
+
+class MessageLoopProxy;
+
+}  // namespace base
+
+namespace media {
+
+class GpuVideoAcceleratorFactories;
+
+}  // namespace media
+
+namespace content {
+
+// This class uses a media::VideoEncodeAccelerator to implement a
+// webrtc::VideoEncoder class for WebRTC.  Internally, VEA methods are
+// trampolined to a private RTCVideoEncoder::Impl instance.  The class runs on
+// the |impl_message_loop_proxy_|, which is queried from the |gpu_factories_|
+// and is presently the media thread.  VEA::Client callbacks are posted back to
+// the thread that the RTCVideoEncoder constructor is called on.

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

This description made me expect a http://c2.com/cgi/wiki?PimplIdiom (and thus
confused over the fact that this class has private methods & members, which it
wouldn't if you were using pimpl) but that's not what you're doing at all. 
Instead Impl serves the purpose of lifecycle extension, apparently because
you're worried about webrtc deleting this directly.  But I don't believe it
does; I believe it goes through the factory's DeleteEncoder method to delete
this, so you can introduce the needed lifecycle extension there.  I think that
would be a far preferable choice to the current approach because ATM it RVE
implies it ownes RVE::Impl (via a scoped_ptr) and webrtc implies it owns RVE
(via the factory's deleteencoder which is a pass-through), but in reality, it is
~Impl that deletes its "owning" RVE, and deleteencoder only schedules a future
~Impl to run.  This covert ownership inversion is well worth avoiding IMO.

[sheu, 2013/08/02 01:27:49]

"covert ownership inversion" would be a good underground punk-rock band name,
BTW.

+class CONTENT_EXPORT RTCVideoEncoder
+    : NON_EXPORTED_BASE(public webrtc::VideoEncoder) {
+ public:
+  RTCVideoEncoder(
+      media::VideoCodecProfile profile,
+      const scoped_refptr<media::GpuVideoAcceleratorFactories>& gpu_factories);
+  virtual ~RTCVideoEncoder();
+
+  // webrtc::VideoEncoder implementation.  Tasks are posted to |impl_| using the
+  // appropriate VEA methods.
+  virtual int32_t InitEncode(const webrtc::VideoCodec* codec_settings,
+                             int32_t number_of_cores,
+                             uint32_t max_payload_size) OVERRIDE;
+  virtual int32_t Encode(
+      const webrtc::I420VideoFrame& input_image,
+      const webrtc::CodecSpecificInfo* codec_specific_info,
+      const std::vector<webrtc::VideoFrameType>* frame_types) OVERRIDE;
+  virtual int32_t RegisterEncodeCompleteCallback(
+      webrtc::EncodedImageCallback* callback) OVERRIDE;
+  virtual int32_t Release() OVERRIDE;
+  virtual int32_t SetChannelParameters(uint32_t packet_loss, int rtt) OVERRIDE;
+  virtual int32_t SetRates(uint32_t new_bit_rate, uint32_t frame_rate) OVERRIDE;
+
+ private:
+  class Impl;
+  friend class RTCVideoEncoder::Impl;
+
+  enum {
+    kInputBufferExtraCount = 1,  // The number of input buffers allocated, more
+                                 // than what is requested by
+                                 // VEA::RequireBitstreamBuffers().

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

Where are these numbers from?  What is the effect of changing them?

+    kOutputBufferCount = 3,
+  };
+
+  // Handlers for VEA::Client methods, which translate back to the
+  // WebRTC::VideoEncoder methods.

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

no they're not ;)

+  void RequireBitstreamBuffers(int input_count,
+                               const gfx::Size& input_dimensions,
+                               size_t output_size);
+  void NotifyInputDone(int32 bitstream_buffer_id);
+  void BitstreamBufferReady(int32 bitstream_buffer_id,
+                            size_t payload_size,
+                            bool key_frame);
+  void NotifyError(int32_t error);
+
+  base::ThreadChecker thread_checker_;

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

Not reviewing the rest of this file and its .cc since I expect them to change
dramatically as a result of my comment at l.37.

+
+  // The video codec profile we were created with.
+  media::VideoCodecProfile video_codec_profile_;
+
+  // Factory for creating VEAs, shared memory buffers, etc.
+  const scoped_refptr<media::GpuVideoAcceleratorFactories> gpu_factories_;
+
+  // Message loop that |impl_| runs on; queried from |gpu_factories_|.
+  const scoped_refptr<base::MessageLoopProxy> impl_message_loop_proxy_;
+
+  // Weak pointer and factory for posting back VEA::Client notifications to
+  // RTCVideoEncoder.
+  base::WeakPtrFactory<RTCVideoEncoder> weak_this_factory_;
+  base::WeakPtr<RTCVideoEncoder> weak_this_;
+
+  // webrtc::VideoEncoder encode complete callback.
+  webrtc::EncodedImageCallback* encoded_image_callback_;
+
+  // The RTCVideoEncoder::Impl that does all the work.
+  scoped_ptr<Impl> impl_;
+
+  // We cannot immediately return error conditions to the WebRTC user of this
+  // class, as there is no error callback in the webrtc::VideoEncoder interface.
+  // Instead, we cache an error status here and return it the next time an
+  // interface entry point is called.
+  int32_t impl_status_;
+
+  // Frame parameters.
+  gfx::Size input_frame_dimensions_;
+  gfx::Size output_frame_dimensions_;
+
+  // Required output buffer size.
+  size_t output_buffer_size_;
+
+  // Shared memory buffers for input/output with the VEA.
+  ScopedVector<base::SharedMemory> input_buffers_;
+  ScopedVector<base::SharedMemory> output_buffers_;
+
+  // Input buffers ready to be filled with input from Encode().
+  std::vector<int> input_buffers_free_;
+
+  DISALLOW_COPY_AND_ASSIGN(RTCVideoEncoder);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_RENDERER_MEDIA_RTC_VIDEO_ENCODER_H_