Make RTCVideoEncoder usable from non-postable thread.

content/renderer/media/rtc_video_encoder.cc

 // 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.
 
 #include "content/renderer/media/rtc_video_encoder.h"
 
 #include <string.h>
 
 #include "base/bind.h"
 #include "base/location.h"
@@ skipping 74 matching lines @@
   }
   return true;
 }
 
 }  // namespace
 
 // This private class of RTCVideoEncoder does the actual work of communicating
 // with a media::VideoEncodeAccelerator for handling video encoding.  It can
 // be created on any thread, but should subsequently be posted to (and Destroy()
 // called on) a single thread.  Callbacks to RTCVideoEncoder are posted to the
 // thread on which the instance was constructed.

[emircan, 2016/04/12 20:15:02]

"Callbacks to RTCVideoEncoder are ... constructed" AFAIU, there aren't callbacks
posted there any more. Safe to remove?

[wuchengli, 2016/04/13 15:26:26]

Yes. Removed. Also updated other comments here.

 //
 // This class separates state related to the thread that RTCVideoEncoder
 // operates on (presently the libjingle worker thread) from the thread that
 // |gpu_factories_| provides for accelerator operations (presently the media
 // thread).  The RTCVideoEncoder class can be deleted directly by WebRTC, while
 // RTCVideoEncoder::Impl stays around long enough to properly shut down the VEA.
 class RTCVideoEncoder::Impl
     : public media::VideoEncodeAccelerator::Client,
       public base::RefCountedThreadSafe<RTCVideoEncoder::Impl> {
  public:
-  Impl(const base::WeakPtr<RTCVideoEncoder>& weak_encoder,
-       media::GpuVideoAcceleratorFactories* gpu_factories);
+  Impl(media::GpuVideoAcceleratorFactories* gpu_factories,
+       webrtc::VideoCodecType video_codec_type);
 
   // Create the VEA and call Initialize() on it.  Called once per instantiation,
   // and then the instance is bound forevermore to whichever thread made the
   // call.
   // RTCVideoEncoder expects to be able to call this function synchronously from
   // its own thread, hence the |async_waiter| and |async_retval| arguments.
   void CreateAndInitializeVEA(const gfx::Size& input_visible_size,
                               uint32_t bitrate,
                               media::VideoCodecProfile profile,
                               base::WaitableEvent* async_waiter,
                               int32_t* async_retval);
   // Enqueue a frame from WebRTC for encoding.
   // RTCVideoEncoder expects to be able to call this function synchronously from
   // its own thread, hence the |async_waiter| and |async_retval| arguments.
   void Enqueue(const webrtc::VideoFrame* input_frame,
                bool force_keyframe,
                base::WaitableEvent* async_waiter,
                int32_t* async_retval);
 
   // RTCVideoEncoder is given a buffer to be passed to WebRTC through the
   // RTCVideoEncoder::ReturnEncodedImage() function.  When that is complete,
   // the buffer is returned to Impl by its index using this function.
   void UseOutputBitstreamBufferId(int32_t bitstream_buffer_id);
 
   // Request encoding parameter change for the underlying encoder.
   void RequestEncodingParametersChange(uint32_t bitrate, uint32_t framerate);
 
+  void RegisterEncodeCompleteCallback(base::WaitableEvent* async_waiter,
+                                      int32_t* async_retval,
+                                      webrtc::EncodedImageCallback* callback);
+
   // Destroy this Impl's encoder.  The destructor is not explicitly called, as
   // Impl is a base::RefCountedThreadSafe.
-  void Destroy();
+  void Destroy(base::WaitableEvent* async_waiter);
+
+  // Return the status of Impl. One of WEBRTC_VIDEO_CODEC_XXX value.
+  int32_t GetStatus();

[emircan, 2016/04/12 20:15:02]

Mark method as const.

[wuchengli, 2016/04/13 15:26:26]

Done.

+
+  webrtc::VideoCodecType video_codec_type() { return video_codec_type_; }
 
   // media::VideoEncodeAccelerator::Client implementation.
   void RequireBitstreamBuffers(unsigned int input_count,
                                const gfx::Size& input_coded_size,
                                size_t output_buffer_size) override;
   void BitstreamBufferReady(int32_t bitstream_buffer_id,
                             size_t payload_size,
                             bool key_frame) override;
   void NotifyError(media::VideoEncodeAccelerator::Error error) override;
 
@@ skipping 25 matching lines @@
   void RegisterAsyncWaiter(base::WaitableEvent* waiter, int32_t* retval);
   void SignalAsyncWaiter(int32_t retval);
 
   // Checks if the bitrate would overflow when passing from kbps to bps.
   bool IsBitrateTooHigh(uint32_t bitrate);
 
   // Checks if the frame size is different than hardware accelerator
   // requirements.
   bool RequiresSizeChange(const scoped_refptr<media::VideoFrame>& frame) const;
 
+  // Return an encoded output buffer to WebRTC.
+  void ReturnEncodedImage(const webrtc::EncodedImage& image,
+                          int32_t bitstream_buffer_id,
+                          uint16_t picture_id);
+

[emircan, 2016/04/12 20:15:02]

Can we add a comment about this being attached to gpu_task_runner_, but not the
thread class is constructed on.

[wuchengli, 2016/04/13 15:26:26]

Done.

   base::ThreadChecker thread_checker_;
 
-  // Weak pointer to the parent RTCVideoEncoder, for posting back VEA::Client
-  // notifications.
-  const base::WeakPtr<RTCVideoEncoder> weak_encoder_;
-
-  // The message loop on which to post callbacks to |weak_encoder_|.
-  const scoped_refptr<base::SingleThreadTaskRunner> encoder_task_runner_;
-
   // Factory for creating VEAs, shared memory buffers, etc.
   media::GpuVideoAcceleratorFactories* gpu_factories_;
 
   // webrtc::VideoEncoder expects InitEncode() and Encode() to be synchronous.
   // Do this by waiting on the |async_waiter_| and returning the return value in
   // |async_retval_| when initialization completes, encoding completes, or
   // an error occurs.
   base::WaitableEvent* async_waiter_;
   int32_t* async_retval_;
 
@@ skipping 19 matching lines @@
   // we don't care about ordering.
   std::vector<int> input_buffers_free_;
 
   // The number of output buffers ready to be filled with output from the
   // encoder.
   int output_buffers_free_count_;
 
   // 15 bits running index of the VP8 frames. See VP8 RTP spec for details.
   uint16_t picture_id_;
 
+  // webrtc::VideoEncoder encode complete callback.
+  webrtc::EncodedImageCallback* encoded_image_callback_;
+
+  // The video codec type, as reported to WebRTC.
+  const webrtc::VideoCodecType video_codec_type_;
+
+  // 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. |status_| is read or written on
+  // |gpu_task_runner_| in Impl. It can be read in RTCVideoEncoder on other
+  // threads, hence atomic.
+  std::atomic<int32_t> status_;

[pbos, 2016/04/12 11:25:59]

std::atomic looks banned for now, I think you have to use base/atomicops.h

https://chromium-cpp.appspot.com/#library-blacklist

[wuchengli, 2016/04/13 15:26:26]

Thanks. The comments in base/atomicops.h do not encourage people to use it. I
used a lock instead.

+
   DISALLOW_COPY_AND_ASSIGN(Impl);
 };
 
