From bbb07e69e55761ca43b38f929f3e39f4bba70ef1 Mon Sep 17 00:00:00 2001 From: "pbos@webrtc.org" Date: Mon, 5 Aug 2013 12:01:36 +0000 Subject: [PATCH] Glue code and tests for NACK in new VideoEngine API. The test works by randomly dropping small bursts of packets until enough NACKs have been sent back by the receiver. Retransmitted packets are never dropped in order to assure that all packets are eventually delivered. When enough NACK packets have been received and all dropped packets retransmitted, the test waits for the receiving side to send a number of RTCP packets without NACK lists to assure that the receiving side stops sending NACKs once packets have been retransmitted. BUG=2043 R=stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/1934004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@4482 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/video_engine/internal/video_call.cc | 17 +- .../internal/video_receive_stream.cc | 9 +- .../internal/video_receive_stream.h | 9 +- .../internal/video_send_stream.cc | 10 + .../video_engine/internal/video_send_stream.h | 3 + .../new_include/video_receive_stream.h | 2 +- .../test/common/direct_transport.h | 4 + webrtc/video_engine/test/engine_tests.cc | 284 ++++++++++++++++++ webrtc/video_engine/test/tests.gypi | 15 +- 9 files changed, 341 insertions(+), 12 deletions(-) create mode 100644 webrtc/video_engine/test/engine_tests.cc diff --git a/webrtc/video_engine/internal/video_call.cc b/webrtc/video_engine/internal/video_call.cc index db77c76034..b429f12487 100644 --- a/webrtc/video_engine/internal/video_call.cc +++ b/webrtc/video_engine/internal/video_call.cc @@ -142,7 +142,20 @@ bool VideoCall::DeliverRtcp(ModuleRTPUtility::RTPHeaderParser* rtp_parser, receive_ssrcs_.begin(); it != receive_ssrcs_.end(); ++it) { if (static_cast(it->second) - ->DeliverRtcp(packet, length)) { + ->DeliverRtcp(static_cast(packet), length)) { + rtcp_delivered = true; + } + } + + if (rtcp_delivered) + return true; + + for (std::map::iterator it = + send_ssrcs_.begin(); + it != send_ssrcs_.end(); + ++it) { + if (static_cast(it->second) + ->DeliverRtcp(static_cast(packet), length)) { rtcp_delivered = true; } } @@ -167,7 +180,7 @@ bool VideoCall::DeliverRtp(ModuleRTPUtility::RTPHeaderParser* rtp_parser, VideoReceiveStream* receiver = static_cast(receive_ssrcs_[rtp_header.ssrc]); - return receiver->DeliverRtp(packet, length); + return receiver->DeliverRtp(static_cast(packet), length); } bool VideoCall::DeliverPacket(const void* packet, size_t length) { diff --git a/webrtc/video_engine/internal/video_receive_stream.cc b/webrtc/video_engine/internal/video_receive_stream.cc index 490619e6e1..c6722d2241 100644 --- a/webrtc/video_engine/internal/video_receive_stream.cc +++ b/webrtc/video_engine/internal/video_receive_stream.cc @@ -30,7 +30,7 @@ VideoReceiveStream::VideoReceiveStream( webrtc::VideoEngine* video_engine, const newapi::VideoReceiveStream::Config& config, newapi::Transport* transport) - : transport_(transport), config_(config) { + : transport_(transport), config_(config), channel_(-1) { video_engine_base_ = ViEBase::GetInterface(video_engine); // TODO(mflodman): Use the other CreateChannel method. video_engine_base_->CreateChannel(channel_); @@ -39,6 +39,9 @@ VideoReceiveStream::VideoReceiveStream( rtp_rtcp_ = ViERTP_RTCP::GetInterface(video_engine); assert(rtp_rtcp_ != NULL); + // TODO(pbos): This is not fine grained enough... + rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); + assert(config_.rtp.ssrc != 0); network_ = ViENetwork::GetInterface(video_engine); @@ -98,11 +101,11 @@ void VideoReceiveStream::GetCurrentReceiveCodec(VideoCodec* receive_codec) { // TODO(pbos): Implement } -bool VideoReceiveStream::DeliverRtcp(const void* packet, size_t length) { +bool VideoReceiveStream::DeliverRtcp(const uint8_t* packet, size_t length) { return network_->ReceivedRTCPPacket(channel_, packet, length) == 0; } -bool VideoReceiveStream::DeliverRtp(const void* packet, size_t length) { +bool VideoReceiveStream::DeliverRtp(const uint8_t* packet, size_t length) { return network_->ReceivedRTPPacket(channel_, packet, length) == 0; } diff --git a/webrtc/video_engine/internal/video_receive_stream.h b/webrtc/video_engine/internal/video_receive_stream.h index 9571b712c3..932776a835 100644 --- a/webrtc/video_engine/internal/video_receive_stream.h +++ b/webrtc/video_engine/internal/video_receive_stream.h @@ -43,21 +43,20 @@ class VideoReceiveStream : public newapi::VideoReceiveStream, virtual void GetCurrentReceiveCodec(VideoCodec* receive_codec) OVERRIDE; - virtual bool DeliverRtcp(const void* packet, size_t length); - virtual bool DeliverRtp(const void* packet, size_t length); - virtual int FrameSizeChange(unsigned int width, unsigned int height, unsigned int /*number_of_streams*/) OVERRIDE; - virtual int DeliverFrame(uint8_t* frame, int buffer_size, uint32_t timestamp, int64_t render_time) OVERRIDE; virtual int SendPacket(int /*channel*/, const void* packet, int length) OVERRIDE; - virtual int SendRTCPPacket(int /*channel*/, const void* packet, int length) OVERRIDE; + public: + virtual bool DeliverRtcp(const uint8_t* packet, size_t length); + virtual bool DeliverRtp(const uint8_t* packet, size_t length); + private: newapi::Transport* transport_; newapi::VideoReceiveStream::Config config_; diff --git a/webrtc/video_engine/internal/video_send_stream.cc b/webrtc/video_engine/internal/video_send_stream.cc index b2401daf9a..305d08f9d8 100644 --- a/webrtc/video_engine/internal/video_send_stream.cc +++ b/webrtc/video_engine/internal/video_send_stream.cc @@ -94,6 +94,7 @@ VideoSendStream::VideoSendStream(newapi::Transport* transport, assert(config_.rtp.ssrcs.size() == 1); rtp_rtcp_->SetLocalSSRC(channel_, config_.rtp.ssrcs[0]); + rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); capture_ = ViECapture::GetInterface(video_engine); capture_->AllocateExternalCaptureDevice(capture_id_, external_capture_); @@ -168,11 +169,15 @@ newapi::VideoSendStreamInput* VideoSendStream::Input() { return this; } void VideoSendStream::StartSend() { if (video_engine_base_->StartSend(channel_) != 0) abort(); + if (video_engine_base_->StartReceive(channel_) != 0) + abort(); } void VideoSendStream::StopSend() { if (video_engine_base_->StopSend(channel_) != 0) abort(); + if (video_engine_base_->StopReceive(channel_) != 0) + abort(); } bool VideoSendStream::SetTargetBitrate( @@ -206,5 +211,10 @@ int VideoSendStream::SendRTCPPacket(int /*channel*/, return success ? 0 : -1; } +bool VideoSendStream::DeliverRtcp(const uint8_t* packet, size_t length) { + return network_->ReceivedRTCPPacket( + channel_, packet, static_cast(length)); +} + } // namespace internal } // namespace webrtc diff --git a/webrtc/video_engine/internal/video_send_stream.h b/webrtc/video_engine/internal/video_send_stream.h index 571f428291..686e5d5735 100644 --- a/webrtc/video_engine/internal/video_send_stream.h +++ b/webrtc/video_engine/internal/video_send_stream.h @@ -63,6 +63,9 @@ class VideoSendStream : public newapi::VideoSendStream, virtual int SendRTCPPacket(int /*channel*/, const void* packet, int length) OVERRIDE; + public: + bool DeliverRtcp(const uint8_t* packet, size_t length); + private: newapi::Transport* transport_; newapi::VideoSendStream::Config config_; diff --git a/webrtc/video_engine/new_include/video_receive_stream.h b/webrtc/video_engine/new_include/video_receive_stream.h index d8575ba8c3..ba5286b395 100644 --- a/webrtc/video_engine/new_include/video_receive_stream.h +++ b/webrtc/video_engine/new_include/video_receive_stream.h @@ -93,7 +93,7 @@ class VideoReceiveStream { pre_decode_callback(NULL), post_decode_callback(NULL), target_delay_ms(0) {} - // Codecs the receive stream + // Codecs the receive stream can receive. std::vector codecs; // Receive-stream specific RTP settings. diff --git a/webrtc/video_engine/test/common/direct_transport.h b/webrtc/video_engine/test/common/direct_transport.h index 6f19da29fe..f935641609 100644 --- a/webrtc/video_engine/test/common/direct_transport.h +++ b/webrtc/video_engine/test/common/direct_transport.h @@ -10,6 +10,8 @@ #ifndef WEBRTC_VIDEO_ENGINE_TEST_COMMON_DIRECT_TRANSPORT_H_ #define WEBRTC_VIDEO_ENGINE_TEST_COMMON_DIRECT_TRANSPORT_H_ +#include + #include "webrtc/video_engine/new_include/transport.h" namespace webrtc { @@ -23,10 +25,12 @@ class DirectTransport : public newapi::Transport { void SetReceiver(newapi::PacketReceiver* receiver) { receiver_ = receiver; } virtual bool SendRTP(const uint8_t* data, size_t length) OVERRIDE { + assert(receiver_ != NULL); return receiver_->DeliverPacket(data, length); } virtual bool SendRTCP(const uint8_t* data, size_t length) OVERRIDE { + assert(receiver_ != NULL); return SendRTP(data, length); } diff --git a/webrtc/video_engine/test/engine_tests.cc b/webrtc/video_engine/test/engine_tests.cc new file mode 100644 index 0000000000..8247decf5e --- /dev/null +++ b/webrtc/video_engine/test/engine_tests.cc @@ -0,0 +1,284 @@ +/* + * Copyright (c) 2013 The WebRTC project authors. All Rights Reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#include + +#include "testing/gtest/include/gtest/gtest.h" + +#include "webrtc/modules/rtp_rtcp/interface/rtp_header_parser.h" +#include "webrtc/modules/rtp_rtcp/source/rtcp_utility.h" +#include "webrtc/system_wrappers/interface/critical_section_wrapper.h" +#include "webrtc/system_wrappers/interface/scoped_ptr.h" +#include "webrtc/system_wrappers/interface/event_wrapper.h" +#include "webrtc/video_engine/new_include/video_engine.h" +#include "webrtc/video_engine/test/common/frame_generator.h" +#include "webrtc/video_engine/test/common/frame_generator_capturer.h" +#include "webrtc/video_engine/test/common/generate_ssrcs.h" + +namespace webrtc { + +class NackObserver { + public: + class SenderTransport : public newapi::Transport { + public: + explicit SenderTransport(NackObserver* observer) + : receiver_(NULL), observer_(observer) {} + + bool SendRTP(const uint8_t* packet, size_t length) { + { + CriticalSectionScoped lock(observer_->crit_.get()); + if (observer_->DropSendPacket(packet, length)) + return true; + ++observer_->sent_rtp_packets_; + } + + return receiver_->DeliverPacket(packet, length); + } + + bool SendRTCP(const uint8_t* packet, size_t length) { + return receiver_->DeliverPacket(packet, length); + } + + newapi::PacketReceiver* receiver_; + NackObserver* observer_; + } sender_transport_; + + class ReceiverTransport : public newapi::Transport { + public: + explicit ReceiverTransport(NackObserver* observer) + : receiver_(NULL), observer_(observer) {} + + bool SendRTP(const uint8_t* packet, size_t length) { + return receiver_->DeliverPacket(packet, length); + } + + bool SendRTCP(const uint8_t* packet, size_t length) { + { + CriticalSectionScoped lock(observer_->crit_.get()); + + RTCPUtility::RTCPParserV2 parser(packet, length, true); + EXPECT_TRUE(parser.IsValid()); + + bool received_nack = false; + RTCPUtility::RTCPPacketTypes packet_type = parser.Begin(); + while (packet_type != RTCPUtility::kRtcpNotValidCode) { + if (packet_type == RTCPUtility::kRtcpRtpfbNackCode) + received_nack = true; + + packet_type = parser.Iterate(); + } + + if (received_nack) { + observer_->ReceivedNack(); + } else { + observer_->RtcpWithoutNack(); + } + } + return receiver_->DeliverPacket(packet, length); + } + + newapi::PacketReceiver* receiver_; + NackObserver* observer_; + } receiver_transport_; + + NackObserver() + : sender_transport_(this), + receiver_transport_(this), + crit_(CriticalSectionWrapper::CreateCriticalSection()), + received_all_retransmissions_(EventWrapper::Create()), + rtp_parser_(RtpHeaderParser::Create()), + drop_burst_count_(0), + sent_rtp_packets_(0), + nacks_left_(4) {} + + EventTypeWrapper Wait() { + // 2 minutes should be more than enough time for the test to finish. + return received_all_retransmissions_->Wait(2 * 60 * 1000); + } + + private: + // Decides whether a current packet should be dropped or not. A retransmitted + // packet will never be dropped. Packets are dropped in short bursts. When + // enough NACKs have been received, no further packets are dropped. + bool DropSendPacket(const uint8_t* packet, size_t length) { + EXPECT_FALSE(RtpHeaderParser::IsRtcp(packet, static_cast(length))); + + RTPHeader header; + EXPECT_TRUE(rtp_parser_->Parse(packet, static_cast(length), &header)); + + // Never drop retransmitted packets. + if (dropped_packets_.find(header.sequenceNumber) != + dropped_packets_.end()) { + retransmitted_packets_.insert(header.sequenceNumber); + return false; + } + + // Enough NACKs received, stop dropping packets. + if (nacks_left_ == 0) + return false; + + // Still dropping packets. + if (drop_burst_count_ > 0) { + --drop_burst_count_; + dropped_packets_.insert(header.sequenceNumber); + return true; + } + + if (sent_rtp_packets_ > 0 && rand() % 20 == 0) { + drop_burst_count_ = rand() % 10; + dropped_packets_.insert(header.sequenceNumber); + return true; + } + + return false; + } + + void ReceivedNack() { + if (nacks_left_ > 0) + --nacks_left_; + rtcp_without_nack_count_ = 0; + } + + void RtcpWithoutNack() { + if (nacks_left_ > 0) + return; + ++rtcp_without_nack_count_; + + // All packets retransmitted and no recent NACKs. + if (dropped_packets_.size() == retransmitted_packets_.size() && + rtcp_without_nack_count_ >= kRequiredRtcpsWithoutNack) { + received_all_retransmissions_->Set(); + } + } + + scoped_ptr crit_; + scoped_ptr received_all_retransmissions_; + + scoped_ptr rtp_parser_; + std::set dropped_packets_; + std::set retransmitted_packets_; + int drop_burst_count_; + uint64_t sent_rtp_packets_; + int nacks_left_; + int rtcp_without_nack_count_; + static const int kRequiredRtcpsWithoutNack = 2; +}; + +struct EngineTestParams { + size_t width, height; + struct { + unsigned int min, start, max; + } bitrate; +}; + +class EngineTest : public ::testing::TestWithParam { + public: + virtual void SetUp() { + video_engine_.reset( + newapi::VideoEngine::Create(newapi::VideoEngineConfig())); + reserved_ssrcs_.clear(); + } + + protected: + newapi::VideoCall* CreateTestCall(newapi::Transport* transport) { + newapi::VideoCall::Config call_config; + call_config.send_transport = transport; + return video_engine_->CreateCall(call_config); + } + + newapi::VideoSendStream::Config CreateTestSendConfig( + newapi::VideoCall* call, + EngineTestParams params) { + newapi::VideoSendStream::Config config = call->GetDefaultSendConfig(); + + test::GenerateRandomSsrcs(&config, &reserved_ssrcs_); + + config.codec.width = static_cast(params.width); + config.codec.height = static_cast(params.height); + config.codec.minBitrate = params.bitrate.min; + config.codec.startBitrate = params.bitrate.start; + config.codec.maxBitrate = params.bitrate.max; + + return config; + } + + test::FrameGeneratorCapturer* CreateTestFrameGeneratorCapturer( + newapi::VideoSendStream* target, + EngineTestParams params) { + return test::FrameGeneratorCapturer::Create( + target->Input(), + test::FrameGenerator::Create( + params.width, params.height, Clock::GetRealTimeClock()), + 30); + } + + scoped_ptr video_engine_; + std::map reserved_ssrcs_; +}; + +// TODO(pbos): What are sane values here for bitrate? Are we missing any +// important resolutions? +EngineTestParams video_1080p = {1920, 1080, {300, 600, 800}}; +EngineTestParams video_720p = {1280, 720, {300, 600, 800}}; +EngineTestParams video_vga = {640, 480, {300, 600, 800}}; +EngineTestParams video_qvga = {320, 240, {300, 600, 800}}; +EngineTestParams video_4cif = {704, 576, {300, 600, 800}}; +EngineTestParams video_cif = {352, 288, {300, 600, 800}}; +EngineTestParams video_qcif = {176, 144, {300, 600, 800}}; + +TEST_P(EngineTest, ReceivesAndRetransmitsNack) { + EngineTestParams params = GetParam(); + + // Set up a video call per sender and receiver. Both send RTCP, and have a set + // RTP history > 0 to enable NACK and retransmissions. + NackObserver observer; + + scoped_ptr sender_call( + CreateTestCall(&observer.sender_transport_)); + scoped_ptr receiver_call( + CreateTestCall(&observer.receiver_transport_)); + + observer.receiver_transport_.receiver_ = sender_call->Receiver(); + observer.sender_transport_.receiver_ = receiver_call->Receiver(); + + + newapi::VideoSendStream::Config send_config = + CreateTestSendConfig(sender_call.get(), params); + send_config.rtp.nack.rtp_history_ms = 1000; + + newapi::VideoReceiveStream::Config receive_config = + receiver_call->GetDefaultReceiveConfig(); + receive_config.rtp.ssrc = send_config.rtp.ssrcs[0]; + receive_config.rtp.nack.rtp_history_ms = send_config.rtp.nack.rtp_history_ms; + + newapi::VideoSendStream* send_stream = + sender_call->CreateSendStream(send_config); + newapi::VideoReceiveStream* receive_stream = + receiver_call->CreateReceiveStream(receive_config); + + scoped_ptr frame_generator_capturer( + CreateTestFrameGeneratorCapturer(send_stream, params)); + ASSERT_TRUE(frame_generator_capturer.get() != NULL); + + receive_stream->StartReceive(); + send_stream->StartSend(); + frame_generator_capturer->Start(); + + EXPECT_EQ(kEventSignaled, observer.Wait()); + + frame_generator_capturer->Stop(); + send_stream->StopSend(); + receive_stream->StopReceive(); + + receiver_call->DestroyReceiveStream(receive_stream); + receiver_call->DestroySendStream(send_stream); +} + +INSTANTIATE_TEST_CASE_P(EngineTest, EngineTest, ::testing::Values(video_vga)); +} // namespace webrtc diff --git a/webrtc/video_engine/test/tests.gypi b/webrtc/video_engine/test/tests.gypi index 250938ded8..1c8ffdde7b 100644 --- a/webrtc/video_engine/test/tests.gypi +++ b/webrtc/video_engine/test/tests.gypi @@ -107,6 +107,7 @@ '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/google-gflags/google-gflags.gyp:google-gflags', '<(webrtc_root)/modules/modules.gyp:video_capture_module', + '<(webrtc_root)/test/test.gyp:test_support', 'video_engine_core', ], }, @@ -132,7 +133,19 @@ 'dependencies': [ '<(DEPTH)/testing/gtest.gyp:gtest', '<(DEPTH)/third_party/google-gflags/google-gflags.gyp:google-gflags', - '<(webrtc_root)/test/test.gyp:test_support', + 'video_tests_common', + ], + }, + { + 'target_name': 'video_engine_tests', + 'type': 'executable', + 'sources': [ + 'engine_tests.cc', + 'test_main.cc', + ], + 'dependencies': [ + '<(DEPTH)/testing/gtest.gyp:gtest', + '<(DEPTH)/third_party/google-gflags/google-gflags.gyp:google-gflags', 'video_tests_common', ], },