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

Add NACK feedback parameter to WebRtcVideoEngine2.

Browse Source 
Also fixing enabling/disabling of NACK. Previous implementation was made
under the assumption that NACK should always be enabled which caused
both missing NACK settings in SDP as well as broken interop between old
and new WebRtcVideoEngines.

BUG=1788
R=pthatcher@webrtc.org

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@6431 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
pbos@webrtc.org 2014-06-13 12:27:38 +00:00
parent e322a175f6
commit f99c2f2dbc
2 changed files with 55 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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

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

@ -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<VideoCodec> 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.
}