diff --git a/webrtc/video_engine/internal/video_receive_stream.cc b/webrtc/video_engine/internal/video_receive_stream.cc index fc742c788..c92cc8d26 100644 --- a/webrtc/video_engine/internal/video_receive_stream.cc +++ b/webrtc/video_engine/internal/video_receive_stream.cc @@ -42,6 +42,14 @@ VideoReceiveStream::VideoReceiveStream(webrtc::VideoEngine* video_engine, // TODO(pbos): This is not fine grained enough... rtp_rtcp_->SetNACKStatus(channel_, config_.rtp.nack.rtp_history_ms > 0); rtp_rtcp_->SetKeyFrameRequestMethod(channel_, kViEKeyFrameRequestPliRtcp); + switch (config_.rtp.rtcp_mode) { + case newapi::kRtcpCompound: + rtp_rtcp_->SetRTCPStatus(channel_, kRtcpCompound_RFC4585); + break; + case newapi::kRtcpReducedSize: + rtp_rtcp_->SetRTCPStatus(channel_, kRtcpNonCompound_RFC5506); + break; + } assert(config_.rtp.ssrc != 0); diff --git a/webrtc/video_engine/new_include/config.h b/webrtc/video_engine/new_include/config.h index d19c8d971..fec34bac3 100644 --- a/webrtc/video_engine/new_include/config.h +++ b/webrtc/video_engine/new_include/config.h @@ -29,15 +29,6 @@ struct RtpStatistics { std::string c_name; }; -namespace newapi { -// RTCP mode to use. Compound mode is described by RFC 4585 and reduced-size -// RTCP mode is described by RFC 5506. -enum RtcpMode { - kRtcpCompound, - kRtcpReducedSize -}; -} // namespace newapi - // Settings for NACK, see RFC 4585 for details. struct NackConfig { NackConfig() : rtp_history_ms(0) {} diff --git a/webrtc/video_engine/new_include/video_receive_stream.h b/webrtc/video_engine/new_include/video_receive_stream.h index 0235ee07c..ba0f260ad 100644 --- a/webrtc/video_engine/new_include/video_receive_stream.h +++ b/webrtc/video_engine/new_include/video_receive_stream.h @@ -22,6 +22,15 @@ namespace webrtc { +namespace newapi { +// RTCP mode to use. Compound mode is described by RFC 4585 and reduced-size +// RTCP mode is described by RFC 5506. +enum RtcpMode { + kRtcpCompound, + kRtcpReducedSize +}; +} // namespace newapi + class VideoDecoder; // TODO(mflodman) Move all these settings to VideoDecoder and move the @@ -96,11 +105,15 @@ class VideoReceiveStream { // Receive-stream specific RTP settings. struct Rtp { - Rtp() : ssrc(0) {} + Rtp() : ssrc(0), rtcp_mode(newapi::kRtcpReducedSize) {} + // TODO(mflodman) Do we require a set ssrc? What happens if the ssrc // changes? uint32_t ssrc; + // See RtcpMode for description. + newapi::RtcpMode rtcp_mode; + // See NackConfig for description. NackConfig nack; diff --git a/webrtc/video_engine/new_include/video_send_stream.h b/webrtc/video_engine/new_include/video_send_stream.h index 5c0ec3273..c85ed786a 100644 --- a/webrtc/video_engine/new_include/video_send_stream.h +++ b/webrtc/video_engine/new_include/video_send_stream.h @@ -86,9 +86,7 @@ class VideoSendStream { static const size_t kDefaultMaxPacketSize = 1500 - 40; // TCP over IPv4. struct Rtp { - Rtp() : mode(newapi::kRtcpReducedSize), - max_packet_size(kDefaultMaxPacketSize) {} - newapi::RtcpMode mode; + Rtp() : max_packet_size(kDefaultMaxPacketSize) {} std::vector ssrcs; diff --git a/webrtc/video_engine/test/call_tests.cc b/webrtc/video_engine/test/call_tests.cc index bb10906ad..2fd25f368 100644 --- a/webrtc/video_engine/test/call_tests.cc +++ b/webrtc/video_engine/test/call_tests.cc @@ -28,6 +28,9 @@ namespace webrtc { +static unsigned int kDefaultTimeoutMs = 30 * 1000; +static unsigned int kLongTimeoutMs = 120 * 1000; + class CallTest : public ::testing::Test { public: CallTest() @@ -106,6 +109,7 @@ class CallTest : public ::testing::Test { } void ReceivesPliAndRecovers(int rtp_history_ms); + void RespectsRtcpMode(newapi::RtcpMode rtcp_mode); scoped_ptr sender_call_; scoped_ptr receiver_call_; @@ -131,7 +135,7 @@ class NackObserver : public test::RtpRtcpObserver { public: NackObserver() - : test::RtpRtcpObserver(120 * 1000), + : test::RtpRtcpObserver(kLongTimeoutMs), rtp_parser_(RtpHeaderParser::Create()), drop_burst_count_(0), sent_rtp_packets_(0), @@ -242,7 +246,7 @@ TEST_F(CallTest, UsesTraceCallback) { done_->Set(); } - EventTypeWrapper Wait() { return done_->Wait(30 * 1000); } + EventTypeWrapper Wait() { return done_->Wait(kDefaultTimeoutMs); } private: unsigned int filter_; @@ -315,7 +319,7 @@ class PliObserver : public test::RtpRtcpObserver, public VideoRenderer { public: explicit PliObserver(bool nack_enabled) - : test::RtpRtcpObserver(120 * 1000), + : test::RtpRtcpObserver(kLongTimeoutMs), rtp_header_parser_(RtpHeaderParser::Create()), nack_enabled_(nack_enabled), first_retransmitted_timestamp_(0), @@ -428,7 +432,9 @@ TEST_F(CallTest, SurvivesIncomingRtpPacketsToDestroyedReceiveStream) { explicit PacketInputObserver(PacketReceiver* receiver) : receiver_(receiver), delivered_packet_(EventWrapper::Create()) {} - EventTypeWrapper Wait() { return delivered_packet_->Wait(30 * 1000); } + EventTypeWrapper Wait() { + return delivered_packet_->Wait(kDefaultTimeoutMs); + } private: virtual bool DeliverPacket(const uint8_t* packet, size_t length) { @@ -474,6 +480,100 @@ TEST_F(CallTest, SurvivesIncomingRtpPacketsToDestroyedReceiveStream) { receive_transport.StopSending(); } +void CallTest::RespectsRtcpMode(newapi::RtcpMode rtcp_mode) { + static const int kRtpHistoryMs = 1000; + static const int kNumCompoundRtcpPacketsToObserve = 10; + class RtcpModeObserver : public test::RtpRtcpObserver { + public: + RtcpModeObserver(newapi::RtcpMode rtcp_mode) + : test::RtpRtcpObserver(kDefaultTimeoutMs), + rtcp_mode_(rtcp_mode), + sent_rtp_(0), + sent_rtcp_(0) {} + + private: + virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE { + if (++sent_rtp_ % 3 == 0) + return DROP_PACKET; + + return SEND_PACKET; + } + + virtual Action OnReceiveRtcp(const uint8_t* packet, + size_t length) OVERRIDE { + ++sent_rtcp_; + RTCPUtility::RTCPParserV2 parser(packet, length, true); + EXPECT_TRUE(parser.IsValid()); + + RTCPUtility::RTCPPacketTypes packet_type = parser.Begin(); + bool has_report_block = false; + while (packet_type != RTCPUtility::kRtcpNotValidCode) { + EXPECT_NE(RTCPUtility::kRtcpSrCode, packet_type); + if (packet_type == RTCPUtility::kRtcpRrCode) { + has_report_block = true; + break; + } + packet_type = parser.Iterate(); + } + + switch (rtcp_mode_) { + case newapi::kRtcpCompound: + if (!has_report_block) { + ADD_FAILURE() << "Received RTCP packet without receiver report for " + "kRtcpCompound."; + observation_complete_->Set(); + } + + if (sent_rtcp_ >= kNumCompoundRtcpPacketsToObserve) + observation_complete_->Set(); + + break; + case newapi::kRtcpReducedSize: + if (!has_report_block) + observation_complete_->Set(); + break; + } + + return SEND_PACKET; + } + + newapi::RtcpMode rtcp_mode_; + int sent_rtp_; + int sent_rtcp_; + } observer(rtcp_mode); + + CreateCalls(Call::Config(observer.SendTransport()), + Call::Config(observer.ReceiveTransport())); + + observer.SetReceivers(receiver_call_->Receiver(), sender_call_->Receiver()); + + CreateTestConfigs(); + send_config_.rtp.nack.rtp_history_ms = kRtpHistoryMs; + receive_config_.rtp.nack.rtp_history_ms = kRtpHistoryMs; + receive_config_.rtp.rtcp_mode = rtcp_mode; + + CreateStreams(); + CreateFrameGenerator(); + StartSending(); + + EXPECT_EQ(kEventSignaled, observer.Wait()) + << (rtcp_mode == newapi::kRtcpCompound + ? "Timed out before observing enough compound packets." + : "Timed out before receiving a non-compound RTCP packet."); + + StopSending(); + observer.StopSending(); + DestroyStreams(); +} + +TEST_F(CallTest, UsesRtcpCompoundMode) { + RespectsRtcpMode(newapi::kRtcpCompound); +} + +TEST_F(CallTest, UsesRtcpReducedSizeMode) { + RespectsRtcpMode(newapi::kRtcpReducedSize); +} + // Test sets up a Call multiple senders with different resolutions and SSRCs. // Another is set up to receive all three of these with different renderers. // Each renderer verifies that it receives the expected resolution, and as soon @@ -493,7 +593,7 @@ TEST_F(CallTest, SendsAndReceivesMultipleStreams) { done_->Set(); } - void Wait() { done_->Wait(30 * 1000); } + void Wait() { done_->Wait(kDefaultTimeoutMs); } private: int width_;