Revert of Reuse allocated encoders in SimulcastEncoderAdapter.

webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc

Index: webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
diff --git a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
index 89aae1b66dc9346e887bd9bb4d31f0a1636f6eb8..8cef40d42e807afec74299b1f7af84b87a1276e3 100644
--- a/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
+++ b/webrtc/modules/video_coding/codecs/vp8/simulcast_encoder_adapter.cc
@@ -62,7 +62,7 @@
 }
 
 int VerifyCodec(const webrtc::VideoCodec* inst) {
-  if (inst == nullptr) {
+  if (inst == NULL) {
     return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
   }
   if (inst->maxFramerate < 1) {
@@ -127,49 +127,36 @@
 namespace webrtc {
 
 SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory)
-    : inited_(0),
-      factory_(factory),
+    : factory_(factory),
       encoded_complete_callback_(nullptr),
       implementation_name_("SimulcastEncoderAdapter") {
-  // The adapter is typically created on the worker thread, but operated on
-  // the encoder task queue.
-  encoder_queue_.Detach();
-
   memset(&codec_, 0, sizeof(webrtc::VideoCodec));
 }
 
 SimulcastEncoderAdapter::~SimulcastEncoderAdapter() {
-  RTC_DCHECK(!Initialized());
-  DestroyStoredEncoders();
+  Release();
 }
 
 int SimulcastEncoderAdapter::Release() {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
-
+  // TODO(pbos): Keep the last encoder instance but call ::Release() on it, then
+  // re-use this instance in ::InitEncode(). This means that changing
+  // resolutions doesn't require reallocation of the first encoder, but only
+  // reinitialization, which makes sense. Then Destroy this instance instead in
+  // ~SimulcastEncoderAdapter().
   while (!streaminfos_.empty()) {
     VideoEncoder* encoder = streaminfos_.back().encoder;
+    EncodedImageCallback* callback = streaminfos_.back().callback;
     encoder->Release();
-    // Even though it seems very unlikely, there are no guarantees that the
-    // encoder will not call back after being Release()'d. Therefore, we disable
-    // the callbacks here.
-    encoder->RegisterEncodeCompleteCallback(nullptr);
-    streaminfos_.pop_back();  // Deletes callback adapter.
-    stored_encoders_.push(encoder);
-  }
-
-  // It's legal to move the encoder to another queue now.
-  encoder_queue_.Detach();
-
-  rtc::AtomicOps::ReleaseStore(&inited_, 0);
-
+    factory_->Destroy(encoder);
+    delete callback;
+    streaminfos_.pop_back();
+  }
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
                                         int number_of_cores,
                                         size_t max_payload_size) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
-
   if (number_of_cores < 1) {
     return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
   }
@@ -185,7 +172,6 @@
   }
 
   int number_of_streams = NumberOfStreams(*inst);
-  RTC_DCHECK_LE(number_of_streams, kMaxSimulcastStreams);
   const bool doing_simulcast = (number_of_streams > 1);
 
   if (doing_simulcast && !ValidSimulcastResolutions(*inst, number_of_streams)) {
@@ -216,7 +202,7 @@
       start_bitrate_kbps =
           std::max(codec_.simulcastStream[i].minBitrate, start_bitrate_kbps);
       bool highest_resolution_stream = (i == (number_of_streams - 1));
-      PopulateStreamCodec(codec_, i, start_bitrate_kbps,
+      PopulateStreamCodec(&codec_, i, start_bitrate_kbps,
                           highest_resolution_stream, &stream_codec);
     }
     TemporalLayersFactoryAdapter tl_factory_adapter(i,
@@ -228,17 +214,7 @@
       stream_codec.qpMax = kDefaultMaxQp;
     }
 
-    // If an existing encoder instance exists, reuse it.
-    // TODO(brandtr): Set initial RTP state (e.g., picture_id/tl0_pic_idx) here,
-    // when we start storing that state outside the encoder wrappers.
-    VideoEncoder* encoder;
-    if (!stored_encoders_.empty()) {
-      encoder = stored_encoders_.top();
-      stored_encoders_.pop();
-    } else {
-      encoder = factory_->Create();
-    }
-
+    VideoEncoder* encoder = factory_->Create();
     ret = encoder->InitEncode(&stream_codec, number_of_cores, max_payload_size);
     if (ret < 0) {
       // Explicitly destroy the current encoder; because we haven't registered a
@@ -247,30 +223,21 @@
       Release();
       return ret;
     }
-    std::unique_ptr<EncodedImageCallback> callback(
-        new AdapterEncodedImageCallback(this, i));
-    encoder->RegisterEncodeCompleteCallback(callback.get());
-    streaminfos_.emplace_back(encoder, std::move(callback), stream_codec.width,
-                              stream_codec.height, start_bitrate_kbps > 0);
-
-    if (i != 0) {
+    EncodedImageCallback* callback = new AdapterEncodedImageCallback(this, i);
+    encoder->RegisterEncodeCompleteCallback(callback);
+    streaminfos_.push_back(StreamInfo(encoder, callback, stream_codec.width,
+                                      stream_codec.height,
+                                      start_bitrate_kbps > 0));
+    if (i != 0)
       implementation_name += ", ";
-    }
     implementation_name += streaminfos_[i].encoder->ImplementationName();
   }
-
   if (doing_simulcast) {
     implementation_name_ =
         "SimulcastEncoderAdapter (" + implementation_name + ")";
   } else {
     implementation_name_ = implementation_name;
   }
-
-  // To save memory, don't store encoders that we don't use.
-  DestroyStoredEncoders();
-
-  rtc::AtomicOps::ReleaseStore(&inited_, 1);
-
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
@@ -278,12 +245,10 @@
     const VideoFrame& input_image,
     const CodecSpecificInfo* codec_specific_info,
     const std::vector<FrameType>* frame_types) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