-RTCVideoEncoder::Impl::Impl(const base::WeakPtr<RTCVideoEncoder>& weak_encoder,
-                            media::GpuVideoAcceleratorFactories* gpu_factories)
-    : weak_encoder_(weak_encoder),
-      encoder_task_runner_(base::ThreadTaskRunnerHandle::Get()),
-      gpu_factories_(gpu_factories),
+RTCVideoEncoder::Impl::Impl(media::GpuVideoAcceleratorFactories* gpu_factories,
+                            webrtc::VideoCodecType video_codec_type)
+    : gpu_factories_(gpu_factories),
       async_waiter_(NULL),
       async_retval_(NULL),
       input_next_frame_(NULL),
       input_next_frame_keyframe_(false),
-      output_buffers_free_count_(0) {
+      output_buffers_free_count_(0),
+      encoded_image_callback_(nullptr),
+      video_codec_type_(video_codec_type),
+      status_(WEBRTC_VIDEO_CODEC_UNINITIALIZED) {
   thread_checker_.DetachFromThread();
   // Picture ID should start on a random number.
   picture_id_ = static_cast<uint16_t>(base::RandInt(0, 0x7FFF));
 }
 
 void RTCVideoEncoder::Impl::CreateAndInitializeVEA(
     const gfx::Size& input_visible_size,
     uint32_t bitrate,
     media::VideoCodecProfile profile,
     base::WaitableEvent* async_waiter,
     int32_t* async_retval) {
   DVLOG(3) << "Impl::CreateAndInitializeVEA()";
   DCHECK(thread_checker_.CalledOnValidThread());
 
+  status_.store(WEBRTC_VIDEO_CODEC_UNINITIALIZED);
   RegisterAsyncWaiter(async_waiter, async_retval);
 
   // Check for overflow converting bitrate (kilobits/sec) to bits/sec.
   if (IsBitrateTooHigh(bitrate))
     return;
 
   video_encoder_ = gpu_factories_->CreateVideoEncodeAccelerator();
   if (!video_encoder_) {
     LogAndNotifyError(FROM_HERE, "Error creating VideoEncodeAccelerator",
                       media::VideoEncodeAccelerator::kPlatformFailureError);
     return;
   }
   input_visible_size_ = input_visible_size;
   if (!video_encoder_->Initialize(media::PIXEL_FORMAT_I420, input_visible_size_,
                                   profile, bitrate * 1000, this)) {
     LogAndNotifyError(FROM_HERE, "Error initializing video_encoder",
                       media::VideoEncodeAccelerator::kInvalidArgumentError);
     return;
   }
+  // RequireBitstreamBuffers or NotifyError will be called and the waiter will
+  // be signaled.
 }
 
 void RTCVideoEncoder::Impl::Enqueue(const webrtc::VideoFrame* input_frame,
                                     bool force_keyframe,
                                     base::WaitableEvent* async_waiter,
                                     int32_t* async_retval) {
   DVLOG(3) << "Impl::Enqueue()";
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(!input_next_frame_);
 
   RegisterAsyncWaiter(async_waiter, async_retval);
+  int32_t retval = status_.load();
+  if (retval != WEBRTC_VIDEO_CODEC_OK) {
+    SignalAsyncWaiter(retval);
+    return;
+  }
+
   // If there are no free input and output buffers, drop the frame to avoid a
   // deadlock. If there is a free input buffer, EncodeOneFrame will run and
   // unblock Encode(). If there are no free input buffers but there is a free
   // output buffer, EncodeFrameFinished will be called later to unblock
   // Encode().
   //
   // The caller of Encode() holds a webrtc lock. The deadlock happens when:
   // (1) Encode() is waiting for the frame to be encoded in EncodeOneFrame().
   // (2) There are no free input buffers and they cannot be freed because
   //     the encoder has no output buffers.
@@ skipping 39 matching lines @@
   DCHECK(thread_checker_.CalledOnValidThread());
 
   // Check for overflow converting bitrate (kilobits/sec) to bits/sec.
   if (IsBitrateTooHigh(bitrate))
     return;
 
   if (video_encoder_)
     video_encoder_->RequestEncodingParametersChange(bitrate * 1000, framerate);
 }
 
-void RTCVideoEncoder::Impl::Destroy() {
+void RTCVideoEncoder::Impl::Destroy(base::WaitableEvent* async_waiter) {
   DVLOG(3) << "Impl::Destroy()";
   DCHECK(thread_checker_.CalledOnValidThread());
-  video_encoder_.reset();
+  if (video_encoder_) {
+    video_encoder_.reset();
+    status_.store(WEBRTC_VIDEO_CODEC_UNINITIALIZED);
+  }
+  async_waiter->Signal();
+}
+
+int32_t RTCVideoEncoder::Impl::GetStatus() {
+  return status_.load();
 }
 
 void RTCVideoEncoder::Impl::RequireBitstreamBuffers(
     unsigned int input_count,
     const gfx::Size& input_coded_size,
     size_t output_buffer_size) {
   DVLOG(3) << "Impl::RequireBitstreamBuffers(): input_count=" << input_count
            << ", input_coded_size=" << input_coded_size.ToString()
            << ", output_buffer_size=" << output_buffer_size;
   DCHECK(thread_checker_.CalledOnValidThread());
@@ skipping 26 matching lines @@
     }
     output_buffers_.push_back(shm.release());
   }
 
   // Immediately provide all output buffers to the VEA.
   for (size_t i = 0; i < output_buffers_.size(); ++i) {
     video_encoder_->UseOutputBitstreamBuffer(media::BitstreamBuffer(
         i, output_buffers_[i]->handle(), output_buffers_[i]->mapped_size()));
     output_buffers_free_count_++;
   }
+  DCHECK_EQ(status_.load(), WEBRTC_VIDEO_CODEC_UNINITIALIZED);
+  status_.store(WEBRTC_VIDEO_CODEC_OK);
   SignalAsyncWaiter(WEBRTC_VIDEO_CODEC_OK);
 }
 
 void RTCVideoEncoder::Impl::BitstreamBufferReady(int32_t bitstream_buffer_id,
                                                  size_t payload_size,
                                                  bool key_frame) {
   DVLOG(3) << "Impl::BitstreamBufferReady(): "
               "bitstream_buffer_id=" << bitstream_buffer_id
            << ", payload_size=" << payload_size
            << ", key_frame=" << key_frame;
@@ skipping 15 matching lines @@
 
   // Use webrtc timestamps to ensure correct RTP sender behavior.
   // TODO(hshi): obtain timestamp from the capturer, see crbug.com/350106.
   const int64_t capture_time_us = webrtc::TickTime::MicrosecondTimestamp();
 
   // Derive the capture time (in ms) and RTP timestamp (in 90KHz ticks).
   const int64_t capture_time_ms = capture_time_us / 1000;
   const uint32_t rtp_timestamp =
       static_cast<uint32_t>(capture_time_us * 90 / 1000);
 
-  std::unique_ptr<webrtc::EncodedImage> image(new webrtc::EncodedImage(
+  webrtc::EncodedImage image(
       reinterpret_cast<uint8_t*>(output_buffer->memory()), payload_size,
-      output_buffer->mapped_size()));
-  image->_encodedWidth = input_visible_size_.width();
-  image->_encodedHeight = input_visible_size_.height();
-  image->_timeStamp = rtp_timestamp;
-  image->capture_time_ms_ = capture_time_ms;
-  image->_frameType =
+      output_buffer->mapped_size());
+  image._encodedWidth = input_visible_size_.width();
+  image._encodedHeight = input_visible_size_.height();
+  image._timeStamp = rtp_timestamp;
+  image.capture_time_ms_ = capture_time_ms;
+  image._frameType =
       (key_frame ? webrtc::kVideoFrameKey : webrtc::kVideoFrameDelta);
-  image->_completeFrame = true;
+  image._completeFrame = true;
 
-  encoder_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&RTCVideoEncoder::ReturnEncodedImage, weak_encoder_,
-                 base::Passed(&image), bitstream_buffer_id, picture_id_));
+  ReturnEncodedImage(image, bitstream_buffer_id, picture_id_);
   // Picture ID must wrap after reaching the maximum.
   picture_id_ = (picture_id_ + 1) & 0x7FFF;
 }
 
 void RTCVideoEncoder::Impl::NotifyError(
     media::VideoEncodeAccelerator::Error error) {
   DCHECK(thread_checker_.CalledOnValidThread());
   int32_t retval;
   switch (error) {
     case media::VideoEncodeAccelerator::kInvalidArgumentError:
       retval = WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
       break;
     default:
       retval = WEBRTC_VIDEO_CODEC_ERROR;
   }
 
   video_encoder_.reset();
 
-  if (async_waiter_) {
+  status_.store(retval);
+  if (async_waiter_)
     SignalAsyncWaiter(retval);
-  } else {
-    encoder_task_runner_->PostTask(
-        FROM_HERE,
-        base::Bind(&RTCVideoEncoder::NotifyError, weak_encoder_, retval));
-  }
 }
 
 RTCVideoEncoder::Impl::~Impl() { DCHECK(!video_encoder_); }
 
 void RTCVideoEncoder::Impl::LogAndNotifyError(
     const tracked_objects::Location& location,
     const std::string& str,
     media::VideoEncodeAccelerator::Error error) {
   static const char* kErrorNames[] = {
       "kIllegalStateError", "kInvalidArgumentError", "kPlatformFailureError"};
@@ skipping 109 matching lines @@
                     media::VideoEncodeAccelerator::kInvalidArgumentError);
   return true;
 }
 
 bool RTCVideoEncoder::Impl::RequiresSizeChange(
     const scoped_refptr<media::VideoFrame>& frame) const {
   return (frame->coded_size() != input_frame_coded_size_ ||
           frame->visible_rect() != gfx::Rect(input_visible_size_));
 }
 
