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 8cef40d42e807afec74299b1f7af84b87a1276e3..89aae1b66dc9346e887bd9bb4d31f0a1636f6eb8 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 @@ bool ValidSimulcastResolutions(const webrtc::VideoCodec& codec,
 }
 
 int VerifyCodec(const webrtc::VideoCodec* inst) {
-  if (inst == NULL) {
+  if (inst == nullptr) {
     return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
   }
   if (inst->maxFramerate < 1) {
@@ -127,36 +127,49 @@ class TemporalLayersFactoryAdapter : public webrtc::TemporalLayersFactory {
 namespace webrtc {
 
 SimulcastEncoderAdapter::SimulcastEncoderAdapter(VideoEncoderFactory* factory)
-    : factory_(factory),
+    : inited_(0),
+      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() {
-  Release();
+  RTC_DCHECK(!Initialized());
+  DestroyStoredEncoders();
 }
 
 int SimulcastEncoderAdapter::Release() {
-  // 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().
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
+
   while (!streaminfos_.empty()) {
     VideoEncoder* encoder = streaminfos_.back().encoder;
-    EncodedImageCallback* callback = streaminfos_.back().callback;
     encoder->Release();
-    factory_->Destroy(encoder);
-    delete callback;
-    streaminfos_.pop_back();
+    // 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);
+
   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;
   }
@@ -172,6 +185,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
   }
 
   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)) {
@@ -202,7 +216,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
       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,
@@ -214,7 +228,17 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
       stream_codec.qpMax = kDefaultMaxQp;
     }
 
-    VideoEncoder* encoder = factory_->Create();
+    // 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();
+    }
+
     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
@@ -223,21 +247,30 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst,
       Release();
       return ret;
     }
-    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)
+    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) {
       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;
 }
 
@@ -245,10 +278,12 @@ int SimulcastEncoderAdapter::Encode(
     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_ == NULL) {
+  if (encoded_complete_callback_ == nullptr) {
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
   }
 
@@ -275,8 +310,9 @@ int SimulcastEncoderAdapter::Encode(
   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) {
@@ -338,12 +374,14 @@ int SimulcastEncoderAdapter::Encode(
 
 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);
   }
@@ -352,20 +390,26 @@ int SimulcastEncoderAdapter::SetChannelParameters(uint32_t packet_loss,
 
 int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate,
                                                uint32_t new_framerate) {
-  if (!Initialized())
+  RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_);
+
+  if (!Initialized()) {
     return WEBRTC_VIDEO_CODEC_UNINITIALIZED;
+  }
 
-  if (new_framerate < 1)
+  if (new_framerate < 1) {
     return WEBRTC_VIDEO_CODEC_ERR_PARAMETER;
+  }
 
-  if (codec_.maxBitrate > 0 && bitrate.get_sum_kbps() > codec_.maxBitrate)
+  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) {
@@ -388,8 +432,9 @@ int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate,
     // 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);
   }
@@ -397,6 +442,8 @@ int SimulcastEncoderAdapter::SetRateAllocation(const BitrateAllocation& bitrate,
   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,
@@ -412,24 +459,25 @@ EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage(
 }
 
 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.
-  if (stream_index == 0) {
+  const bool lowest_resolution_stream = (stream_index == 0);
+  if (lowest_resolution_stream) {
     // Settings for lowest spatial resolutions.
     stream_codec->qpMax = kLowestResMaxQp;
   }
@@ -449,28 +497,42 @@ void SimulcastEncoderAdapter::PopulateStreamCodec(
 }
 
 bool SimulcastEncoderAdapter::Initialized() const {
-  return !streaminfos_.empty();
+  return rtc::AtomicOps::AcquireLoad(&inited_) == 1;
+}
+
+void SimulcastEncoderAdapter::DestroyStoredEncoders() {
+  while (!stored_encoders_.empty()) {
+    VideoEncoder* encoder = stored_encoders_.top();
+    factory_->Destroy(encoder);
+    stored_encoders_.pop();
+  }
 }
 
 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();
 }