From 2a72c6506a49b15d5e079eaa28cb80abb445684b Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Thu, 26 Feb 2015 16:01:24 +0000 Subject: [PATCH] Keep feedback params in SetDefaultEncoderConfig. Prevents NACK etc. from breaking completely as it won't be reported in the generated SDP. BUG=1788 R=mflodman@webrtc.org Review URL: https://webrtc-codereview.appspot.com/40109004 Cr-Commit-Position: refs/heads/master@{#8519} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8519 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/media/webrtc/webrtcvideoengine2.cc | 4 +++- .../webrtc/webrtcvideoengine2_unittest.cc | 20 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/talk/media/webrtc/webrtcvideoengine2.cc b/talk/media/webrtc/webrtcvideoengine2.cc index 9006934121..ebf05fa58b 100644 --- a/talk/media/webrtc/webrtcvideoengine2.cc +++ b/talk/media/webrtc/webrtcvideoengine2.cc @@ -372,7 +372,9 @@ bool WebRtcVideoEngine2::SetDefaultEncoderConfig( bool supports_codec = false; for (size_t i = 0; i < video_codecs_.size(); ++i) { if (CodecNameMatches(video_codecs_[i].name, codec.name)) { - video_codecs_[i] = codec; + video_codecs_[i].width = codec.width; + video_codecs_[i].height = codec.height; + video_codecs_[i].framerate = codec.framerate; supports_codec = true; break; } diff --git a/talk/media/webrtc/webrtcvideoengine2_unittest.cc b/talk/media/webrtc/webrtcvideoengine2_unittest.cc index 283ade1af9..261bc78670 100644 --- a/talk/media/webrtc/webrtcvideoengine2_unittest.cc +++ b/talk/media/webrtc/webrtcvideoengine2_unittest.cc @@ -497,6 +497,26 @@ TEST_F(WebRtcVideoEngine2Test, FindCodec) { EXPECT_TRUE(engine_.FindCodec(rtx)); } +TEST_F(WebRtcVideoEngine2Test, SetDefaultEncoderConfigPreservesFeedbackParams) { + cricket::VideoCodec max_settings( + engine_.codecs()[0].id, engine_.codecs()[0].name, + engine_.codecs()[0].width / 2, engine_.codecs()[0].height / 2, 30, 0); + // This codec shouldn't have NACK by default or the test is pointless. + EXPECT_FALSE(max_settings.HasFeedbackParam( + FeedbackParam(kRtcpFbParamNack, kParamValueEmpty))); + // The engine should by default have it however. + EXPECT_TRUE(engine_.codecs()[0].HasFeedbackParam( + FeedbackParam(kRtcpFbParamNack, kParamValueEmpty))); + + // Set constrained max codec settings. + EXPECT_TRUE(engine_.SetDefaultEncoderConfig( + cricket::VideoEncoderConfig(max_settings))); + + // Verify that feedback parameters are retained. + EXPECT_TRUE(engine_.codecs()[0].HasFeedbackParam( + FeedbackParam(kRtcpFbParamNack, kParamValueEmpty))); +} + TEST_F(WebRtcVideoEngine2Test, DefaultRtxCodecHasAssociatedPayloadTypeSet) { std::vector engine_codecs = engine_.codecs(); for (size_t i = 0; i < engine_codecs.size(); ++i) {