+void RTCVideoEncoder::Impl::RegisterEncodeCompleteCallback(
+    base::WaitableEvent* async_waiter,
+    int32_t* async_retval,
+    webrtc::EncodedImageCallback* callback) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(3) << "RegisterEncodeCompleteCallback()";
+  RegisterAsyncWaiter(async_waiter, async_retval);
+  if (status_.load() == WEBRTC_VIDEO_CODEC_OK) {

[emircan, 2016/04/12 20:15:03]

Remove { } since it is a one-liner.

[wuchengli, 2016/04/13 15:26:26]

Done.

+    encoded_image_callback_ = callback;
+  }
+  SignalAsyncWaiter(status_.load());
+}
+
+void RTCVideoEncoder::Impl::ReturnEncodedImage(
+    const webrtc::EncodedImage& image,
+    int32_t bitstream_buffer_id,
+    uint16_t picture_id) {
+  DCHECK(thread_checker_.CalledOnValidThread());
+  DVLOG(3) << "ReturnEncodedImage(): "
+           << "bitstream_buffer_id=" << bitstream_buffer_id
+           << ", picture_id=" << picture_id;
+
+  if (!encoded_image_callback_)
+    return;
+
+  webrtc::RTPFragmentationHeader header;
+  memset(&header, 0, sizeof(header));
+  switch (video_codec_type_) {
+    case webrtc::kVideoCodecVP8:
+      // Generate a header describing a single fragment.
+      header.VerifyAndAllocateFragmentationHeader(1);
+      header.fragmentationOffset[0] = 0;
+      header.fragmentationLength[0] = image._length;
+      header.fragmentationPlType[0] = 0;
+      header.fragmentationTimeDiff[0] = 0;
+      break;
+    case webrtc::kVideoCodecH264:
+      if (!GetRTPFragmentationHeaderH264(&header, image._buffer,
+                                         image._length)) {
+        DLOG(ERROR) << "Failed to get RTP fragmentation header for H264";
+        NotifyError(
+            (media::VideoEncodeAccelerator::Error)WEBRTC_VIDEO_CODEC_ERROR);
+        return;
+      }
+      break;
+    default:
+      NOTREACHED() << "Invalid video codec type";
+      return;
+  }
+
+  webrtc::CodecSpecificInfo info;
+  memset(&info, 0, sizeof(info));
+  info.codecType = video_codec_type_;
+  if (video_codec_type_ == webrtc::kVideoCodecVP8) {
+    info.codecSpecific.VP8.pictureId = picture_id;
+    info.codecSpecific.VP8.tl0PicIdx = -1;
+    info.codecSpecific.VP8.keyIdx = -1;
+  }
+
+  int32_t retval = encoded_image_callback_->Encoded(image, &info, &header);

[emircan, 2016/04/12 20:15:02]

const

[wuchengli, 2016/04/13 15:26:26]

Done.

+  if (retval < 0) {
+    DVLOG(2) << "ReturnEncodedImage(): encoded_image_callback_ returned "
+             << retval;
+  }
+
+  UseOutputBitstreamBufferId(bitstream_buffer_id);
+}
+
 RTCVideoEncoder::RTCVideoEncoder(
     webrtc::VideoCodecType type,
     media::GpuVideoAcceleratorFactories* gpu_factories)
