Clean up ownership of webrtc::VideoEncoder

webrtc/media/engine/webrtcvideoengine.cc

Index: webrtc/media/engine/webrtcvideoengine.cc
diff --git a/webrtc/media/engine/webrtcvideoengine.cc b/webrtc/media/engine/webrtcvideoengine.cc
index 6808f2c59afedc7d468b9326cc3350510244206a..9c11bb946930a5b5f09a48c4a65c617fc302702d 100644
--- a/webrtc/media/engine/webrtcvideoengine.cc
+++ b/webrtc/media/engine/webrtcvideoengine.cc
@@ -24,6 +24,7 @@
 #include "webrtc/media/engine/constants.h"
 #include "webrtc/media/engine/internaldecoderfactory.h"
 #include "webrtc/media/engine/internalencoderfactory.h"
+#include "webrtc/media/engine/scopedvideoencoder.h"
 #include "webrtc/media/engine/simulcast.h"
 #include "webrtc/media/engine/simulcast_encoder_adapter.h"
 #include "webrtc/media/engine/videodecodersoftwarefallbackwrapper.h"
@@ -55,75 +56,6 @@ bool IsFlexfecAdvertisedFieldTrialEnabled() {
   return webrtc::field_trial::IsEnabled("WebRTC-FlexFEC-03-Advertised");
 }
 
