Revert to recreating RTCVideoEncoder::Impl

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 146 matching lines @@
 
   enum {
     kInputBufferExtraCount = 1,  // The number of input buffers allocated, more
                                  // than what is requested by
                                  // VEA::RequireBitstreamBuffers().
     kOutputBufferCount = 3,
   };
 
   ~Impl() override;
 
+  // Resets the class members.
+  void Reset();
+
   // Logs the |error| and |str| sent from |location| and NotifyError()s forward.
   void LogAndNotifyError(const tracked_objects::Location& location,
                          const std::string& str,
                          media::VideoEncodeAccelerator::Error error);
 
   // Perform encoding on an input frame from the input queue.
   void EncodeOneFrame();
 
   // Notify that an input frame is finished for encoding.  |index| is the index
   // of the completed frame in |input_buffers_|.
@@ skipping 78 matching lines @@
 
   DISALLOW_COPY_AND_ASSIGN(Impl);
 };
 
 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),
       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));
+  Reset();

[wuchengli, 2016/04/29 13:58:24]

This means RTCVideoEncoder is reused after RTCVideoEncoder::Release is called.
You can consider changing the code back before my patch (#1) -- new |impl_| in
RTCVideoEncoder::InitEncode and release it in RTCVideoEncoder::Release. Then we
don't need to reset anything.

If you prefer Reset(), we should also clear |input_next_frame_| and
|encoded_image_callback_| in Reset().

#1:
https://codereview.chromium.org/1845563006/diff/100001/content/renderer/media...

[emircan, 2016/04/29 19:37:27]

Sure, reverting to earlier sounds better as the destructed class will clear all
the members. I thought the current re-use was an intended behavior, so I was
going for Reset().

 }
 
 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());
@@ skipping 96 matching lines @@
     video_encoder_->RequestEncodingParametersChange(bitrate * 1000, framerate);
 }
 
 void RTCVideoEncoder::Impl::Destroy(base::WaitableEvent* async_waiter) {
   DVLOG(3) << "Impl::Destroy()";
   DCHECK(thread_checker_.CalledOnValidThread());
   if (video_encoder_) {
     video_encoder_.reset();
     SetStatus(WEBRTC_VIDEO_CODEC_UNINITIALIZED);
   }
+  Reset();
   async_waiter->Signal();
 }
 
 int32_t RTCVideoEncoder::Impl::GetStatus() const {
   base::AutoLock lock(status_lock_);
   return status_;
 }
 
 void RTCVideoEncoder::Impl::SetStatus(int32_t status) {
   base::AutoLock lock(status_lock_);
@@ skipping 111 matching lines @@
 
   video_encoder_.reset();
 
   SetStatus(retval);
   if (async_waiter_)
     SignalAsyncWaiter(retval);
 }
 
 RTCVideoEncoder::Impl::~Impl() { DCHECK(!video_encoder_); }
 
+void RTCVideoEncoder::Impl::Reset() {
+  input_next_frame_keyframe_ = false;
+  input_buffers_.clear();
+  output_buffers_.clear();
+  input_buffers_free_.clear();
+  output_buffers_free_count_ = 0;
+  // Picture ID should start on a random number.
+  picture_id_ = static_cast<uint16_t>(base::RandInt(0, 0x7FFF));
+}
+
 void RTCVideoEncoder::Impl::LogAndNotifyError(
     const tracked_objects::Location& location,
     const std::string& str,
     media::VideoEncodeAccelerator::Error error) {
   static const char* const kErrorNames[] = {
       "kIllegalStateError", "kInvalidArgumentError", "kPlatformFailureError"};
   static_assert(
       arraysize(kErrorNames) == media::VideoEncodeAccelerator::kErrorMax + 1,
       "Different number of errors and textual descriptions");
   DLOG(ERROR) << location.ToString() << kErrorNames[error] << " - " << str;
@@ skipping 305 matching lines @@
   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