Replace WebRtcVideoEncoderFactory::VideoCodec with cricket::VideoCodec

webrtc/media/engine/webrtcvideoengine2.cc

 /*
  *  Copyright (c) 2014 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 31 matching lines @@
 class EncoderFactoryAdapter : public webrtc::VideoEncoderFactory {
  public:
   // EncoderFactoryAdapter doesn't take ownership of |factory|, which is owned
   // by e.g. PeerConnectionFactory.
   explicit EncoderFactoryAdapter(cricket::WebRtcVideoEncoderFactory* factory)
       : factory_(factory) {}
   virtual ~EncoderFactoryAdapter() {}
 
   // Implement webrtc::VideoEncoderFactory.
   webrtc::VideoEncoder* Create() override {
-    return factory_->CreateVideoEncoder(webrtc::kVideoCodecVP8);
+    return factory_->CreateVideoEncoder(VideoCodec(kVp8CodecName));
   }
 
   void Destroy(webrtc::VideoEncoder* encoder) override {
     return factory_->DestroyVideoEncoder(encoder);
   }
 
  private:
   cricket::WebRtcVideoEncoderFactory* const factory_;
 };
 
@@ skipping 29 matching lines @@
 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<VideoCodec>& codecs) {
+      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 (codec.type == webrtc::kVideoCodecVP8) {
+      if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
         return true;
       }
     }
     return false;
   }
 
   webrtc::VideoEncoder* CreateVideoEncoder(
-      webrtc::VideoCodecType type) override {
+      const cricket::VideoCodec& codec) override {
     RTC_DCHECK(factory_ != NULL);
     // If it's a codec type we can simulcast, create a wrapped encoder.
-    if (type == webrtc::kVideoCodecVP8) {
+    if (CodecNamesEq(codec.name.c_str(), kVp8CodecName)) {
       return new webrtc::SimulcastEncoderAdapter(
           new EncoderFactoryAdapter(factory_));
     }
-    webrtc::VideoEncoder* encoder = factory_->CreateVideoEncoder(type);
+    webrtc::VideoEncoder* encoder = factory_->CreateVideoEncoder(codec);
     if (encoder) {
       non_simulcast_encoders_.push_back(encoder);
     }
     return encoder;
   }
 
-  const std::vector<VideoCodec>& codecs() const override {
-    return factory_->codecs();
+  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_;
 };
 
-bool CodecIsInternallySupported(const std::string& codec_name) {
-  if (CodecNamesEq(codec_name, kVp8CodecName)) {
-    return true;
-  }
-  if (CodecNamesEq(codec_name, kVp9CodecName)) {
-    return webrtc::VP9Encoder::IsSupported() &&
-           webrtc::VP9Decoder::IsSupported();
-  }
-  if (CodecNamesEq(codec_name, kH264CodecName)) {
-    return webrtc::H264Encoder::IsSupported() &&
-        webrtc::H264Decoder::IsSupported();
-  }
-  return false;
-}
-
 void AddDefaultFeedbackParams(VideoCodec* codec) {
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamCcm, kRtcpFbCcmParamFir));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kParamValueEmpty));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamNack, kRtcpFbNackParamPli));
   codec->AddFeedbackParam(FeedbackParam(kRtcpFbParamRemb, kParamValueEmpty));
   codec->AddFeedbackParam(
       FeedbackParam(kRtcpFbParamTransportCc, kParamValueEmpty));
 }
 
 static VideoCodec MakeVideoCodecWithDefaultFeedbackParams(int payload_type,
@@ skipping 252 matching lines @@
     return;
   }
   codecs->push_back(VideoCodec::CreateRtxCodec(rtx_payload_type, codec.id));
 }
 
 std::vector<VideoCodec> DefaultVideoCodecList() {
   std::vector<VideoCodec> codecs;
   AddCodecAndMaybeRtxCodec(
       MakeVideoCodecWithDefaultFeedbackParams(kDefaultVp8PlType, kVp8CodecName),
       &codecs);
-  if (CodecIsInternallySupported(kVp9CodecName)) {
+  if (webrtc::VP9Encoder::IsSupported() && webrtc::VP9Decoder::IsSupported()) {
     AddCodecAndMaybeRtxCodec(MakeVideoCodecWithDefaultFeedbackParams(
                                  kDefaultVp9PlType, kVp9CodecName),
                              &codecs);
   }
-  if (CodecIsInternallySupported(kH264CodecName)) {
+  if (webrtc::H264Encoder::IsSupported() &&
+      webrtc::H264Decoder::IsSupported()) {
     VideoCodec codec = MakeVideoCodecWithDefaultFeedbackParams(
         kDefaultH264PlType, kH264CodecName);
     // TODO(hta): Move all parameter generation for SDP into the codec
     // implementation, for all codecs and parameters.
     // TODO(hta): Move selection of profile-level-id to H.264 codec
     // implementation.
     // TODO(hta): Set FMTP parameters for all codecs of type H264.
     codec.SetParam(kH264FmtpProfileLevelId,
                    kH264ProfileLevelConstrainedBaseline);
     codec.SetParam(kH264FmtpLevelAsymmetryAllowed, "1");
     codec.SetParam(kH264FmtpPacketizationMode, "1");
     AddCodecAndMaybeRtxCodec(codec, &codecs);
   }
   AddCodecAndMaybeRtxCodec(VideoCodec(kDefaultRedPlType, kRedCodecName),
                            &codecs);
   codecs.push_back(VideoCodec(kDefaultUlpfecType, kUlpfecCodecName));
   return codecs;
 }
 
+static std::vector<VideoCodec> GetSupportedCodecs(
+    const WebRtcVideoEncoderFactory* external_encoder_factory);
+
 rtc::scoped_refptr<webrtc::VideoEncoderConfig::EncoderSpecificSettings>
 WebRtcVideoChannel2::WebRtcVideoSendStream::ConfigureVideoEncoderSettings(
     const VideoCodec& codec) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   bool is_screencast = parameters_.options.is_screencast.value_or(false);
   // No automatic resizing when using simulcast or screencast.
   bool automatic_resize =
       !is_screencast && parameters_.config.rtp.ssrcs.size() == 1;
   bool frame_dropping = !is_screencast;
   bool denoising;
@@ skipping 77 matching lines @@
   if (default_recv_ssrc_ != 0) {
     channel->SetSink(default_recv_ssrc_, default_sink_);
   }
 }
 
 WebRtcVideoEngine2::WebRtcVideoEngine2()
     : initialized_(false),
       external_decoder_factory_(NULL),
       external_encoder_factory_(NULL) {
   LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()";
-  video_codecs_ = GetSupportedCodecs();
+  video_codecs_ = GetSupportedCodecs(external_encoder_factory_);
 }
 
 WebRtcVideoEngine2::~WebRtcVideoEngine2() {
   LOG(LS_INFO) << "WebRtcVideoEngine2::~WebRtcVideoEngine2";
 }
 
 void WebRtcVideoEngine2::Init() {
   LOG(LS_INFO) << "WebRtcVideoEngine2::Init";
   initialized_ = true;
 }
@@ skipping 44 matching lines @@
   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->codecs())) {
+          encoder_factory->supported_codecs())) {
     simulcast_encoder_factory_.reset(
         new WebRtcSimulcastEncoderFactory(encoder_factory));
     encoder_factory = simulcast_encoder_factory_.get();
   }
   external_encoder_factory_ = encoder_factory;
 
-  video_codecs_ = GetSupportedCodecs();
+  video_codecs_ = GetSupportedCodecs(encoder_factory);
 }
 
-std::vector<VideoCodec> WebRtcVideoEngine2::GetSupportedCodecs() const {
+static std::vector<VideoCodec> GetSupportedCodecs(
+    const WebRtcVideoEncoderFactory* external_encoder_factory) {
   std::vector<VideoCodec> supported_codecs = DefaultVideoCodecList();
 
-  if (external_encoder_factory_ == NULL) {
+  if (external_encoder_factory == nullptr) {
     LOG(LS_INFO) << "Supported codecs: "
                  << CodecVectorToString(supported_codecs);
     return supported_codecs;
   }
 
   std::stringstream out;
-  const std::vector<WebRtcVideoEncoderFactory::VideoCodec>& codecs =
-      external_encoder_factory_->codecs();
+  const std::vector<VideoCodec>& codecs =
+      external_encoder_factory->supported_codecs();
   for (size_t i = 0; i < codecs.size(); ++i) {
-    out << codecs[i].name;
+    VideoCodec codec = codecs[i];
+    out << codec.name;
     if (i != codecs.size() - 1) {
       out << ", ";
     }
     // Don't add internally-supported codecs twice.
-    if (CodecIsInternallySupported(codecs[i].name)) {
+    bool is_codec_already_added = false;
+    for (const VideoCodec& supported_codec : supported_codecs) {
+      if (codec.Matches(supported_codec)) {
+        is_codec_already_added = true;
+        break;
+      }
+    }

[kthelgason, 2016/10/28 08:26:54]

This code exists in a few places. We could use std::any_of here to avoid
manually writing the loop, and to make the intent more clear.

[magjed_webrtc, 2016/10/28 10:27:31]

Thanks! Much better to extract it to a function. I went with std::any_of, but it
might not be more clear than a regular for-loop. It's this:
return std::any_of(supported_codecs.begin(), supported_codecs.end(),
                   [&codec](const VideoCodec& supported_codec) -> bool {
                     return supported_codec.Matches(codec);
                   });
vs this:
for (const VideoCodec& supported_codec : supported_codecs) {
  if (supported_codec.Matches(codec))
    return true;
}
return false;

+    if (is_codec_already_added) {
       continue;
     }
 
     // External video encoders are given payloads 120-127. This also means that
     // we only support up to 8 external payload types.
     // TODO(deadbeef): mediasession.cc already has code to dynamically
     // determine a payload type. We should be able to just leave the payload
     // type empty and let mediasession determine it. However, currently RTX
     // codecs are associated to codecs by payload type, meaning we DO need
     // to allocate unique payload types here. So to make this change we would
     // need to make RTX codecs associated by name instead.
     const int kExternalVideoPayloadTypeBase = 120;
     size_t payload_type = kExternalVideoPayloadTypeBase + i;
     RTC_DCHECK(payload_type < 128);
-    VideoCodec codec(static_cast<int>(payload_type), codecs[i].name);
+    codec.id = payload_type;
 
     AddDefaultFeedbackParams(&codec);
     AddCodecAndMaybeRtxCodec(codec, &supported_codecs);
   }
   LOG(LS_INFO) << "Supported codecs (incl. external codecs): "
                << CodecVectorToString(supported_codecs);
   LOG(LS_INFO) << "Codecs supported by the external encoder factory: "
                << out.str();
   return supported_codecs;
 }
@@ skipping 22 matching lines @@
   recv_codecs_ = FilterSupportedCodecs(MapCodecs(recv_codecs));
 }
 
 WebRtcVideoChannel2::~WebRtcVideoChannel2() {
   for (auto& kv : send_streams_)
     delete kv.second;
   for (auto& kv : receive_streams_)
     delete kv.second;
 }
 
-bool WebRtcVideoChannel2::CodecIsExternallySupported(
-    const std::string& name) const {
-  if (external_encoder_factory_ == NULL) {
-    return false;
-  }
-
-  const std::vector<WebRtcVideoEncoderFactory::VideoCodec> external_codecs =
-      external_encoder_factory_->codecs();
-  for (size_t c = 0; c < external_codecs.size(); ++c) {
-    if (CodecNamesEq(name, external_codecs[c].name)) {
-      return true;
-    }
-  }
-  return false;
-}
-
 std::vector<WebRtcVideoChannel2::VideoCodecSettings>
 WebRtcVideoChannel2::FilterSupportedCodecs(
-    const std::vector<WebRtcVideoChannel2::VideoCodecSettings>& mapped_codecs)
-    const {
-  std::vector<VideoCodecSettings> supported_codecs;
-  for (size_t i = 0; i < mapped_codecs.size(); ++i) {
-    const VideoCodecSettings& codec = mapped_codecs[i];
-    if (CodecIsInternallySupported(codec.codec.name) ||
-        CodecIsExternallySupported(codec.codec.name)) {
-      supported_codecs.push_back(codec);
+    const std::vector<VideoCodecSettings>& mapped_codecs) const {
+  const std::vector<VideoCodec> supported_codecs =
+      GetSupportedCodecs(external_encoder_factory_);
+  std::vector<VideoCodecSettings> filtered_codecs;
+  for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
+    for (const VideoCodec& supported_codec : supported_codecs) {
+      if (mapped_codec.codec.Matches(supported_codec)) {
+        filtered_codecs.push_back(mapped_codec);
+        break;
+      }
     }
   }

[kthelgason, 2016/10/28 08:26:54]

The inner loop here is identical to a previous one in this class. Perhaps we can
extract that out into a function, perhaps "bool IsCodecSupported(VideoCodec
codec)" and reuse that here. Then this whole thing could be a single
std::copy_if call.

[magjed_webrtc, 2016/10/28 10:27:31]

Done, the inner loop is now extracted out into a IsCodecSupported helper
function. I didn't go with std::copy_if, because I think it adds more noise than
clarity. It's this:
  std::copy_if(
      mapped_codecs.begin(), mapped_codecs.end(),
      std::back_inserter(filtered_codecs),
      [&supported_codecs](const VideoCodecSettings& mapped_codec) -> bool {
        return IsCodecSupported(supported_codecs, mapped_codec.codec);
      });
vs
  for (const VideoCodecSettings& mapped_codec : mapped_codecs) {
    if (IsCodecSupported(supported_codecs, mapped_codec.codec))
      filtered_codecs.push_back(mapped_codec);
  }

[kthelgason, 2016/10/28 13:19:45]

I agree with your assessment that this adds more noise than clarity. I had
imagined something like:
std::copy_if(mapped_codecs.begin(), mapped_codecs.end(),
    std::back_inserter(filtered_codecs), &IsCodecSupported)

but this is not javascript, and that obviously does not work :)

[magjed_webrtc, 2016/10/28 13:30:17]

Yeah, it's close to working with:
std::copy_if(mapped_codecs.begin(), mapped_codecs.end(),
    std::back_inserter(filtered_codecs),
    std::bind(&IsCodecSupported, supported_codecs));
Not quite though, because:
 1. std::bind is not yet an allowed C++11 feature in WebRTC.
 2. mapped_codec is of type VideoCodecSettings and not VideoCodec, so I need to
do mapped_codec.codec which essentially requires a lambda in C++.
Haskell has much nicer and cleaner glue functions :)

-  return supported_codecs;
+  return filtered_codecs;
 }
 
 bool WebRtcVideoChannel2::ReceiveCodecsHaveChanged(
     std::vector<VideoCodecSettings> before,
     std::vector<VideoCodecSettings> after) {
   if (before.size() != after.size()) {
     return true;
   }
   // The receive codec order doesn't matter, so we sort the codecs before
   // comparing. This is necessary because currently the
@@ skipping 1005 matching lines @@
   // the source is changed since the next source might be screen capture
   // with another resolution and frame rate.
   cpu_restricted_counter_ = 0;
 }
 
 const std::vector<uint32_t>&
 WebRtcVideoChannel2::WebRtcVideoSendStream::GetSsrcs() const {
   return ssrcs_;
 }
 
-webrtc::VideoCodecType CodecTypeFromName(const std::string& name) {
-  if (CodecNamesEq(name, kVp8CodecName)) {
-    return webrtc::kVideoCodecVP8;
-  } else if (CodecNamesEq(name, kVp9CodecName)) {
-    return webrtc::kVideoCodecVP9;
-  } else if (CodecNamesEq(name, kH264CodecName)) {
-    return webrtc::kVideoCodecH264;
-  }
-  return webrtc::kVideoCodecUnknown;
-}
-
 WebRtcVideoChannel2::WebRtcVideoSendStream::AllocatedEncoder
 WebRtcVideoChannel2::WebRtcVideoSendStream::CreateVideoEncoder(
     const VideoCodec& codec) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   webrtc::VideoCodecType type = CodecTypeFromName(codec.name);
 
   // Do not re-create encoders of the same type.
   if (type == allocated_encoder_.type && allocated_encoder_.encoder != NULL) {
     return allocated_encoder_;
   }
 
   if (external_encoder_factory_ != NULL) {
     webrtc::VideoEncoder* encoder =
-        external_encoder_factory_->CreateVideoEncoder(type);
+        external_encoder_factory_->CreateVideoEncoder(codec);
     if (encoder != NULL) {
       return AllocatedEncoder(encoder, type, true);
     }
   }
 
   if (type == webrtc::kVideoCodecVP8) {
     return AllocatedEncoder(
         webrtc::VideoEncoder::Create(webrtc::VideoEncoder::kVp8), type, false);
   } else if (type == webrtc::kVideoCodecVP9) {
     return AllocatedEncoder(
@@ skipping 880 matching lines @@
         rtx_mapping[video_codecs[i].codec.id] !=
             ulpfec_config.red_payload_type) {
       video_codecs[i].rtx_payload_type = rtx_mapping[video_codecs[i].codec.id];
     }
   }
 
   return video_codecs;
 }
 
 }  // namespace cricket