Revert of Stop caching supported codecs in WebRtcVideoEngine2 (patchset #1 id:1 of https://codereview.webrtc.org/2492473002/ )
Reason for revert:
This CL probably broke Chromium FYI.
Original issue's description:
> Stop caching supported codecs in WebRtcVideoEngine2
>
> We currently cache the result of GetSupportedCodecs in a member variable
> |video_codecs_| in WebRtcVideoEngine2. This means we need to keep
> |video_codecs_| and the result of GetSupportedCodecs in sync, which is
> error prone. It's simpler to just call GetSupportedCodecs when we need
> it, and we actually end up making fewer calls, so it's faster as well.
> This CL also returns all std::vectors by-value instead of by-ref. Move
> semantic together with in-place filtering of codecs actually end up with
> fewer copies, and it's also simpler to not return references.
>
> BUG=webrtc:6337
>
> Committed: https://crrev.com/9f71ec5a3e3175751f4475b126cfda89767363f2
> Cr-Commit-Position: refs/heads/master@{#15007}
TBR=tommi@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6337
Review-Url: https://codereview.webrtc.org/2489173004
Cr-Commit-Position: refs/heads/master@{#15014}
diff --git a/webrtc/media/base/mediaengine.h b/webrtc/media/base/mediaengine.h
index edf0cb0..d1d1623 100644
--- a/webrtc/media/base/mediaengine.h
+++ b/webrtc/media/base/mediaengine.h
@@ -74,7 +74,7 @@
virtual const std::vector<AudioCodec>& audio_send_codecs() = 0;
virtual const std::vector<AudioCodec>& audio_recv_codecs() = 0;
virtual RtpCapabilities GetAudioCapabilities() = 0;
- virtual const std::vector<VideoCodec> video_codecs() = 0;
+ virtual const std::vector<VideoCodec>& video_codecs() = 0;
virtual RtpCapabilities GetVideoCapabilities() = 0;
// Starts AEC dump using existing file, a maximum file size in bytes can be
@@ -147,7 +147,7 @@
virtual RtpCapabilities GetAudioCapabilities() {
return voice_.GetCapabilities();
}
- virtual const std::vector<VideoCodec> video_codecs() {
+ virtual const std::vector<VideoCodec>& video_codecs() {
return video_.codecs();
}
virtual RtpCapabilities GetVideoCapabilities() {
diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc
index dfdae5e..3cacc7d 100644
--- a/webrtc/media/engine/webrtcvideoengine2.cc
+++ b/webrtc/media/engine/webrtcvideoengine2.cc
@@ -543,6 +543,7 @@
external_decoder_factory_(NULL),
external_encoder_factory_(NULL) {
LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()";
+ video_codecs_ = GetSupportedCodecs(external_encoder_factory_);
}
WebRtcVideoEngine2::~WebRtcVideoEngine2() {
@@ -565,8 +566,8 @@
external_decoder_factory_);
}
-const std::vector<VideoCodec> WebRtcVideoEngine2::codecs() const {
- return GetSupportedCodecs(external_encoder_factory_);
+const std::vector<VideoCodec>& WebRtcVideoEngine2::codecs() const {
+ return video_codecs_;
}
RtpCapabilities WebRtcVideoEngine2::GetCapabilities() const {
@@ -613,6 +614,8 @@
encoder_factory = simulcast_encoder_factory_.get();
}
external_encoder_factory_ = encoder_factory;
+
+ video_codecs_ = GetSupportedCodecs(encoder_factory);
}
static std::vector<VideoCodec> GetSupportedCodecs(
diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h
index 1032e9c..ef893d2 100644
--- a/webrtc/media/engine/webrtcvideoengine2.h
+++ b/webrtc/media/engine/webrtcvideoengine2.h
@@ -105,7 +105,7 @@
const MediaConfig& config,
const VideoOptions& options);
- const std::vector<VideoCodec> codecs() const;
+ const std::vector<VideoCodec>& codecs() const;
RtpCapabilities GetCapabilities() const;
// Set a WebRtcVideoDecoderFactory for external decoding. Video engine does
@@ -119,6 +119,8 @@
WebRtcVideoEncoderFactory* encoder_factory);
private:
+ std::vector<VideoCodec> video_codecs_;
+
bool initialized_;
WebRtcVideoDecoderFactory* external_decoder_factory_;
diff --git a/webrtc/pc/channelmanager.cc b/webrtc/pc/channelmanager.cc
index fb6f44f..06475e9 100644
--- a/webrtc/pc/channelmanager.cc
+++ b/webrtc/pc/channelmanager.cc
@@ -29,9 +29,6 @@
namespace cricket {
-static bool IsRtxCodec(const VideoCodec& codec) {
- return _stricmp(kRtxCodecName, codec.name.c_str()) == 0;
-}
using rtc::Bind;
@@ -140,13 +137,18 @@
*ext = media_engine_->GetAudioCapabilities().header_extensions;
}
-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());
+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);
}
- return codecs;
}
void ChannelManager::GetSupportedVideoRtpHeaderExtensions(
diff --git a/webrtc/pc/channelmanager.h b/webrtc/pc/channelmanager.h
index 68b1ab2..15a3752 100644
--- a/webrtc/pc/channelmanager.h
+++ b/webrtc/pc/channelmanager.h
@@ -75,7 +75,7 @@
void GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
void GetSupportedAudioReceiveCodecs(std::vector<AudioCodec>* codecs) const;
void GetSupportedAudioRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
- std::vector<VideoCodec> GetSupportedVideoCodecs() const;
+ void GetSupportedVideoCodecs(std::vector<VideoCodec>* codecs) const;
void GetSupportedVideoRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
void GetSupportedDataCodecs(std::vector<DataCodec>* codecs) const;
diff --git a/webrtc/pc/channelmanager_unittest.cc b/webrtc/pc/channelmanager_unittest.cc
index 34b347b..174d064 100644
--- a/webrtc/pc/channelmanager_unittest.cc
+++ b/webrtc/pc/channelmanager_unittest.cc
@@ -173,17 +173,17 @@
const VideoCodec rtx_codec(96, "rtx");
// By default RTX is disabled.
- codecs = cm_->GetSupportedVideoCodecs();
+ cm_->GetSupportedVideoCodecs(&codecs);
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
// Enable and check.
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
- codecs = cm_->GetSupportedVideoCodecs();
+ cm_->GetSupportedVideoCodecs(&codecs);
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
// Disable and check.
EXPECT_TRUE(cm_->SetVideoRtxEnabled(false));
- codecs = cm_->GetSupportedVideoCodecs();
+ cm_->GetSupportedVideoCodecs(&codecs);
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
// Cannot toggle rtx after initialization.
@@ -194,7 +194,7 @@
// Can set again after terminate.
cm_->Terminate();
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
- codecs = cm_->GetSupportedVideoCodecs();
+ cm_->GetSupportedVideoCodecs(&codecs);
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
}
diff --git a/webrtc/pc/mediasession.cc b/webrtc/pc/mediasession.cc
index 0ebc766..ebba9ac 100644
--- a/webrtc/pc/mediasession.cc
+++ b/webrtc/pc/mediasession.cc
@@ -1274,7 +1274,7 @@
channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_);
channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_);
channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_);
- video_codecs_ = channel_manager->GetSupportedVideoCodecs();
+ channel_manager->GetSupportedVideoCodecs(&video_codecs_);
channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_);
channel_manager->GetSupportedDataCodecs(&data_codecs_);
NegotiateCodecs(audio_recv_codecs_, audio_send_codecs_,
