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}
This commit is contained in:
@ -74,7 +74,7 @@ class MediaEngineInterface {
|
|||||||
virtual const std::vector<AudioCodec>& audio_send_codecs() = 0;
|
virtual const std::vector<AudioCodec>& audio_send_codecs() = 0;
|
||||||
virtual const std::vector<AudioCodec>& audio_recv_codecs() = 0;
|
virtual const std::vector<AudioCodec>& audio_recv_codecs() = 0;
|
||||||
virtual RtpCapabilities GetAudioCapabilities() = 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;
|
virtual RtpCapabilities GetVideoCapabilities() = 0;
|
||||||
|
|
||||||
// Starts AEC dump using existing file, a maximum file size in bytes can be
|
// Starts AEC dump using existing file, a maximum file size in bytes can be
|
||||||
@ -147,7 +147,7 @@ class CompositeMediaEngine : public MediaEngineInterface {
|
|||||||
virtual RtpCapabilities GetAudioCapabilities() {
|
virtual RtpCapabilities GetAudioCapabilities() {
|
||||||
return voice_.GetCapabilities();
|
return voice_.GetCapabilities();
|
||||||
}
|
}
|
||||||
virtual const std::vector<VideoCodec> video_codecs() {
|
virtual const std::vector<VideoCodec>& video_codecs() {
|
||||||
return video_.codecs();
|
return video_.codecs();
|
||||||
}
|
}
|
||||||
virtual RtpCapabilities GetVideoCapabilities() {
|
virtual RtpCapabilities GetVideoCapabilities() {
|
||||||
|
|||||||
@ -543,6 +543,7 @@ WebRtcVideoEngine2::WebRtcVideoEngine2()
|
|||||||
external_decoder_factory_(NULL),
|
external_decoder_factory_(NULL),
|
||||||
external_encoder_factory_(NULL) {
|
external_encoder_factory_(NULL) {
|
||||||
LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()";
|
LOG(LS_INFO) << "WebRtcVideoEngine2::WebRtcVideoEngine2()";
|
||||||
|
video_codecs_ = GetSupportedCodecs(external_encoder_factory_);
|
||||||
}
|
}
|
||||||
|
|
||||||
WebRtcVideoEngine2::~WebRtcVideoEngine2() {
|
WebRtcVideoEngine2::~WebRtcVideoEngine2() {
|
||||||
@ -565,8 +566,8 @@ WebRtcVideoChannel2* WebRtcVideoEngine2::CreateChannel(
|
|||||||
external_decoder_factory_);
|
external_decoder_factory_);
|
||||||
}
|
}
|
||||||
|
|
||||||
const std::vector<VideoCodec> WebRtcVideoEngine2::codecs() const {
|
const std::vector<VideoCodec>& WebRtcVideoEngine2::codecs() const {
|
||||||
return GetSupportedCodecs(external_encoder_factory_);
|
return video_codecs_;
|
||||||
}
|
}
|
||||||
|
|
||||||
RtpCapabilities WebRtcVideoEngine2::GetCapabilities() const {
|
RtpCapabilities WebRtcVideoEngine2::GetCapabilities() const {
|
||||||
@ -613,6 +614,8 @@ void WebRtcVideoEngine2::SetExternalEncoderFactory(
|
|||||||
encoder_factory = simulcast_encoder_factory_.get();
|
encoder_factory = simulcast_encoder_factory_.get();
|
||||||
}
|
}
|
||||||
external_encoder_factory_ = encoder_factory;
|
external_encoder_factory_ = encoder_factory;
|
||||||
|
|
||||||
|
video_codecs_ = GetSupportedCodecs(encoder_factory);
|
||||||
}
|
}
|
||||||
|
|
||||||
static std::vector<VideoCodec> GetSupportedCodecs(
|
static std::vector<VideoCodec> GetSupportedCodecs(
|
||||||
|
|||||||
@ -105,7 +105,7 @@ class WebRtcVideoEngine2 {
|
|||||||
const MediaConfig& config,
|
const MediaConfig& config,
|
||||||
const VideoOptions& options);
|
const VideoOptions& options);
|
||||||
|
|
||||||
const std::vector<VideoCodec> codecs() const;
|
const std::vector<VideoCodec>& codecs() const;
|
||||||
RtpCapabilities GetCapabilities() const;
|
RtpCapabilities GetCapabilities() const;
|
||||||
|
|
||||||
// Set a WebRtcVideoDecoderFactory for external decoding. Video engine does
|
// Set a WebRtcVideoDecoderFactory for external decoding. Video engine does
|
||||||
@ -119,6 +119,8 @@ class WebRtcVideoEngine2 {
|
|||||||
WebRtcVideoEncoderFactory* encoder_factory);
|
WebRtcVideoEncoderFactory* encoder_factory);
|
||||||
|
|
||||||
private:
|
private:
|
||||||
|
std::vector<VideoCodec> video_codecs_;
|
||||||
|
|
||||||
bool initialized_;
|
bool initialized_;
|
||||||
|
|
||||||
WebRtcVideoDecoderFactory* external_decoder_factory_;
|
WebRtcVideoDecoderFactory* external_decoder_factory_;
|
||||||
|
|||||||
@ -29,9 +29,6 @@
|
|||||||
|
|
||||||
namespace cricket {
|
namespace cricket {
|
||||||
|
|
||||||
static bool IsRtxCodec(const VideoCodec& codec) {
|
|
||||||
return _stricmp(kRtxCodecName, codec.name.c_str()) == 0;
|
|
||||||
}
|
|
||||||
|
|
||||||
using rtc::Bind;
|
using rtc::Bind;
|
||||||
|
|
||||||
@ -140,13 +137,18 @@ void ChannelManager::GetSupportedAudioRtpHeaderExtensions(
|
|||||||
*ext = media_engine_->GetAudioCapabilities().header_extensions;
|
*ext = media_engine_->GetAudioCapabilities().header_extensions;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::vector<VideoCodec> ChannelManager::GetSupportedVideoCodecs() const {
|
void ChannelManager::GetSupportedVideoCodecs(
|
||||||
std::vector<VideoCodec> codecs = media_engine_->video_codecs();
|
std::vector<VideoCodec>* codecs) const {
|
||||||
if (!enable_rtx_) {
|
codecs->clear();
|
||||||
codecs.erase(std::remove_if(codecs.begin(), codecs.end(), &IsRtxCodec),
|
|
||||||
codecs.end());
|
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(
|
void ChannelManager::GetSupportedVideoRtpHeaderExtensions(
|
||||||
|
|||||||
@ -75,7 +75,7 @@ class ChannelManager {
|
|||||||
void GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
|
void GetSupportedAudioSendCodecs(std::vector<AudioCodec>* codecs) const;
|
||||||
void GetSupportedAudioReceiveCodecs(std::vector<AudioCodec>* codecs) const;
|
void GetSupportedAudioReceiveCodecs(std::vector<AudioCodec>* codecs) const;
|
||||||
void GetSupportedAudioRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
|
void GetSupportedAudioRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
|
||||||
std::vector<VideoCodec> GetSupportedVideoCodecs() const;
|
void GetSupportedVideoCodecs(std::vector<VideoCodec>* codecs) const;
|
||||||
void GetSupportedVideoRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
|
void GetSupportedVideoRtpHeaderExtensions(RtpHeaderExtensions* ext) const;
|
||||||
void GetSupportedDataCodecs(std::vector<DataCodec>* codecs) const;
|
void GetSupportedDataCodecs(std::vector<DataCodec>* codecs) const;
|
||||||
|
|
||||||
|
|||||||
@ -173,17 +173,17 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) {
|
|||||||
const VideoCodec rtx_codec(96, "rtx");
|
const VideoCodec rtx_codec(96, "rtx");
|
||||||
|
|
||||||
// By default RTX is disabled.
|
// By default RTX is disabled.
|
||||||
codecs = cm_->GetSupportedVideoCodecs();
|
cm_->GetSupportedVideoCodecs(&codecs);
|
||||||
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
|
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
|
||||||
|
|
||||||
// Enable and check.
|
// Enable and check.
|
||||||
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
|
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
|
||||||
codecs = cm_->GetSupportedVideoCodecs();
|
cm_->GetSupportedVideoCodecs(&codecs);
|
||||||
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
|
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
|
||||||
|
|
||||||
// Disable and check.
|
// Disable and check.
|
||||||
EXPECT_TRUE(cm_->SetVideoRtxEnabled(false));
|
EXPECT_TRUE(cm_->SetVideoRtxEnabled(false));
|
||||||
codecs = cm_->GetSupportedVideoCodecs();
|
cm_->GetSupportedVideoCodecs(&codecs);
|
||||||
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
|
EXPECT_FALSE(ContainsMatchingCodec(codecs, rtx_codec));
|
||||||
|
|
||||||
// Cannot toggle rtx after initialization.
|
// Cannot toggle rtx after initialization.
|
||||||
@ -194,7 +194,7 @@ TEST_F(ChannelManagerTest, SetVideoRtxEnabled) {
|
|||||||
// Can set again after terminate.
|
// Can set again after terminate.
|
||||||
cm_->Terminate();
|
cm_->Terminate();
|
||||||
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
|
EXPECT_TRUE(cm_->SetVideoRtxEnabled(true));
|
||||||
codecs = cm_->GetSupportedVideoCodecs();
|
cm_->GetSupportedVideoCodecs(&codecs);
|
||||||
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
|
EXPECT_TRUE(ContainsMatchingCodec(codecs, rtx_codec));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -1274,7 +1274,7 @@ MediaSessionDescriptionFactory::MediaSessionDescriptionFactory(
|
|||||||
channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_);
|
channel_manager->GetSupportedAudioReceiveCodecs(&audio_recv_codecs_);
|
||||||
channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_);
|
channel_manager->GetSupportedAudioSendCodecs(&audio_send_codecs_);
|
||||||
channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_);
|
channel_manager->GetSupportedAudioRtpHeaderExtensions(&audio_rtp_extensions_);
|
||||||
video_codecs_ = channel_manager->GetSupportedVideoCodecs();
|
channel_manager->GetSupportedVideoCodecs(&video_codecs_);
|
||||||
channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_);
|
channel_manager->GetSupportedVideoRtpHeaderExtensions(&video_rtp_extensions_);
|
||||||
channel_manager->GetSupportedDataCodecs(&data_codecs_);
|
channel_manager->GetSupportedDataCodecs(&data_codecs_);
|
||||||
NegotiateCodecs(audio_recv_codecs_, audio_send_codecs_,
|
NegotiateCodecs(audio_recv_codecs_, audio_send_codecs_,
|
||||||
|
|||||||
Reference in New Issue
Block a user