generic-library/webrtc
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Base NACK on send codecs.

Browse Source 
Addressing discrepancy where NACK used to be set from send codecs in
WebRtcVideoEngine(1), and before this change, from recv codecs in
WebRtcVideoEngine2. This should address that NACK might be sent even if
the remote side does not support it.

BUG=4626
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/53409004

Cr-Commit-Position: refs/heads/master@{#9171}
This commit is contained in:
Peter Boström 2015-05-11 14:34:58 +02:00
parent 126c03ea02
commit 67c9df7982
3 changed files with 33 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
talk/media/webrtc/webrtcvideoengine2.cc
View File

@ -943,7 +943,8 @@ bool WebRtcVideoChannel2::SetSendCodecs(const std::vector<VideoCodec>& codecs) {
} }
for (auto& kv : receive_streams_) { for (auto& kv : receive_streams_) {
DCHECK(kv.second != nullptr); 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 // 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 =
parameters_.config.rtp.nack.rtp_history_ms = kNackHistoryMs; HasNack(codec_settings.codec) ? kNackHistoryMs : 0;
}
options.suspend_below_min_bitrate.Get( options.suspend_below_min_bitrate.Get(
&parameters_.config.suspend_below_min_bitrate); &parameters_.config.suspend_below_min_bitrate);
@ -2298,10 +2298,15 @@ void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRecvCodecs(
RecreateWebRtcStream(); RecreateWebRtcStream();
} }
void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetRemb(bool enabled) { void WebRtcVideoChannel2::WebRtcVideoReceiveStream::SetNackAndRemb(
if (config_.rtp.remb == enabled) 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; return;
config_.rtp.remb = enabled; }
config_.rtp.remb = remb_enabled;
config_.rtp.nack.rtp_history_ms = nack_history_ms;
RecreateWebRtcStream(); RecreateWebRtcStream();
} }

2
talk/media/webrtc/webrtcvideoengine2.h
View File

@ -412,7 +412,7 @@ class WebRtcVideoChannel2 : public rtc::MessageHandler,
const std::vector<uint32>& GetSsrcs() const; const std::vector<uint32>& GetSsrcs() const;
void SetRemb(bool enabled); void SetNackAndRemb(bool nack_enabled, bool remb_enabled);
void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs); void SetRecvCodecs(const std::vector<VideoCodecSettings>& recv_codecs);
void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions); void SetRtpExtensions(const std::vector<webrtc::RtpExtension>& extensions);

31
talk/media/webrtc/webrtcvideoengine2_unittest.cc
View File

@ -1313,21 +1313,30 @@ TEST_F(WebRtcVideoChannel2Test, NackIsEnabledByDefault) {
recv_stream->GetConfig().rtp.nack.rtp_history_ms); 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<VideoCodec> codecs; std::vector<VideoCodec> codecs;
codecs.push_back(kVp8Codec); codecs.push_back(kVp8Codec);
EXPECT_TRUE(codecs[0].feedback_params.params().empty());
// Send side. EXPECT_TRUE(channel_->SetSendCodecs(codecs));
ASSERT_TRUE(channel_->SetSendCodecs(codecs)); recv_stream = fake_call_->GetVideoReceiveStreams()[0];
FakeVideoSendStream* send_stream = EXPECT_EQ(0, recv_stream->GetConfig().rtp.nack.rtp_history_ms);
AddSendStream(cricket::StreamParams::CreateLegacy(1)); send_stream = fake_call_->GetVideoSendStreams()[0];
EXPECT_EQ(0, send_stream->GetConfig().rtp.nack.rtp_history_ms); EXPECT_EQ(0, send_stream->GetConfig().rtp.nack.rtp_history_ms);
// Receiver side. // Verify that NACK is turned on when setting default codecs since the
ASSERT_TRUE(channel_->SetRecvCodecs(codecs)); // default codecs have NACK enabled.
FakeVideoReceiveStream* recv_stream = EXPECT_TRUE(channel_->SetSendCodecs(engine_.codecs()));
AddRecvStream(cricket::StreamParams::CreateLegacy(1)); recv_stream = fake_call_->GetVideoReceiveStreams()[0];
EXPECT_EQ(0, recv_stream->GetConfig().rtp.nack.rtp_history_ms); 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) { TEST_F(WebRtcVideoChannel2Test, DISABLED_VideoProtectionInterop) {