-    : video_codec_type_(type),
-      gpu_factories_(gpu_factories),
+    : gpu_factories_(gpu_factories),
       gpu_task_runner_(gpu_factories->GetTaskRunner()),
-      encoded_image_callback_(NULL),
-      impl_status_(WEBRTC_VIDEO_CODEC_UNINITIALIZED),
-      weak_factory_(this) {
+      impl_(new Impl(gpu_factories_, type)) {
   DVLOG(1) << "RTCVideoEncoder(): codec type=" << type;
 }
 
 RTCVideoEncoder::~RTCVideoEncoder() {
   DVLOG(3) << "~RTCVideoEncoder";
-  DCHECK(thread_checker_.CalledOnValidThread());
   Release();
-  DCHECK(!impl_.get());
 }
 
 int32_t RTCVideoEncoder::InitEncode(const webrtc::VideoCodec* codec_settings,
                                     int32_t number_of_cores,
                                     size_t max_payload_size) {
   DVLOG(1) << "InitEncode(): codecType=" << codec_settings->codecType
            << ", width=" << codec_settings->width
            << ", height=" << codec_settings->height
            << ", startBitrate=" << codec_settings->startBitrate;
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DCHECK(!impl_.get());
 
-  const media::VideoCodecProfile profile =
-      WebRTCVideoCodecToVideoCodecProfile(video_codec_type_, codec_settings);
+  const media::VideoCodecProfile profile = WebRTCVideoCodecToVideoCodecProfile(
+      impl_->video_codec_type(), codec_settings);
 
-  weak_factory_.InvalidateWeakPtrs();
-  impl_ = new Impl(weak_factory_.GetWeakPtr(), gpu_factories_);
   base::WaitableEvent initialization_waiter(true, false);
   int32_t initialization_retval = WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   gpu_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RTCVideoEncoder::Impl::CreateAndInitializeVEA,
                  impl_,
                  gfx::Size(codec_settings->width, codec_settings->height),
                  codec_settings->startBitrate,
                  profile,
                  &initialization_waiter,
                  &initialization_retval));
 
   // webrtc::VideoEncoder expects this call to be synchronous.
   initialization_waiter.Wait();
   RecordInitEncodeUMA(initialization_retval, profile);
   return initialization_retval;
 }
 
 int32_t RTCVideoEncoder::Encode(
     const webrtc::VideoFrame& input_image,
     const webrtc::CodecSpecificInfo* codec_specific_info,
     const std::vector<webrtc::FrameType>* frame_types) {
   DVLOG(3) << "Encode()";
-  if (!impl_.get()) {
-    DVLOG(3) << "Encode(): returning impl_status_=" << impl_status_;
-    return impl_status_;
-  }
 
   const bool want_key_frame = frame_types && frame_types->size() &&
                               frame_types->front() == webrtc::kVideoFrameKey;
   base::WaitableEvent encode_waiter(true, false);
   int32_t encode_retval = WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   gpu_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RTCVideoEncoder::Impl::Enqueue,
                  impl_,
                  &input_image,
                  want_key_frame,
                  &encode_waiter,
                  &encode_retval));
 
   // webrtc::VideoEncoder expects this call to be synchronous.
   encode_waiter.Wait();
   DVLOG(3) << "Encode(): returning encode_retval=" << encode_retval;
   return encode_retval;
 }
 
 int32_t RTCVideoEncoder::RegisterEncodeCompleteCallback(
     webrtc::EncodedImageCallback* callback) {
   DVLOG(3) << "RegisterEncodeCompleteCallback()";
-  DCHECK(thread_checker_.CalledOnValidThread());
-  if (!impl_.get()) {
-    DVLOG(3) << "RegisterEncodeCompleteCallback(): returning " << impl_status_;
-    return impl_status_;
-  }
-
-  encoded_image_callback_ = callback;
-  return WEBRTC_VIDEO_CODEC_OK;
+  base::WaitableEvent register_waiter(true, false);
+  int32_t register_retval = WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  gpu_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&RTCVideoEncoder::Impl::RegisterEncodeCompleteCallback, impl_,
+                 &register_waiter, &register_retval, callback));
+  register_waiter.Wait();
+  return register_retval;
 }
 
 int32_t RTCVideoEncoder::Release() {
   DVLOG(3) << "Release()";
-  DCHECK(thread_checker_.CalledOnValidThread());
 
-  if (impl_.get()) {
-    gpu_task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&RTCVideoEncoder::Impl::Destroy, impl_));
-    impl_ = NULL;
-    weak_factory_.InvalidateWeakPtrs();
-    impl_status_ = WEBRTC_VIDEO_CODEC_UNINITIALIZED;
-  }
+  base::WaitableEvent release_waiter(true, false);
+  gpu_task_runner_->PostTask(
+      FROM_HERE,
+      base::Bind(&RTCVideoEncoder::Impl::Destroy, impl_, &release_waiter));
+  release_waiter.Wait();
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t RTCVideoEncoder::SetChannelParameters(uint32_t packet_loss,
                                               int64_t rtt) {
   DVLOG(3) << "SetChannelParameters(): packet_loss=" << packet_loss
            << ", rtt=" << rtt;
   // Ignored.
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int32_t RTCVideoEncoder::SetRates(uint32_t new_bit_rate, uint32_t frame_rate) {
   DVLOG(3) << "SetRates(): new_bit_rate=" << new_bit_rate
            << ", frame_rate=" << frame_rate;
-  if (!impl_.get()) {
-    DVLOG(3) << "SetRates(): returning " << impl_status_;
-    return impl_status_;
+  int32_t retval = impl_->GetStatus();

[emircan, 2016/04/12 20:15:03]

const

[wuchengli, 2016/04/13 15:26:26]

Done.

+  if (retval != WEBRTC_VIDEO_CODEC_OK) {
+    DVLOG(3) << "SetRates(): returning " << retval;
+    return retval;
   }
 

[emircan, 2016/04/12 20:15:02]

A question: Should l.371 return WEBRTC_VIDEO_CODEC_OK as it is right now or some
error?

[wuchengli, 2016/04/13 15:26:26]

After l.371 (IsBitrateTooHigh) returns false, |status_| will be set to
WEBRTC_VIDEO_CODEC_ERR_PARAMETER. The next RtcVideoEncoder::Encode will return
the error. RTCVideoEncoder::Impl::RequestEncodingParametersChange is called
frequently (maybe once per second). It's better not to change it from
asynchronous to synchronous. If it should be changed, it should be another CL
and needs some heavy testing.

   gpu_task_runner_->PostTask(
       FROM_HERE,
       base::Bind(&RTCVideoEncoder::Impl::RequestEncodingParametersChange,
                  impl_,
                  new_bit_rate,
                  frame_rate));
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
-void RTCVideoEncoder::ReturnEncodedImage(
-    std::unique_ptr<webrtc::EncodedImage> image,
-    int32_t bitstream_buffer_id,
-    uint16_t picture_id) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DVLOG(3) << "ReturnEncodedImage(): "
-           << "bitstream_buffer_id=" << bitstream_buffer_id
-           << ", picture_id=" << picture_id;
-
-  if (!encoded_image_callback_)
-    return;
-
-  webrtc::RTPFragmentationHeader header;
-  memset(&header, 0, sizeof(header));
-  switch (video_codec_type_) {
-    case webrtc::kVideoCodecVP8:
-      // Generate a header describing a single fragment.
-      header.VerifyAndAllocateFragmentationHeader(1);
-      header.fragmentationOffset[0] = 0;
-      header.fragmentationLength[0] = image->_length;
-      header.fragmentationPlType[0] = 0;
-      header.fragmentationTimeDiff[0] = 0;
-      break;
-    case webrtc::kVideoCodecH264:
-      if (!GetRTPFragmentationHeaderH264(
-          &header, image->_buffer, image->_length)) {
-        DLOG(ERROR) << "Failed to get RTP fragmentation header for H264";
-        NotifyError(WEBRTC_VIDEO_CODEC_ERROR);
-        return;
-      }
-      break;
-    default:
-      NOTREACHED() << "Invalid video codec type";
-      return;
-  }
-
-  webrtc::CodecSpecificInfo info;
-  memset(&info, 0, sizeof(info));
-  info.codecType = video_codec_type_;
-  if (video_codec_type_ == webrtc::kVideoCodecVP8) {
-    info.codecSpecific.VP8.pictureId = picture_id;
-    info.codecSpecific.VP8.tl0PicIdx = -1;
-    info.codecSpecific.VP8.keyIdx = -1;
-  }
-
-  int32_t retval = encoded_image_callback_->Encoded(*image, &info, &header);
-  if (retval < 0) {
-    DVLOG(2) << "ReturnEncodedImage(): encoded_image_callback_ returned "
-             << retval;
-  }
-
-  // The call through webrtc::EncodedImageCallback is synchronous, so we can
-  // immediately recycle the output buffer back to the Impl.
-  gpu_task_runner_->PostTask(
-      FROM_HERE,
-      base::Bind(&RTCVideoEncoder::Impl::UseOutputBitstreamBufferId,
-                 impl_,
-                 bitstream_buffer_id));
-}
-
-void RTCVideoEncoder::NotifyError(int32_t error) {
-  DCHECK(thread_checker_.CalledOnValidThread());
-  DVLOG(1) << "NotifyError(): error=" << error;
-
-  impl_status_ = error;
-  gpu_task_runner_->PostTask(FROM_HERE,
-                         base::Bind(&RTCVideoEncoder::Impl::Destroy, impl_));
-  impl_ = NULL;
-}
-
 void RTCVideoEncoder::RecordInitEncodeUMA(
     int32_t init_retval, media::VideoCodecProfile profile) {
   UMA_HISTOGRAM_BOOLEAN("Media.RTCVideoEncoderInitEncodeSuccess",
                         init_retval == WEBRTC_VIDEO_CODEC_OK);
   if (init_retval == WEBRTC_VIDEO_CODEC_OK) {
     UMA_HISTOGRAM_ENUMERATION("Media.RTCVideoEncoderProfile",
                               profile,
                               media::VIDEO_CODEC_PROFILE_MAX + 1);
   }
 }
 
 }  // namespace content