-
   if (!Initialized()) {
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   }
-  if (encoded_complete_callback_ == nullptr) {
+  if (encoded_complete_callback_ == NULL) {
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   }
 
@@ -310,9 +275,8 @@
   int src_height = input_image.height();
   for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) {
     // Don't encode frames in resolutions that we don't intend to send.
-    if (!streaminfos_[stream_idx].send_stream) {
+    if (!streaminfos_[stream_idx].send_stream)
       continue;
-    }
 
     std::vector<FrameType> stream_frame_types;
     if (send_key_frame) {
@@ -374,14 +338,12 @@
 
 int SimulcastEncoderAdapter::RegisterEncodeCompleteCallback(
     EncodedImageCallback* callback) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
   encoded_complete_callback_ = callback;
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
 int SimulcastEncoderAdapter::SetChannelParameters(uint32_t packet_loss,
                                                   int64_t rtt) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
   for (size_t stream_idx = 0; stream_idx < streaminfos_.size(); ++stream_idx) {
     streaminfos_[stream_idx].encoder->SetChannelParameters(packet_loss, rtt);
   }
@@ -390,26 +352,20 @@
 
 int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate,
                                                uint32_t new_framerate) {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
-
-  if (!Initialized()) {
+  if (!Initialized())
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
-  }
-
-  if (new_framerate < 1) {
-    return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
-  }
-
-  if (codec_.maxBitrate > 0 && bitrate.get_sum_kbps() > codec_.maxBitrate) {
-    return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
-  }
+
+  if (new_framerate < 1)
+    return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
+
+  if (codec_.maxBitrate > 0 && bitrate.get_sum_kbps() > codec_.maxBitrate)
+    return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
 
   if (bitrate.get_sum_bps() > 0) {
     // Make sure the bitrate fits the configured min bitrates. 0 is a special
     // value that means paused, though, so leave it alone.
-    if (bitrate.get_sum_kbps() < codec_.minBitrate) {
+    if (bitrate.get_sum_kbps() < codec_.minBitrate)
       return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
-    }
 
     if (codec_.numberOfSimulcastStreams > 0 &&
         bitrate.get_sum_kbps() < codec_.simulcastStream[0].minBitrate) {
@@ -432,9 +388,8 @@
     // Slice the temporal layers out of the full allocation and pass it on to
     // the encoder handling the current simulcast stream.
     BitrateAllocation stream_allocation;
-    for (int i = 0; i < kMaxTemporalStreams; ++i) {
+    for (int i = 0; i < kMaxTemporalStreams; ++i)
       stream_allocation.SetBitrate(0, i, bitrate.GetBitrate(stream_idx, i));
-    }
     streaminfos_[stream_idx].encoder->SetRateAllocation(stream_allocation,
                                                         new_framerate);
   }
@@ -442,8 +397,6 @@
   return WEBRTC_VIDEO_CODEC_OK;
 }
 
-// TODO(brandtr): Add task checker to this member function, when all encoder
-// callbacks are coming in on the encoder queue.
 EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage(
     size_t stream_idx,
     const EncodedImage& encodedImage,
@@ -459,25 +412,24 @@
 }
 
 void SimulcastEncoderAdapter::PopulateStreamCodec(
-    const webrtc::VideoCodec& inst,
+    const webrtc::VideoCodec* inst,
     int stream_index,
     uint32_t start_bitrate_kbps,
     bool highest_resolution_stream,
     webrtc::VideoCodec* stream_codec) {
-  *stream_codec = inst;
+  *stream_codec = *inst;
 
   // Stream specific settings.
   stream_codec->VP8()->numberOfTemporalLayers =
-      inst.simulcastStream[stream_index].numberOfTemporalLayers;
+      inst->simulcastStream[stream_index].numberOfTemporalLayers;
   stream_codec->numberOfSimulcastStreams = 0;
-  stream_codec->width = inst.simulcastStream[stream_index].width;
-  stream_codec->height = inst.simulcastStream[stream_index].height;
-  stream_codec->maxBitrate = inst.simulcastStream[stream_index].maxBitrate;
-  stream_codec->minBitrate = inst.simulcastStream[stream_index].minBitrate;
-  stream_codec->qpMax = inst.simulcastStream[stream_index].qpMax;
+  stream_codec->width = inst->simulcastStream[stream_index].width;
+  stream_codec->height = inst->simulcastStream[stream_index].height;
+  stream_codec->maxBitrate = inst->simulcastStream[stream_index].maxBitrate;
+  stream_codec->minBitrate = inst->simulcastStream[stream_index].minBitrate;
+  stream_codec->qpMax = inst->simulcastStream[stream_index].qpMax;
   // Settings that are based on stream/resolution.
-  const bool lowest_resolution_stream = (stream_index == 0);
-  if (lowest_resolution_stream) {
+  if (stream_index == 0) {
     // Settings for lowest spatial resolutions.
     stream_codec->qpMax = kLowestResMaxQp;
   }
@@ -497,42 +449,28 @@
 }
 
 bool SimulcastEncoderAdapter::Initialized() const {
-  return rtc::AtomicOps::AcquireLoad(&inited_) == 1;
-}
-
-void SimulcastEncoderAdapter::DestroyStoredEncoders() {
-  while (!stored_encoders_.empty()) {
-    VideoEncoder* encoder = stored_encoders_.top();
-    factory_->Destroy(encoder);
-    stored_encoders_.pop();
-  }
+  return !streaminfos_.empty();
 }
 
 bool SimulcastEncoderAdapter::SupportsNativeHandle() const {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
   // We should not be calling this method before streaminfos_ are configured.
   RTC_DCHECK(!streaminfos_.empty());
   for (const auto& streaminfo : streaminfos_) {
-    if (!streaminfo.encoder->SupportsNativeHandle()) {
+    if (!streaminfo.encoder->SupportsNativeHandle())
       return false;
-    }
   }
   return true;
 }
 
 VideoEncoder::ScalingSettings SimulcastEncoderAdapter::GetScalingSettings()
     const {
-  // TODO(brandtr): Investigate why the sequence checker below fails on mac.
-  // RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
   // Turn off quality scaling for simulcast.
-  if (!Initialized() || NumberOfStreams(codec_) != 1) {
+  if (!Initialized() || NumberOfStreams(codec_) != 1)
     return VideoEncoder::ScalingSettings(false);
-  }
   return streaminfos_[0].encoder->GetScalingSettings();
 }
 
 const char* SimulcastEncoderAdapter::ImplementationName() const {
-  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
   return implementation_name_.c_str();
 }