diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 8cbe029b0d..0c69ec581e 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -943,7 +943,8 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector& codecs) { } for (auto& kv : receive_streams_) { DCHECK(kv.second != nullptr); - kv.second->SetRemb(HasRemb(supported_codecs.front().codec)); + kv.second->SetNackAndRemb(HasNack(supported_codecs.front().codec), + HasRemb(supported_codecs.front().codec)); } // TODO(holmer): Changing the codec parameters shouldn't necessarily mean that @@ -1920,9 +1921,8 @@ void WebRtcVideoChannel2::WebRtcVideoSendStream::SetCodecAndOptions( } } - if (HasNack(codec_settings.codec)) { - parameters_.config.rtp.nack.rtp_history_ms = kNackHistoryMs; - } + parameters_.config.rtp.nack.rtp_history_ms = + HasNack(codec_settings.codec) ? kNackHistoryMs : 0; options.suspend_below_min_bitrate.Get( ¶meters_.config.suspend_below_min_bitrate); @@ -2298,10 +2298,15 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs( RecreateWebRtcStream(); } -void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRemb(bool enabled) { - if (config_.rtp.remb == enabled) +void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb( + bool nack_enabled, bool remb_enabled) { + int nack_history_ms = nack_enabled ? kNackHistoryMs : 0; + if (config_.rtp.nack.rtp_history_ms == nack_history_ms && + config_.rtp.remb == remb_enabled) { return; - config_.rtp.remb = enabled; + } + config_.rtp.remb = remb_enabled; + config_.rtp.nack.rtp_history_ms = nack_history_ms; RecreateWebRtcStream(); } diff --git a/talk/media/webrtc/webrtcvideoengine2.h b/talk/media/webrtc/webrtcvideoengine2.h index 3bff8c2c4e..8c33069fe8 100644 --- a/talk/media/webrtc/webrtcvideoengine2.h +++ b/talk/media/webrtc/webrtcvideoengine2.h @@ -412,7 +412,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler, const std::vector& GetSsrcs() const; - void SetRemb(bool enabled); + void SetNackAndRemb(bool nack_enabled, bool remb_enabled); void SetRecvCodecs(const std::vector& recv_codecs); void SetRtpExtensions(const std::vector& extensions); diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 89074ccd1e..4cffada418 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -1313,21 +1313,30 @@ TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) { recv_stream->GetConfig().rtp.nack.rtp_history_ms); } -TEST_F(WebRtcVideoChannel2Test, NackCanBeDisabled) { +TEST_F(WebRtcVideoChannel2Test, NackCanBeEnabledAndDisabled) { + FakeVideoReceiveStream* recv_stream = AddRecvStream(); + FakeVideoSendStream* send_stream = AddSendStream(); + + EXPECT_GT(recv_stream->GetConfig().rtp.nack.rtp_history_ms, 0); + EXPECT_GT(send_stream->GetConfig().rtp.nack.rtp_history_ms, 0); + + // Verify that NACK is turned off when send(!) codecs without NACK are set. std::vector codecs; codecs.push_back(kVp8Codec); - - // Send side. - ASSERT_TRUE(channel_->SetSendCodecs(codecs)); - FakeVideoSendStream* send_stream = - AddSendStream(cricket::StreamParams::CreateLegacy(1)); + EXPECT_TRUE(codecs[0].feedback_params.params().empty()); + EXPECT_TRUE(channel_->SetSendCodecs(codecs)); + recv_stream = fake_call_->GetVideoReceiveStreams()[0]; + EXPECT_EQ(0, recv_stream->GetConfig().rtp.nack.rtp_history_ms); + send_stream = fake_call_->GetVideoSendStreams()[0]; EXPECT_EQ(0, send_stream->GetConfig().rtp.nack.rtp_history_ms); - // Receiver side. - ASSERT_TRUE(channel_->SetRecvCodecs(codecs)); - FakeVideoReceiveStream* recv_stream = - AddRecvStream(cricket::StreamParams::CreateLegacy(1)); - EXPECT_EQ(0, recv_stream->GetConfig().rtp.nack.rtp_history_ms); + // Verify that NACK is turned on when setting default codecs since the + // default codecs have NACK enabled. + EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); + recv_stream = fake_call_->GetVideoReceiveStreams()[0]; + EXPECT_GT(recv_stream->GetConfig().rtp.nack.rtp_history_ms, 0); + send_stream = fake_call_->GetVideoSendStreams()[0]; + EXPECT_GT(send_stream->GetConfig().rtp.nack.rtp_history_ms, 0); } TEST_F(WebRtcVideoChannel2Test, DISABLED_VideoProtectionInterop) {