Base decision to send REMB on send codecs.
Fixes bug where Chromium would send REMB even though the remote party doesn't announce support for it (because it was based on local codec settings instead of remote ones). BUG=4626 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/54389004 Cr-Commit-Position: refs/heads/master@{#9170}
This commit is contained in:
@ -937,12 +937,13 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
|
||||
send_codec_.Set(supported_codecs.front());
|
||||
|
||||
rtc::CritScope stream_lock(&stream_crit_);
|
||||
for (std::map<uint32, WebRtcVideoSendStream*>::iterator it =
|
||||
send_streams_.begin();
|
||||
it != send_streams_.end();
|
||||
++it) {
|
||||
DCHECK(it->second != NULL);
|
||||
it->second->SetCodec(supported_codecs.front());
|
||||
for (auto& kv : send_streams_) {
|
||||
DCHECK(kv.second != nullptr);
|
||||
kv.second->SetCodec(supported_codecs.front());
|
||||
}
|
||||
for (auto& kv : receive_streams_) {
|
||||
DCHECK(kv.second != nullptr);
|
||||
kv.second->SetRemb(HasRemb(supported_codecs.front().codec));
|
||||
}
|
||||
|
||||
// TODO(holmer): Changing the codec parameters shouldn't necessarily mean that
|
||||
@ -1170,6 +1171,12 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp,
|
||||
config.audio_channel_id = voice_channel_id_;
|
||||
}
|
||||
|
||||
config.rtp.remb = false;
|
||||
VideoCodecSettings send_codec;
|
||||
if (send_codec_.Get(&send_codec)) {
|
||||
config.rtp.remb = HasRemb(send_codec.codec);
|
||||
}
|
||||
|
||||
receive_streams_[ssrc] = new WebRtcVideoReceiveStream(
|
||||
call_.get(), sp.ssrcs, external_decoder_factory_, default_stream, config,
|
||||
recv_codecs_);
|
||||
@ -2286,12 +2293,18 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs(
|
||||
config_.rtp.fec = recv_codecs.front().fec;
|
||||
config_.rtp.nack.rtp_history_ms =
|
||||
HasNack(recv_codecs.begin()->codec) ? kNackHistoryMs : 0;
|
||||
config_.rtp.remb = HasRemb(recv_codecs.begin()->codec);
|
||||
|
||||
ClearDecoders(&old_decoders);
|
||||
RecreateWebRtcStream();
|
||||
}
|
||||
|
||||
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRemb(bool enabled) {
|
||||
if (config_.rtp.remb == enabled)
|
||||
return;
|
||||
config_.rtp.remb = enabled;
|
||||
RecreateWebRtcStream();
|
||||
}
|
||||
|
||||
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRtpExtensions(
|
||||
const std::vector<webrtc::RtpExtension>& extensions) {
|
||||
config_.rtp.extensions = extensions;
|
||||
|
@ -412,6 +412,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
|
||||
|
||||
const std::vector<uint32>& GetSsrcs() const;
|
||||
|
||||
void SetRemb(bool enabled);
|
||||
void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
|
||||
void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);
|
||||
|
||||
|
@ -1277,17 +1277,17 @@ TEST_F(WebRtcVideoChannel2Test, RembCanBeEnabledAndDisabled) {
|
||||
FakeVideoReceiveStream* stream = AddRecvStream();
|
||||
EXPECT_TRUE(stream->GetConfig().rtp.remb);
|
||||
|
||||
// Verify that REMB is turned off when codecs without REMB are set.
|
||||
// Verify that REMB is turned off when send(!) codecs without REMB are set.
|
||||
std::vector<VideoCodec> codecs;
|
||||
codecs.push_back(kVp8Codec);
|
||||
EXPECT_TRUE(codecs[0].feedback_params.params().empty());
|
||||
EXPECT_TRUE(channel_->SetRecvCodecs(codecs));
|
||||
EXPECT_TRUE(channel_->SetSendCodecs(codecs));
|
||||
stream = fake_call_->GetVideoReceiveStreams()[0];
|
||||
EXPECT_FALSE(stream->GetConfig().rtp.remb);
|
||||
|
||||
// Verify that REMB is turned on when setting default codecs since the
|
||||
// default codecs have REMB enabled.
|
||||
EXPECT_TRUE(channel_->SetRecvCodecs(engine_.codecs()));
|
||||
EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
|
||||
stream = fake_call_->GetVideoReceiveStreams()[0];
|
||||
EXPECT_TRUE(stream->GetConfig().rtp.remb);
|
||||
}
|
||||
|
Reference in New Issue
Block a user