Stop caching supported codecs in WebRtcVideoEngine2

webrtc/pc/channelmanager.cc

 /*
  *  Copyright 2004 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 11 matching lines @@
 #include "webrtc/media/base/device.h"
 #include "webrtc/media/base/hybriddataengine.h"
 #include "webrtc/media/base/rtpdataengine.h"
 #ifdef HAVE_SCTP
 #include "webrtc/media/sctp/sctpdataengine.h"
 #endif
 #include "webrtc/pc/srtpfilter.h"
 
 namespace cricket {
 
+static bool IsRtxCodec(const VideoCodec& codec) {
+  return _stricmp(kRtxCodecName, codec.name.c_str()) == 0;

[tommi, 2016/11/09 16:24:30]

in tests, we have case sensitive comparisons for this codec name. Outside of
tests we have stricmp in a couple of places (looks like you're copying one
here), but in SdpAudioFormat we actually have a case sensitive comparison when
we do lookups.

Can you check with Harald to see what is the right thing to be doing?  I suspect
case sensitive is what we want.  If so, you could replace IsRtxCodec with simply
codec.name.compare(kRtxCodecName) == 0

Btw, there are also macros for doing case sensitive and case insensitive string
compare.  See STR_NCASE_CMP.

[magjed_webrtc, 2016/11/09 20:37:53]

Ok, I will ask hta@ for advice. The code here is just moved from the existing
code at line 147, but it might be wrong.

Regarding std::string::compare, won't that create a temporary std::string out of
the kRtxCodecName (char*), which will hurt performance?

[tommi, 2016/11/09 20:59:50]

There's a version of compare() that takes a const char*, so the performance
should be the same as with strcmp.

[magjed_webrtc, 2016/11/09 21:15:10]

Ah cool, I missed that.

+}
 
 using rtc::Bind;
 
 static DataEngineInterface* ConstructDataEngine() {
 #ifdef HAVE_SCTP
   return new HybridDataEngine(new RtpDataEngine(), new SctpDataEngine());
 #else
   return new RtpDataEngine();
 #endif
 }
@@ skipping 88 matching lines @@
 void ChannelManager::GetSupportedAudioReceiveCodecs(
     std::vector<AudioCodec>* codecs) const {
   *codecs = media_engine_->audio_recv_codecs();
 }
 
 void ChannelManager::GetSupportedAudioRtpHeaderExtensions(
     RtpHeaderExtensions* ext) const {
   *ext = media_engine_->GetAudioCapabilities().header_extensions;
 }
 
-void ChannelManager::GetSupportedVideoCodecs(
-    std::vector<VideoCodec>* codecs) const {
-  codecs->clear();
-
-  std::vector<VideoCodec>::const_iterator it;
-  for (it = media_engine_->video_codecs().begin();
-      it != media_engine_->video_codecs().end(); ++it) {
-    if (!enable_rtx_ && _stricmp(kRtxCodecName, it->name.c_str()) == 0) {
-      continue;
-    }
-    codecs->push_back(*it);
+std::vector<VideoCodec> ChannelManager::GetSupportedVideoCodecs() const {
+  std::vector<VideoCodec> codecs = media_engine_->video_codecs();
+  if (!enable_rtx_) {
+    codecs.erase(std::remove_if(codecs.begin(), codecs.end(), &IsRtxCodec),
+                 codecs.end());
   }
+  return codecs;
 }
 
 void ChannelManager::GetSupportedVideoRtpHeaderExtensions(
     RtpHeaderExtensions* ext) const {
   *ext = media_engine_->GetVideoCapabilities().header_extensions;
 }
 
 void ChannelManager::GetSupportedDataCodecs(
     std::vector<DataCodec>* codecs) const {
   *codecs = data_media_engine_->data_codecs();
@@ skipping 249 matching lines @@
                           media_engine_.get(), file, max_size_bytes));
 }
 
 void ChannelManager::StopAecDump() {
   worker_thread_->Invoke<void>(
       RTC_FROM_HERE,
       Bind(&MediaEngineInterface::StopAecDump, media_engine_.get()));
 }
 
 }  // namespace cricket