diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index bf731c4ca..d6d1354bf 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -161,6 +161,22 @@ static bool FindBestVideoFormat(int max_width, return false; } +static void AddDefaultFeedbackParams(VideoCodec* codec) { + const FeedbackParam kFir(kRtcpFbParamCcm, kRtcpFbCcmParamFir); + codec->AddFeedbackParam(kFir); + const FeedbackParam kNack(kRtcpFbParamNack, kParamValueEmpty); + codec->AddFeedbackParam(kNack); + const FeedbackParam kPli(kRtcpFbParamNack, kRtcpFbNackParamPli); + codec->AddFeedbackParam(kPli); + const FeedbackParam kRemb(kRtcpFbParamRemb, kParamValueEmpty); + codec->AddFeedbackParam(kRemb); +} + +static bool IsNackEnabled(const VideoCodec& codec) { + return codec.HasFeedbackParam( + FeedbackParam(kRtcpFbParamNack, kParamValueEmpty)); +} + static VideoCodec DefaultVideoCodec() { VideoCodec default_codec(kDefaultVideoCodecPref.payload_type, kDefaultVideoCodecPref.name, @@ -168,6 +184,7 @@ static VideoCodec DefaultVideoCodec() { kDefaultVideoFormat.height, kDefaultFramerate, 0); + AddDefaultFeedbackParams(&default_codec); return default_codec; } @@ -950,7 +967,9 @@ bool WebRtcVideoChannel2::AddSendStream(const StreamParams& sp) { config.rtp.rtx.payload_type = codec_settings.rtx_payload_type; } - config.rtp.nack.rtp_history_ms = kNackHistoryMs; + if (IsNackEnabled(codec_settings.codec)) { + config.rtp.nack.rtp_history_ms = kNackHistoryMs; + } config.rtp.max_packet_size = kVideoMtu; WebRtcVideoSendStream* stream = @@ -1024,7 +1043,9 @@ bool WebRtcVideoChannel2::AddRecvStream(const StreamParams& sp) { config.rtp.remote_ssrc = ssrc; config.rtp.local_ssrc = rtcp_receiver_report_ssrc_; - config.rtp.nack.rtp_history_ms = kNackHistoryMs; + if (IsNackEnabled(recv_codecs_.begin()->codec)) { + config.rtp.nack.rtp_history_ms = kNackHistoryMs; + } config.rtp.remb = true; // TODO(pbos): This protection is against setting the same local ssrc as // remote which is not permitted by the lower-level API. RTCP requires a diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 4dd27bc79..c9ff18290 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -48,6 +48,18 @@ static const cricket::VideoCodec kUlpfecCodec(117, "ulpfec", 0, 0, 0, 0); static const uint32 kSsrcs1[] = {1}; static const uint32 kRtxSsrcs1[] = {4}; + +void VerifyCodecHasDefaultFeedbackParams(const cricket::VideoCodec& codec) { + EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamNack, cricket::kParamValueEmpty))); + EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamNack, cricket::kRtcpFbNackParamPli))); + EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamRemb, cricket::kParamValueEmpty))); + EXPECT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamCcm, cricket::kRtcpFbCcmParamFir))); +} + } // namespace namespace cricket { @@ -744,7 +756,9 @@ TEST_F(WebRtcVideoChannel2Test, DISABLED_RembOnOff) { FAIL() << "Not implemented."; // TODO(pbos): Implement. } -TEST_F(WebRtcVideoChannel2Test, NackIsEnabled) { +TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) { + VerifyCodecHasDefaultFeedbackParams(default_codec_); + EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs())); EXPECT_TRUE(channel_->SetSend(true)); @@ -763,6 +777,23 @@ TEST_F(WebRtcVideoChannel2Test, NackIsEnabled) { recv_stream->GetConfig().rtp.nack.rtp_history_ms); } +TEST_F(WebRtcVideoChannel2Test, NackCanBeDisabled) { + std::vector codecs; + codecs.push_back(kVp8Codec); + + // Send side. + ASSERT_TRUE(channel_->SetSendCodecs(codecs)); + FakeVideoSendStream* send_stream = + AddSendStream(cricket::StreamParams::CreateLegacy(1)); + 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); +} + TEST_F(WebRtcVideoChannel2Test, DISABLED_VideoProtectionInterop) { FAIL() << "Not implemented."; // TODO(pbos): Implement. }