From d9b9560ee50c236efcb690ee479021b415f7dfd4 Mon Sep 17 00:00:00 2001 From: "sprang@webrtc.org" Date: Mon, 27 Jan 2014 13:03:02 +0000 Subject: [PATCH] Drop early packets when not sending in TransportAdapter. Particularly, suppress periodic RTCP packets before VideoSendStream.StartSending() or VideoReceiveStream.StartReceiving() have been called, respectively. RTCP packets are sent periodically, by the Process thread, for every ViE channel even those not sending. BUG= R=pbos@webrtc.org, stefan@webrtc.org Review URL: https://webrtc-codereview.appspot.com/7569004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5438 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/video/call_perf_tests.cc | 1 + webrtc/video/rampup_tests.cc | 1 + webrtc/video/transport_adapter.cc | 16 +++++++++++++++- webrtc/video/transport_adapter.h | 5 +++++ webrtc/video/video_receive_stream.cc | 2 ++ webrtc/video/video_send_stream.cc | 2 ++ webrtc/video/video_send_stream_tests.cc | 16 ++++++++++++---- 7 files changed, 38 insertions(+), 5 deletions(-) diff --git a/webrtc/video/call_perf_tests.cc b/webrtc/video/call_perf_tests.cc index 0637ec3ee7..8234bf53f1 100644 --- a/webrtc/video/call_perf_tests.cc +++ b/webrtc/video/call_perf_tests.cc @@ -230,6 +230,7 @@ TEST_F(CallPerfTest, PlaysOutAudioAndVideoInSync) { audio_observer.SetReceivers(&voe_packet_receiver, &voe_packet_receiver); internal::TransportAdapter transport_adapter(audio_observer.SendTransport()); + transport_adapter.Enable(); EXPECT_EQ(0, voe_network->RegisterExternalTransport(channel, transport_adapter)); diff --git a/webrtc/video/rampup_tests.cc b/webrtc/video/rampup_tests.cc index 7486357859..266172d4de 100644 --- a/webrtc/video/rampup_tests.cc +++ b/webrtc/video/rampup_tests.cc @@ -71,6 +71,7 @@ class StreamObserver : public newapi::Transport, public RemoteBitrateObserver { // be able to produce an RTCP with REMB. RtpRtcp::Configuration config; config.receive_statistics = receive_stats_.get(); + feedback_transport_.Enable(); config.outgoing_transport = &feedback_transport_; rtp_rtcp_.reset(RtpRtcp::CreateRtpRtcp(config)); rtp_rtcp_->SetREMBStatus(true); diff --git a/webrtc/video/transport_adapter.cc b/webrtc/video/transport_adapter.cc index 7cc6a0a434..6f27d9972a 100644 --- a/webrtc/video/transport_adapter.cc +++ b/webrtc/video/transport_adapter.cc @@ -14,11 +14,14 @@ namespace webrtc { namespace internal { TransportAdapter::TransportAdapter(newapi::Transport* transport) - : transport_(transport) {} + : transport_(transport), enabled_(0) {} int TransportAdapter::SendPacket(int /*channel*/, const void* packet, int length) { + if (enabled_.Value() == 0) + return false; + bool success = transport_->SendRtp(static_cast(packet), static_cast(length)); return success ? length : -1; @@ -27,10 +30,21 @@ int TransportAdapter::SendPacket(int /*channel*/, int TransportAdapter::SendRTCPPacket(int /*channel*/, const void* packet, int length) { + if (enabled_.Value() == 0) + return false; + bool success = transport_->SendRtcp(static_cast(packet), static_cast(length)); return success ? length : -1; } +void TransportAdapter::Enable() { + // If this exchange fails it means enabled_ was already true, no need to + // check result and iterate. + enabled_.CompareExchange(1, 0); +} + +void TransportAdapter::Disable() { enabled_.CompareExchange(0, 1); } + } // namespace internal } // namespace webrtc diff --git a/webrtc/video/transport_adapter.h b/webrtc/video/transport_adapter.h index 3686f38a92..79f995be9b 100644 --- a/webrtc/video/transport_adapter.h +++ b/webrtc/video/transport_adapter.h @@ -11,6 +11,7 @@ #define WEBRTC_VIDEO_ENGINE_INTERNAL_TRANSPORT_ADAPTER_H_ #include "webrtc/common_types.h" +#include "webrtc/system_wrappers/interface/atomic32.h" #include "webrtc/transport.h" namespace webrtc { @@ -25,8 +26,12 @@ class TransportAdapter : public webrtc::Transport { virtual int SendRTCPPacket(int /*channel*/, const void* packet, int length) OVERRIDE; + void Enable(); + void Disable(); + private: newapi::Transport *transport_; + Atomic32 enabled_; }; } // namespace internal } // namespace webrtc diff --git a/webrtc/video/video_receive_stream.cc b/webrtc/video/video_receive_stream.cc index 38947c4ac0..cc735d2a6f 100644 --- a/webrtc/video/video_receive_stream.cc +++ b/webrtc/video/video_receive_stream.cc @@ -168,6 +168,7 @@ VideoReceiveStream::~VideoReceiveStream() { } void VideoReceiveStream::StartReceiving() { + transport_adapter_.Enable(); if (render_->StartRender(channel_) != 0) abort(); if (video_engine_base_->StartReceive(channel_) != 0) @@ -179,6 +180,7 @@ void VideoReceiveStream::StopReceiving() { abort(); if (video_engine_base_->StopReceive(channel_) != 0) abort(); + transport_adapter_.Disable(); } void VideoReceiveStream::GetCurrentReceiveCodec(VideoCodec* receive_codec) { diff --git a/webrtc/video/video_send_stream.cc b/webrtc/video/video_send_stream.cc index a9fd50dfa7..3b592f818c 100644 --- a/webrtc/video/video_send_stream.cc +++ b/webrtc/video/video_send_stream.cc @@ -259,6 +259,7 @@ void VideoSendStream::SwapFrame(I420VideoFrame* frame) { VideoSendStreamInput* VideoSendStream::Input() { return this; } void VideoSendStream::StartSending() { + transport_adapter_.Enable(); video_engine_base_->StartSend(channel_); video_engine_base_->StartReceive(channel_); } @@ -266,6 +267,7 @@ void VideoSendStream::StartSending() { void VideoSendStream::StopSending() { video_engine_base_->StopSend(channel_); video_engine_base_->StopReceive(channel_); + transport_adapter_.Disable(); } bool VideoSendStream::SetCodec(const VideoCodec& codec) { diff --git a/webrtc/video/video_send_stream_tests.cc b/webrtc/video/video_send_stream_tests.cc index 3eb4f90e84..d3333e0f35 100644 --- a/webrtc/video/video_send_stream_tests.cc +++ b/webrtc/video/video_send_stream_tests.cc @@ -433,7 +433,9 @@ TEST_F(VideoSendStreamTest, SupportsFec) { transport_adapter_(SendTransport()), send_count_(0), received_media_(false), - received_fec_(false) {} + received_fec_(false) { + transport_adapter_.Enable(); + } virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE { RTPHeader header; @@ -504,7 +506,9 @@ void VideoSendStreamTest::TestNackRetransmission( send_count_(0), retransmit_ssrc_(retransmit_ssrc), retransmit_payload_type_(retransmit_payload_type), - nacked_sequence_number_(-1) {} + nacked_sequence_number_(-1) { + transport_adapter_.Enable(); + } virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE { RTPHeader header; @@ -764,7 +768,9 @@ TEST_F(VideoSendStreamTest, SuspendBelowMinBitrate) { suspended_frame_count_(0), low_remb_bps_(0), high_remb_bps_(0), - crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) {} + crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) { + transport_adapter_.Enable(); + } void SetReceiver(PacketReceiver* receiver) { transport_.SetReceiver(receiver); @@ -890,7 +896,9 @@ TEST_F(VideoSendStreamTest, NoPaddingWhenVideoIsMuted) { last_packet_time_ms_(-1), transport_adapter_(ReceiveTransport()), capturer_(NULL), - crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) {} + crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) { + transport_adapter_.Enable(); + } void SetCapturer(test::FrameGeneratorCapturer* capturer) { capturer_ = capturer;