Migrate WebrtcDummyVideoEncoderFactory to new video encoder factory API.

As part of an effort to make all video codecs in WebRTC injectable and optional, migrate an encoder factory that inherited from the old class to the new one. In the new class, the unique_ptr is returned from the factory method, so let the factory keep a raw pointer and let the encoder tell the factory to reset it when released.

Bug: webrtc:7925
Change-Id: Ia058a7f09a2960b47918e441adc06ea7328aaa4c
Reviewed-on: https://chromium-review.googlesource.com/1179832
Commit-Queue: Anders Carlsson <andersc@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584707}

remoting/protocol/BUILD.gn

@@ -260,6 +260,7 @@ static_library("protocol") {
       "//third_party/webrtc/api/audio_codecs:audio_codecs_api",
       "//third_party/webrtc/api/audio_codecs/opus:audio_decoder_opus",
       "//third_party/webrtc/api/audio_codecs/opus:audio_encoder_opus",
+      "//third_party/webrtc/api/video_codecs:builtin_video_decoder_factory",
       "//third_party/webrtc_overrides:init_webrtc",
     ]
   }

remoting/protocol/webrtc_dummy_video_encoder.cc

@@ -16,6 +16,7 @@
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
 #include "remoting/protocol/video_channel_state_observer.h"
+#include "third_party/webrtc/api/video_codecs/sdp_video_format.h"
 
 #if defined(USE_H264_ENCODER)
 #include "media/video/h264_parser.h"
@@ -55,13 +56,18 @@ bool GetRTPFragmentationHeaderH264(webrtc::RTPFragmentationHeader* header,
 
 WebrtcDummyVideoEncoder::WebrtcDummyVideoEncoder(
     scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-    base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer)
+    base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer,
+    WebrtcDummyVideoEncoderFactory* factory)
     : main_task_runner_(main_task_runner),
       state_(kUninitialized),
-      video_channel_state_observer_(video_channel_state_observer) {
-}
+      video_channel_state_observer_(video_channel_state_observer),
+      factory_(factory) {}
 