-// An encoder factory that wraps Create requests for simulcastable codec types
-// with a webrtc::SimulcastEncoderAdapter. Non simulcastable codec type
-// requests are just passed through to the contained encoder factory.
-class WebRtcSimulcastEncoderFactory
-    : public cricket::WebRtcVideoEncoderFactory {
- public:
-  // WebRtcSimulcastEncoderFactory doesn't take ownership of |factory|, which is
-  // owned by e.g. PeerConnectionFactory.
-  explicit WebRtcSimulcastEncoderFactory(
-      cricket::WebRtcVideoEncoderFactory* factory)
-      : factory_(factory) {}
-
-  static bool UseSimulcastEncoderFactory(
-      const std::vector<cricket::VideoCodec>& codecs) {
-    // If any codec is VP8, use the simulcast factory. If asked to create a
-    // non-VP8 codec, we'll just return a contained factory encoder directly.
-    for (const auto& codec : codecs) {
-      if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
-        return true;
-      }
-    }
-    return false;
-  }
-
-  webrtc::VideoEncoder* CreateVideoEncoder(
-      const cricket::VideoCodec& codec) override {
-    RTC_DCHECK(factory_ != NULL);
-    // If it's a codec type we can simulcast, create a wrapped encoder.
-    if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
-      return new webrtc::SimulcastEncoderAdapter(factory_);
-    }
-    webrtc::VideoEncoder* encoder = factory_->CreateVideoEncoder(codec);
-    if (encoder) {
-      non_simulcast_encoders_.push_back(encoder);
-    }
-    return encoder;
-  }
-
-  const std::vector<cricket::VideoCodec>& supported_codecs() const override {
-    return factory_->supported_codecs();
-  }
-
-  bool EncoderTypeHasInternalSource(
-      webrtc::VideoCodecType type) const override {
-    return factory_->EncoderTypeHasInternalSource(type);
-  }
-
-  void DestroyVideoEncoder(webrtc::VideoEncoder* encoder) override {
-    // Check first to see if the encoder wasn't wrapped in a
-    // SimulcastEncoderAdapter. In that case, ask the factory to destroy it.
-    if (std::remove(non_simulcast_encoders_.begin(),
-                    non_simulcast_encoders_.end(),
-                    encoder) != non_simulcast_encoders_.end()) {
-      factory_->DestroyVideoEncoder(encoder);
-      return;
-    }
-
-    // Otherwise, SimulcastEncoderAdapter can be deleted directly, and will call
-    // DestroyVideoEncoder on the factory for individual encoder instances.
-    delete encoder;
-  }
-
- private:
-  cricket::WebRtcVideoEncoderFactory* factory_;
-  // A list of encoders that were created without being wrapped in a
-  // SimulcastEncoderAdapter.
-  std::vector<webrtc::VideoEncoder*> non_simulcast_encoders_;
-};
-
 void AddDefaultFeedbackParams(VideoCodec* codec) {
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
@@ -441,20 +373,6 @@ void WebRtcVideoEngine::SetExternalDecoderFactory(
 void WebRtcVideoEngine::SetExternalEncoderFactory(
     WebRtcVideoEncoderFactory* encoder_factory) {
   RTC_DCHECK(!initialized_);
-  if (external_encoder_factory_ == encoder_factory)
-    return;
-
-  // No matter what happens we shouldn't hold on to a stale
-  // WebRtcSimulcastEncoderFactory.
-  simulcast_encoder_factory_.reset();
-
-  if (encoder_factory &&
-      WebRtcSimulcastEncoderFactory::UseSimulcastEncoderFactory(
-          encoder_factory->supported_codecs())) {
-    simulcast_encoder_factory_.reset(
-        new WebRtcSimulcastEncoderFactory(encoder_factory));
-    encoder_factory = simulcast_encoder_factory_.get();
-  }
   external_encoder_factory_ = encoder_factory;
 }
 
@@ -1496,18 +1414,19 @@ WebRtcVideoChannel::WebRtcVideoSendStream::VideoSendStreamParameters::
       codec_settings(codec_settings) {}
 
 WebRtcVideoChannel::WebRtcVideoSendStream::AllocatedEncoder::AllocatedEncoder(
-    webrtc::VideoEncoder* encoder,
+    std::unique_ptr<webrtc::VideoEncoder> encoder,
+    std::unique_ptr<webrtc::VideoEncoder> external_encoder,
     const cricket::VideoCodec& codec,
-    bool external)
-    : encoder(encoder),
-      external_encoder(nullptr),
-      codec(codec),
-      external(external) {
-  if (external) {
-    external_encoder = encoder;
-    this->encoder =
-        new webrtc::VideoEncoderSoftwareFallbackWrapper(codec, encoder);
-  }
+    bool has_internal_source)
+    : encoder_(std::move(encoder)),
+      external_encoder_(std::move(external_encoder)),
+      codec_(codec),
+      has_internal_source_(has_internal_source) {}
+
+void WebRtcVideoChannel::WebRtcVideoSendStream::AllocatedEncoder::Reset() {
+  external_encoder_.reset();
+  encoder_.reset();
+  codec_ = cricket::VideoCodec();
 }
 
 WebRtcVideoChannel::WebRtcVideoSendStream::WebRtcVideoSendStream(
@@ -1535,7 +1454,6 @@ WebRtcVideoChannel::WebRtcVideoSendStream::WebRtcVideoSendStream(
       encoder_sink_(nullptr),
       parameters_(std::move(config), options, max_bitrate_bps, codec_settings),
       rtp_parameters_(CreateRtpParametersWithOneEncoding()),
-      allocated_encoder_(nullptr, cricket::VideoCodec(), false),
       sending_(false) {
   parameters_.config.rtp.max_packet_size = kVideoMtu;
   parameters_.conference_mode = send_params.conference_mode;
@@ -1592,7 +1510,8 @@ WebRtcVideoChannel::WebRtcVideoSendStream::~WebRtcVideoSendStream() {
   if (stream_ != NULL) {
     call_->DestroyVideoSendStream(stream_);
   }
-  DestroyVideoEncoder(&allocated_encoder_);
+  // Release |allocated_encoder_|.
+  allocated_encoder_.Reset();
 }
 
 bool WebRtcVideoChannel::WebRtcVideoSendStream::SetVideoSend(
@@ -1665,54 +1584,59 @@ WebRtcVideoChannel::WebRtcVideoSendStream::GetSsrcs() const {
 
 WebRtcVideoChannel::WebRtcVideoSendStream::AllocatedEncoder
 WebRtcVideoChannel::WebRtcVideoSendStream::CreateVideoEncoder(
-    const VideoCodec& codec,
-    bool force_encoder_allocation) {
+    const VideoCodec& codec) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
-  // Do not re-create encoders of the same type.
-  if (!force_encoder_allocation && codec == allocated_encoder_.codec &&
-      allocated_encoder_.encoder != nullptr) {
-    return allocated_encoder_;
-  }
 
   // Try creating external encoder.
   if (external_encoder_factory_ != nullptr &&
       FindMatchingCodec(external_encoder_factory_->supported_codecs(), codec)) {
-    webrtc::VideoEncoder* encoder =
-        external_encoder_factory_->CreateVideoEncoder(codec);
-    if (encoder != nullptr)
-      return AllocatedEncoder(encoder, codec, true /* is_external */);
+    std::unique_ptr<webrtc::VideoEncoder> external_encoder;
+    if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
+      // If it's a codec type we can simulcast, create a wrapped encoder.
+      external_encoder = std::unique_ptr<webrtc::VideoEncoder>(
+          new webrtc::SimulcastEncoderAdapter(external_encoder_factory_));
+    } else {
+      external_encoder =
+          CreateScopedVideoEncoder(external_encoder_factory_, codec);
+    }
+    if (external_encoder) {
+      std::unique_ptr<webrtc::VideoEncoder> internal_encoder(
+          new webrtc::VideoEncoderSoftwareFallbackWrapper(
+              codec, external_encoder.get()));
+      const webrtc::VideoCodecType codec_type =
+          webrtc::PayloadStringToCodecType(codec.name);
+      const bool has_internal_source =
+          external_encoder_factory_->EncoderTypeHasInternalSource(codec_type);
+      return AllocatedEncoder(std::move(internal_encoder),
+                              std::move(external_encoder), codec,
+                              has_internal_source);
+    }
   }
 
   // Try creating internal encoder.
+  std::unique_ptr<webrtc::VideoEncoder> internal_encoder;
   if (FindMatchingCodec(internal_encoder_factory_->supported_codecs(), codec)) {
-    if (parameters_.encoder_config.content_type ==
+    if (CodecNamesEq(codec.name.c_str(), kVp8CodecName) &&
+        parameters_.encoder_config.content_type ==
             webrtc::VideoEncoderConfig::ContentType::kScreen &&
         parameters_.conference_mode && UseSimulcastScreenshare()) {
       // TODO(sprang): Remove this adapter once libvpx supports simulcast with
       // same-resolution substreams.
-      WebRtcSimulcastEncoderFactory adapter_factory(
-          internal_encoder_factory_.get());
-      return AllocatedEncoder(adapter_factory.CreateVideoEncoder(codec), codec,
-                              false /* is_external */);
+      internal_encoder = std::unique_ptr<webrtc::VideoEncoder>(
+          new webrtc::SimulcastEncoderAdapter(internal_encoder_factory_.get()));
+    } else {
+      internal_encoder = std::unique_ptr<webrtc::VideoEncoder>(
+          internal_encoder_factory_->CreateVideoEncoder(codec));
     }
-    return AllocatedEncoder(
-        internal_encoder_factory_->CreateVideoEncoder(codec), codec,
-        false /* is_external */);
+    return AllocatedEncoder(std::move(internal_encoder),
+                            nullptr /* external_encoder */, codec,
+                            false /* has_internal_source */);
   }
 
   // This shouldn't happen, we should not be trying to create something we don't
   // support.
   RTC_NOTREACHED();
-  return AllocatedEncoder(NULL, cricket::VideoCodec(), false);
-}
-
-void WebRtcVideoChannel::WebRtcVideoSendStream::DestroyVideoEncoder(
-    AllocatedEncoder* encoder) {
-  RTC_DCHECK_RUN_ON(&thread_checker_);
-  if (encoder->external) {
-    external_encoder_factory_->DestroyVideoEncoder(encoder->external_encoder);
-  }
-  delete encoder->encoder;
+  return AllocatedEncoder();
 }
 
 void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec(
@@ -1722,20 +1646,23 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec(
   parameters_.encoder_config = CreateVideoEncoderConfig(codec_settings.codec);
   RTC_DCHECK_GT(parameters_.encoder_config.number_of_streams, 0);
 
-  AllocatedEncoder new_encoder =
-      CreateVideoEncoder(codec_settings.codec, force_encoder_allocation);
-  parameters_.config.encoder_settings.encoder = new_encoder.encoder;
-  parameters_.config.encoder_settings.full_overuse_time = new_encoder.external;
-  parameters_.config.encoder_settings.payload_name = codec_settings.codec.name;
-  parameters_.config.encoder_settings.payload_type = codec_settings.codec.id;
-  if (new_encoder.external) {
-    webrtc::VideoCodecType type =
-        webrtc::PayloadStringToCodecType(codec_settings.codec.name);
-    parameters_.config.encoder_settings.internal_source =
-        external_encoder_factory_->EncoderTypeHasInternalSource(type);
+  // Do not re-create encoders of the same type. We can't overwrite
+  // |allocated_encoder_| immediately, because we need to release it after the
+  // RecreateWebRtcStream() call.
+  AllocatedEncoder new_encoder;
+  if (force_encoder_allocation || !allocated_encoder_.encoder() ||
+      allocated_encoder_.codec() != codec_settings.codec) {
+    new_encoder = CreateVideoEncoder(codec_settings.codec);
   } else {
-    parameters_.config.encoder_settings.internal_source = false;
+    new_encoder = std::move(allocated_encoder_);
   }
+  parameters_.config.encoder_settings.encoder = new_encoder.encoder();
+  parameters_.config.encoder_settings.full_overuse_time =
+      new_encoder.IsExternal();
+  parameters_.config.encoder_settings.payload_name = codec_settings.codec.name;
+  parameters_.config.encoder_settings.payload_type = codec_settings.codec.id;
+  parameters_.config.encoder_settings.internal_source =
+      new_encoder.HasInternalSource();
   parameters_.config.rtp.ulpfec = codec_settings.ulpfec;
   parameters_.config.rtp.flexfec.payload_type =
       codec_settings.flexfec_payload_type;
@@ -1759,10 +1686,7 @@ void WebRtcVideoChannel::WebRtcVideoSendStream::SetCodec(
 
   LOG(LS_INFO) << "RecreateWebRtcStream (send) because of SetCodec.";
   RecreateWebRtcStream();
-  if (allocated_encoder_.encoder != new_encoder.encoder) {
-    DestroyVideoEncoder(&allocated_encoder_);
-    allocated_encoder_ = new_encoder;
-  }
+  allocated_encoder_ = std::move(new_encoder);
 }
 
 void WebRtcVideoChannel::WebRtcVideoSendStream::SetSendParameters(