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', ], },