-WebrtcDummyVideoEncoder::~WebrtcDummyVideoEncoder() = default;
+WebrtcDummyVideoEncoder::~WebrtcDummyVideoEncoder() {
+  if (factory_) {
+    factory_->EncoderDestroyed(this);
+  }
+}
 
 int32_t WebrtcDummyVideoEncoder::InitEncode(
     const webrtc::VideoCodec* codec_settings,
@@ -224,24 +230,24 @@ webrtc::EncodedImageCallback::Result WebrtcDummyVideoEncoder::SendEncodedFrame(
 
 WebrtcDummyVideoEncoderFactory::WebrtcDummyVideoEncoderFactory()
     : main_task_runner_(base::ThreadTaskRunnerHandle::Get()) {
-  // TODO(isheriff): These do not really affect anything internally
-  // in webrtc.
-  codecs_.push_back(cricket::VideoCodec("VP8"));
-  codecs_.push_back(cricket::VideoCodec("VP9"));
-  codecs_.push_back(cricket::VideoCodec("H264"));
+  formats_.push_back(webrtc::SdpVideoFormat("VP8"));
+  formats_.push_back(webrtc::SdpVideoFormat("VP9"));
+  formats_.push_back(webrtc::SdpVideoFormat("H264"));
 }
 
 WebrtcDummyVideoEncoderFactory::~WebrtcDummyVideoEncoderFactory() {
   DCHECK(encoders_.empty());
 }
 
-webrtc::VideoEncoder* WebrtcDummyVideoEncoderFactory::CreateVideoEncoder(
-    const cricket::VideoCodec& codec) {
-  webrtc::VideoCodecType type = webrtc::PayloadStringToCodecType(codec.name);
-  WebrtcDummyVideoEncoder* encoder = new WebrtcDummyVideoEncoder(
-      main_task_runner_, video_channel_state_observer_);
+std::unique_ptr<webrtc::VideoEncoder>
+WebrtcDummyVideoEncoderFactory::CreateVideoEncoder(
+    const webrtc::SdpVideoFormat& format) {
+  webrtc::VideoCodecType type = webrtc::PayloadStringToCodecType(format.name);
+  std::unique_ptr<WebrtcDummyVideoEncoder> encoder =
+      base::WrapUnique(new WebrtcDummyVideoEncoder(
+          main_task_runner_, video_channel_state_observer_, this));
   base::AutoLock lock(lock_);
-  encoders_.push_back(base::WrapUnique(encoder));
+  encoders_.push_back(encoder.get());
   if (encoder_created_callback_) {
     main_task_runner_->PostTask(FROM_HERE,
                                 base::Bind(encoder_created_callback_, type));
@@ -249,31 +255,19 @@ webrtc::VideoEncoder* WebrtcDummyVideoEncoderFactory::CreateVideoEncoder(
   return encoder;
 }
 
-const std::vector<cricket::VideoCodec>&
-WebrtcDummyVideoEncoderFactory::supported_codecs() const {
-  return codecs_;
+std::vector<webrtc::SdpVideoFormat>
+WebrtcDummyVideoEncoderFactory::GetSupportedFormats() const {
+  return formats_;
 }
 
-bool WebrtcDummyVideoEncoderFactory::EncoderTypeHasInternalSource(
-    webrtc::VideoCodecType type) const {
-  // Returns true to directly provide encoded frames to webrtc.
-  return true;
-}
-
-void WebrtcDummyVideoEncoderFactory::DestroyVideoEncoder(
-    webrtc::VideoEncoder* encoder) {
-  base::AutoLock lock(lock_);
-  if (encoder == nullptr) {
-    LOG(ERROR) << "Attempting to destroy null encoder";
-    return;
-  }
-  for (auto pos = encoders_.begin(); pos != encoders_.end(); ++pos) {
-    if ((*pos).get() == encoder) {
-      encoders_.erase(pos);
-      return;
-    }
-  }
-  NOTREACHED() << "Asked to remove encoder not owned by factory.";
+WebrtcDummyVideoEncoderFactory::CodecInfo
+WebrtcDummyVideoEncoderFactory::QueryVideoEncoder(
+    const webrtc::SdpVideoFormat& format) const {
+  CodecInfo codec_info;
+  codec_info.is_hardware_accelerated = true;
+  // Set internal source to true to directly provide encoded frames to webrtc.
+  codec_info.has_internal_source = true;
+  return codec_info;
 }
 
 webrtc::EncodedImageCallback::Result
@@ -306,5 +300,21 @@ void WebrtcDummyVideoEncoderFactory::SetVideoChannelStateObserver(
   video_channel_state_observer_ = video_channel_state_observer;
 }
 
+void WebrtcDummyVideoEncoderFactory::EncoderDestroyed(
+    WebrtcDummyVideoEncoder* encoder) {
+  base::AutoLock lock(lock_);
+  if (!encoder) {
+    LOG(ERROR) << "Attempting to destroy null encoder";
+    return;
+  }
+  for (auto pos = encoders_.begin(); pos != encoders_.end(); ++pos) {
+    if (*pos == encoder) {
+      encoders_.erase(pos);
+      return;
+    }
+  }
+  NOTREACHED() << "Asked to remove encoder not owned by factory.";
+}
+
 }  // namespace protocol
 }  // namespace remoting

remoting/protocol/webrtc_dummy_video_encoder.h

@@ -14,13 +14,14 @@
 #include "base/single_thread_task_runner.h"
 #include "base/synchronization/lock.h"
 #include "remoting/codec/webrtc_video_encoder.h"
-#include "third_party/webrtc/media/engine/webrtcvideoencoderfactory.h"
+#include "third_party/webrtc/api/video_codecs/video_encoder_factory.h"
 #include "third_party/webrtc/modules/video_coding/include/video_codec_interface.h"
 
 namespace remoting {
 namespace protocol {
 
 class VideoChannelStateObserver;
+class WebrtcDummyVideoEncoderFactory;
 
 // WebrtcDummyVideoEncoder implements webrtc::VideoEncoder interface for WebRTC.
 // It's responsible for getting  feedback on network bandwidth, latency & RTT
@@ -35,7 +36,8 @@ class WebrtcDummyVideoEncoder : public webrtc::VideoEncoder {
 
   WebrtcDummyVideoEncoder(
       scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
-      base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer);
+      base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer,
+      WebrtcDummyVideoEncoderFactory* factory);
   ~WebrtcDummyVideoEncoder() override;
 
   // webrtc::VideoEncoder overrides.
@@ -66,23 +68,26 @@ class WebrtcDummyVideoEncoder : public webrtc::VideoEncoder {
   webrtc::EncodedImageCallback* encoded_callback_ = nullptr;
 
   base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer_;
+
+  // Holds a reference to the creating factory, if any. Will be notified
+  // when this instance is released, so it can stop delivering frames.
+  WebrtcDummyVideoEncoderFactory* factory_ = nullptr;
 };
 
-// This is the external encoder factory implementation that is passed to
-// WebRTC at the time of creation of peer connection. The external encoder
-// factory primarily manages creation and destruction of encoder.
-class WebrtcDummyVideoEncoderFactory
-    : public cricket::WebRtcVideoEncoderFactory {
+// This is the encoder factory implementation that is passed to
+// WebRTC at the time of creation of peer connection. The encoder
+// factory primarily manages creation of encoder.
+class WebrtcDummyVideoEncoderFactory : public webrtc::VideoEncoderFactory {
  public:
   WebrtcDummyVideoEncoderFactory();
   ~WebrtcDummyVideoEncoderFactory() override;
 
-  // cricket::WebRtcVideoEncoderFactory interface.
-  webrtc::VideoEncoder* CreateVideoEncoder(
-      const cricket::VideoCodec& codec) override;
-  const std::vector<cricket::VideoCodec>& supported_codecs() const override;
-  bool EncoderTypeHasInternalSource(webrtc::VideoCodecType type) const override;
-  void DestroyVideoEncoder(webrtc::VideoEncoder* encoder) override;
+  // webrtc::VideoEncoderFactory interface.
+  std::unique_ptr<webrtc::VideoEncoder> CreateVideoEncoder(
+      const webrtc::SdpVideoFormat& format) override;
+  std::vector<webrtc::SdpVideoFormat> GetSupportedFormats() const override;
+  CodecInfo QueryVideoEncoder(
+      const webrtc::SdpVideoFormat& format) const override;
 
   webrtc::EncodedImageCallback::Result SendEncodedFrame(
       const WebrtcVideoEncoder::EncodedFrame& packet,
@@ -102,15 +107,17 @@ class WebrtcDummyVideoEncoderFactory
     return video_channel_state_observer_;
   }
 
+  void EncoderDestroyed(WebrtcDummyVideoEncoder* encoder);
+
  private:
   scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
 
-  std::vector<cricket::VideoCodec> codecs_;
+  std::vector<webrtc::SdpVideoFormat> formats_;
 
   // Protects |video_channel_state_observer_| and |encoders_|.
   base::Lock lock_;
   base::WeakPtr<VideoChannelStateObserver> video_channel_state_observer_;
-  std::vector<std::unique_ptr<WebrtcDummyVideoEncoder>> encoders_;
+  std::vector<WebrtcDummyVideoEncoder*> encoders_;
   base::Callback<void(webrtc::VideoCodecType)> encoder_created_callback_;
 };
 

remoting/protocol/webrtc_transport.cc

@@ -35,6 +35,7 @@
 #include "third_party/webrtc/api/audio_codecs/opus/audio_decoder_opus.h"
 #include "third_party/webrtc/api/audio_codecs/opus/audio_encoder_opus.h"
 #include "third_party/webrtc/api/stats/rtcstats_objects.h"
+#include "third_party/webrtc/api/video_codecs/builtin_video_decoder_factory.h"
 
 using buzz::QName;
 using buzz::XmlElement;
@@ -213,17 +214,22 @@ class WebrtcTransport::PeerConnectionWrapper
  public:
   PeerConnectionWrapper(
       rtc::Thread* worker_thread,
-      std::unique_ptr<cricket::WebRtcVideoEncoderFactory> encoder_factory,
+      std::unique_ptr<webrtc::VideoEncoderFactory> encoder_factory,
       std::unique_ptr<cricket::PortAllocator> port_allocator,
       base::WeakPtr<WebrtcTransport> transport)
       : transport_(transport) {
     audio_module_ = new rtc::RefCountedObject<WebrtcAudioModule>();
 
     peer_connection_factory_ = webrtc::CreatePeerConnectionFactory(
-        worker_thread, rtc::Thread::Current(), audio_module_.get(),
+        worker_thread,  // network_thread
+        worker_thread,
+        rtc::Thread::Current(),  // signaling_thread
+        audio_module_,
         webrtc::CreateAudioEncoderFactory<webrtc::AudioEncoderOpus>(),
         webrtc::CreateAudioDecoderFactory<webrtc::AudioDecoderOpus>(),
-        encoder_factory.release(), nullptr);
+        std::move(encoder_factory), webrtc::CreateBuiltinVideoDecoderFactory(),
+        nullptr,   // audio_mixer
+        nullptr);  // audio_processing
 
     webrtc::PeerConnectionInterface::RTCConfiguration rtc_config;
     rtc_config.enable_dtls_srtp = true;
@@ -305,7 +311,7 @@ class WebrtcTransport::PeerConnectionWrapper
   }
 
  private:
-  scoped_refptr<WebrtcAudioModule> audio_module_;
+  rtc::scoped_refptr<WebrtcAudioModule> audio_module_;
   scoped_refptr<webrtc::PeerConnectionFactoryInterface>
       peer_connection_factory_;
   scoped_refptr<webrtc::PeerConnectionInterface> peer_connection_;