From 5860de02aa071af21a828e72cb46bf13a5e4eb60 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Mon, 16 Sep 2013 13:01:47 +0000 Subject: [PATCH] Implement NACK over RTX for VideoSendStream. BUG=2231 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/2197008 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4751 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../internal/video_send_stream.cc | 12 ++++++ webrtc/video_engine/new_include/config.h | 7 ++-- webrtc/video_engine/test/send_stream_tests.cc | 41 +++++++++++++++---- 3 files changed, 49 insertions(+), 11 deletions(-) diff --git a/webrtc/video_engine/internal/video_send_stream.cc b/webrtc/video_engine/internal/video_send_stream.cc index 76d8bc3d2c..5adf8b9562 100644 --- a/webrtc/video_engine/internal/video_send_stream.cc +++ b/webrtc/video_engine/internal/video_send_stream.cc @@ -105,6 +105,18 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, } rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); rtp_rtcp_->SetTransmissionSmoothingStatus(channel_, config_.pacing); + if (!config_.rtp.rtx.ssrcs.empty()) { + assert(config_.rtp.rtx.ssrcs.size() == config_.rtp.ssrcs.size()); + for (size_t i = 0; i < config_.rtp.rtx.ssrcs.size(); ++i) { + rtp_rtcp_->SetLocalSSRC( + channel_, config_.rtp.rtx.ssrcs[i], kViEStreamTypeRtx, i); + } + + if (config_.rtp.rtx.rtx_payload_type != 0) { + rtp_rtcp_->SetRtxSendPayloadType(channel_, + config_.rtp.rtx.rtx_payload_type); + } + } for (size_t i = 0; i < config_.rtp.extensions.size(); ++i) { const std::string& extension = config_.rtp.extensions[i].name; diff --git a/webrtc/video_engine/new_include/config.h b/webrtc/video_engine/new_include/config.h index 5eff973e89..d19c8d971c 100644 --- a/webrtc/video_engine/new_include/config.h +++ b/webrtc/video_engine/new_include/config.h @@ -12,6 +12,7 @@ #define WEBRTC_VIDEO_ENGINE_NEW_INCLUDE_CONFIG_H_ #include +#include namespace webrtc { @@ -60,9 +61,9 @@ struct FecConfig { // Settings for RTP retransmission payload format, see RFC 4588 for details. struct RtxConfig { - RtxConfig() : ssrc(0), rtx_payload_type(0), video_payload_type(0) {} - // SSRC to use for the RTX stream. - uint32_t ssrc; + RtxConfig() : rtx_payload_type(0), video_payload_type(0) {} + // SSRCs to use for the RTX streams. + std::vector ssrcs; // Payload type to use for the RTX stream. int rtx_payload_type; diff --git a/webrtc/video_engine/test/send_stream_tests.cc b/webrtc/video_engine/test/send_stream_tests.cc index 1753b43538..879a3e416b 100644 --- a/webrtc/video_engine/test/send_stream_tests.cc +++ b/webrtc/video_engine/test/send_stream_tests.cc @@ -47,6 +47,7 @@ class VideoSendStreamTest : public ::testing::Test { protected: static const uint32_t kSendSsrc; + static const uint32_t kSendRtxSsrc; void RunSendTest(Call* call, const VideoSendStream::Config& config, SendTransportObserver* observer) { @@ -75,10 +76,13 @@ class VideoSendStreamTest : public ::testing::Test { return config; } + void TestNackRetransmission(uint32_t retransmit_ssrc); + test::FakeEncoder fake_encoder_; }; const uint32_t VideoSendStreamTest::kSendSsrc = 0xC0FFEE; +const uint32_t VideoSendStreamTest::kSendRtxSsrc = 0xBADCAFE; TEST_F(VideoSendStreamTest, SendsSetSsrc) { class SendSsrcObserver : public SendTransportObserver { @@ -215,15 +219,15 @@ TEST_F(VideoSendStreamTest, SupportsTransmissionTimeOffset) { RunSendTest(call.get(), send_config, &observer); } -TEST_F(VideoSendStreamTest, RespondsToNack) { +void VideoSendStreamTest::TestNackRetransmission(uint32_t retransmit_ssrc) { class NackObserver : public SendTransportObserver, webrtc::Transport { public: - NackObserver() + NackObserver(uint32_t retransmit_ssrc) : SendTransportObserver(30 * 1000), thread_(ThreadWrapper::CreateThread(NackProcess, this)), send_call_receiver_(NULL), send_count_(0), - ssrc_(0), + retransmit_ssrc_(retransmit_ssrc), nacked_sequence_number_(0) {} ~NackObserver() { @@ -247,7 +251,7 @@ TEST_F(VideoSendStreamTest, RespondsToNack) { EXPECT_EQ(0, rtcp_sender.RegisterSendTransport(this)); rtcp_sender.SetRTCPStatus(kRtcpNonCompound); - rtcp_sender.SetRemoteSSRC(ssrc_); + rtcp_sender.SetRemoteSSRC(kSendSsrc); RTCPSender::FeedbackState feedback_state; EXPECT_EQ(0, rtcp_sender.SendRTCP( @@ -277,14 +281,23 @@ TEST_F(VideoSendStreamTest, RespondsToNack) { // Nack second packet after receiving the third one. if (++send_count_ == 3) { - ssrc_ = header.ssrc; nacked_sequence_number_ = header.sequenceNumber - 1; unsigned int id; EXPECT_TRUE(thread_->Start(id)); } - if (header.sequenceNumber == nacked_sequence_number_) + uint16_t sequence_number = header.sequenceNumber; + + if (header.ssrc == retransmit_ssrc_ && retransmit_ssrc_ != kSendSsrc) { + // Not kSendSsrc, assume correct RTX packet. Extract sequence number. + const uint8_t* rtx_header = packet + header.headerLength; + sequence_number = (rtx_header[0] << 8) + rtx_header[1]; + } + + if (sequence_number == nacked_sequence_number_) { + EXPECT_EQ(retransmit_ssrc_, header.ssrc); send_test_complete_->Set(); + } return true; } @@ -292,9 +305,9 @@ TEST_F(VideoSendStreamTest, RespondsToNack) { scoped_ptr thread_; PacketReceiver* send_call_receiver_; int send_count_; - uint32_t ssrc_; + uint32_t retransmit_ssrc_; uint16_t nacked_sequence_number_; - } observer; + } observer(retransmit_ssrc); Call::Config call_config(&observer); scoped_ptr call(Call::Create(call_config)); @@ -302,8 +315,20 @@ TEST_F(VideoSendStreamTest, RespondsToNack) { VideoSendStream::Config send_config = GetSendTestConfig(call.get()); send_config.rtp.nack.rtp_history_ms = 1000; + if (retransmit_ssrc != kSendSsrc) + send_config.rtp.rtx.ssrcs.push_back(retransmit_ssrc); RunSendTest(call.get(), send_config, &observer); } +TEST_F(VideoSendStreamTest, RetransmitsNack) { + // Normal NACKs should use the send SSRC. + TestNackRetransmission(kSendSsrc); +} + +TEST_F(VideoSendStreamTest, RetransmitsNackOverRtx) { + // NACKs over RTX should use a separate SSRC. + TestNackRetransmission(kSendRtxSsrc); +} + } // namespace webrtc