Prepare the AudioSendStream to be hooked up to send-side BWE.

webrtc/voice_engine/channel.cc

Index: webrtc/voice_engine/channel.cc
diff --git a/webrtc/voice_engine/channel.cc b/webrtc/voice_engine/channel.cc
index 54aa802d73e4ccbca87d0f93cdeb73b536ad3329..e2de4dbad95d94f03b3c782b39211d3f57948a71 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -15,12 +15,14 @@
 #include "webrtc/base/checks.h"
 #include "webrtc/base/format_macros.h"
 #include "webrtc/base/logging.h"
+#include "webrtc/base/thread_checker.h"
 #include "webrtc/base/timeutils.h"
 #include "webrtc/common.h"
 #include "webrtc/config.h"
 #include "webrtc/modules/audio_device/include/audio_device.h"
 #include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/include/module_common_types.h"
+#include "webrtc/modules/pacing/packet_router.h"
 #include "webrtc/modules/rtp_rtcp/include/receive_statistics.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_payload_registry.h"
 #include "webrtc/modules/rtp_rtcp/include/rtp_receiver.h"
@@ -44,6 +46,108 @@
 namespace webrtc {
 namespace voe {
 

[the sun, 2015/12/04 12:04:48]

I like this split into three. It avoids most of the potential for congestion and
is not invasive on the RtpRtcp module (like adding the locks there would be).
Good compromise!

[stefan-webrtc, 2015/12/04 13:12:39]

Acknowledged.

+class TransportFeedbackProxy : public TransportFeedbackObserver {
+ public:
+  TransportFeedbackProxy() : feedback_observer_(nullptr) {
+    pacer_thread_.DetachFromThread();
+    network_thread_.DetachFromThread();
+  }
+
+  void SetTransportFeedbackObserver(
+      TransportFeedbackObserver* feedback_observer) {
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    feedback_observer_ = feedback_observer;
+  }
+
+  // Implements TransportFeedbackObserver.
+  void AddPacket(uint16_t sequence_number,
+                 size_t length,
+                 bool was_paced) override {
+    RTC_DCHECK(pacer_thread_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);

[the sun, 2015/12/04 12:04:48]

Don't you need to
if (feedback_observer_) {
  feedback_observer_->AddPacket(...)
}
??
here and below?

[stefan-webrtc, 2015/12/04 13:12:39]

Not in practice since we only use these methods if we are also using
AudioSendStream, and then we always set these. This is at least true if we never
send packets before we have created the audio send stream. 

But maybe it's good to add the ifs anyway...

[the sun, 2015/12/04 14:01:57]

Well, you're setting them to nullptr when the AudioSendStream is destroyed,
which is a little before the channel is deleted, so a context switch could
happen in between.

+    feedback_observer_->AddPacket(sequence_number, length, was_paced);
+  }
+  void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {
+    RTC_DCHECK(network_thread_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    feedback_observer_->OnTransportFeedback(feedback);
+  }
+
+ private:
+  rtc::CriticalSection crit_;
+  rtc::ThreadChecker thread_checker_;
+  rtc::ThreadChecker pacer_thread_;
+  rtc::ThreadChecker network_thread_;
+  TransportFeedbackObserver* feedback_observer_ GUARDED_BY(&crit_);
+};
+
+class TransportSequenceNumberProxy : public TransportSequenceNumberAllocator {
+ public:
+  TransportSequenceNumberProxy() : seq_num_allocator_(nullptr) {
+    pacer_thread_.DetachFromThread();
+  }
+
+  void SetSequenceNumberAllocator(
+      TransportSequenceNumberAllocator* seq_num_allocator) {
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    seq_num_allocator_ = seq_num_allocator;
+  }
+
+  // Implements TransportSequenceNumberAllocator.
+  uint16_t AllocateSequenceNumber() override {
+    RTC_DCHECK(pacer_thread_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    RTC_DCHECK(seq_num_allocator_ != nullptr);

[the sun, 2015/12/04 12:04:48]

nit: drop " != nullptr"

[stefan-webrtc, 2015/12/04 13:12:39]

Done.

+    return seq_num_allocator_->AllocateSequenceNumber();
+  }
+
+ private:
+  rtc::CriticalSection crit_;
+  rtc::ThreadChecker thread_checker_;
+  rtc::ThreadChecker pacer_thread_;
+  TransportSequenceNumberAllocator* seq_num_allocator_ GUARDED_BY(&crit_);
+};
+
+class PacketSenderProxy : public RtpPacketSender {
+ public:
+  PacketSenderProxy() : packet_sender_(nullptr) {
+    encoder_thread_.DetachFromThread();
+  }
+
+  void SetPacketSender(RtpPacketSender* rtp_packet_sender) {
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    packet_sender_ = rtp_packet_sender;
+  }
+
+  // Implements RtpPacketSender.
+  void InsertPacket(Priority priority,
+                    uint32_t ssrc,
+                    uint16_t sequence_number,
+                    int64_t capture_time_ms,
+                    size_t bytes,
+                    bool retransmission) override {
+    RTC_DCHECK(encoder_thread_.CalledOnValidThread());
+    RtpPacketSender* packet_sender;
+    {
+      rtc::CritScope lock(&crit_);
+      if (packet_sender_ == nullptr)
+        return;
+      packet_sender = packet_sender_;
+    }
+    packet_sender->InsertPacket(priority, ssrc, sequence_number,

[the sun, 2015/12/04 12:04:48]

You can't do this reliably since someone else owns the packet_sender. You need
to call the method on packet_sender_ while holding the lock.

[stefan-webrtc, 2015/12/04 13:12:39]

Ack, I accidentally left this.

+                                capture_time_ms, bytes, retransmission);
+  }
+
+ private:
+  rtc::ThreadChecker thread_checker_;
+  rtc::ThreadChecker encoder_thread_;
+  rtc::CriticalSection crit_;
+  RtpPacketSender* packet_sender_ GUARDED_BY(&crit_);

[the sun, 2015/12/04 12:04:48]

nit: might as well be consistent and prefix "rtp_"

[stefan-webrtc, 2015/12/04 13:12:39]

Done.

+};
+
 // Extend the default RTCP statistics struct with max_jitter, defined as the
 // maximum jitter value seen in an RTCP report block.
 struct ChannelStatistics : public RtcpStatistics {
@@ -690,89 +794,97 @@ Channel::Channel(int32_t channelId,
                  uint32_t instanceId,
                  RtcEventLog* const event_log,
                  const Config& config)
-  : _fileCritSect(*CriticalSectionWrapper::CreateCriticalSection()),
-    _callbackCritSect(*CriticalSectionWrapper::CreateCriticalSection()),
-    volume_settings_critsect_(*CriticalSectionWrapper::CreateCriticalSection()),
-    _instanceId(instanceId),
-    _channelId(channelId),
-    event_log_(event_log),
-    rtp_header_parser_(RtpHeaderParser::Create()),
-    rtp_payload_registry_(
-        new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))),
-    rtp_receive_statistics_(
-        ReceiveStatistics::Create(Clock::GetRealTimeClock())),
-    rtp_receiver_(
-        RtpReceiver::CreateAudioReceiver(Clock::GetRealTimeClock(),
-                                         this,
-                                         this,
-                                         this,
-                                         rtp_payload_registry_.get())),
-    telephone_event_handler_(rtp_receiver_->GetTelephoneEventHandler()),
-    _outputAudioLevel(),
-    _externalTransport(false),
-    _inputFilePlayerPtr(NULL),
-    _outputFilePlayerPtr(NULL),
-    _outputFileRecorderPtr(NULL),
-    // Avoid conflict with other channels by adding 1024 - 1026,
-    // won't use as much as 1024 channels.
-    _inputFilePlayerId(VoEModuleId(instanceId, channelId) + 1024),
-    _outputFilePlayerId(VoEModuleId(instanceId, channelId) + 1025),
-    _outputFileRecorderId(VoEModuleId(instanceId, channelId) + 1026),
-    _outputFileRecording(false),
-    _inbandDtmfQueue(VoEModuleId(instanceId, channelId)),
-    _inbandDtmfGenerator(VoEModuleId(instanceId, channelId)),
-    _outputExternalMedia(false),
-    _inputExternalMediaCallbackPtr(NULL),
-    _outputExternalMediaCallbackPtr(NULL),
-    _timeStamp(0),  // This is just an offset, RTP module will add it's own
-                    // random offset
-    _sendTelephoneEventPayloadType(106),
-    ntp_estimator_(Clock::GetRealTimeClock()),
-    jitter_buffer_playout_timestamp_(0),
-    playout_timestamp_rtp_(0),
-    playout_timestamp_rtcp_(0),
-    playout_delay_ms_(0),
-    _numberOfDiscardedPackets(0),
-    send_sequence_number_(0),
-    ts_stats_lock_(CriticalSectionWrapper::CreateCriticalSection()),
-    rtp_ts_wraparound_handler_(new rtc::TimestampWrapAroundHandler()),
-    capture_start_rtp_time_stamp_(-1),
-    capture_start_ntp_time_ms_(-1),
-    _engineStatisticsPtr(NULL),
-    _outputMixerPtr(NULL),
-    _transmitMixerPtr(NULL),
-    _moduleProcessThreadPtr(NULL),
-    _audioDeviceModulePtr(NULL),
-    _voiceEngineObserverPtr(NULL),
-    _callbackCritSectPtr(NULL),
-    _transportPtr(NULL),
-    _rxVadObserverPtr(NULL),
-    _oldVadDecision(-1),
-    _sendFrameType(0),
-    _externalMixing(false),
-    _mixFileWithMicrophone(false),
-    _mute(false),
-    _panLeft(1.0f),
-    _panRight(1.0f),
-    _outputGain(1.0f),
-    _playOutbandDtmfEvent(false),
-    _playInbandDtmfEvent(false),
-    _lastLocalTimeStamp(0),
-    _lastPayloadType(0),
-    _includeAudioLevelIndication(false),
-    _outputSpeechType(AudioFrame::kNormalSpeech),
-    video_sync_lock_(CriticalSectionWrapper::CreateCriticalSection()),
-    _average_jitter_buffer_delay_us(0),
-    _previousTimestamp(0),
-    _recPacketDelayMs(20),
-    _RxVadDetection(false),
-    _rxAgcIsEnabled(false),
-    _rxNsIsEnabled(false),
-    restored_packet_in_use_(false),
-    rtcp_observer_(new VoERtcpObserver(this)),
-    network_predictor_(new NetworkPredictor(Clock::GetRealTimeClock())),
-    assoc_send_channel_lock_(CriticalSectionWrapper::CreateCriticalSection()),
-    associate_send_channel_(ChannelOwner(nullptr)) {
+    : _fileCritSect(*CriticalSectionWrapper::CreateCriticalSection()),
+      _callbackCritSect(*CriticalSectionWrapper::CreateCriticalSection()),
+      volume_settings_critsect_(
+          *CriticalSectionWrapper::CreateCriticalSection()),
+      _instanceId(instanceId),
+      _channelId(channelId),
+      event_log_(event_log),
+      rtp_header_parser_(RtpHeaderParser::Create()),
+      rtp_payload_registry_(
+          new RTPPayloadRegistry(RTPPayloadStrategy::CreateStrategy(true))),
+      rtp_receive_statistics_(
+          ReceiveStatistics::Create(Clock::GetRealTimeClock())),
+      rtp_receiver_(
+          RtpReceiver::CreateAudioReceiver(Clock::GetRealTimeClock(),
+                                           this,
+                                           this,
+                                           this,
+                                           rtp_payload_registry_.get())),
+      telephone_event_handler_(rtp_receiver_->GetTelephoneEventHandler()),
+      _outputAudioLevel(),
+      _externalTransport(false),
+      _inputFilePlayerPtr(NULL),
+      _outputFilePlayerPtr(NULL),
+      _outputFileRecorderPtr(NULL),
+      // Avoid conflict with other channels by adding 1024 - 1026,
+      // won't use as much as 1024 channels.
+      _inputFilePlayerId(VoEModuleId(instanceId, channelId) + 1024),
+      _outputFilePlayerId(VoEModuleId(instanceId, channelId) + 1025),
+      _outputFileRecorderId(VoEModuleId(instanceId, channelId) + 1026),
+      _outputFileRecording(false),
+      _inbandDtmfQueue(VoEModuleId(instanceId, channelId)),
+      _inbandDtmfGenerator(VoEModuleId(instanceId, channelId)),
+      _outputExternalMedia(false),
+      _inputExternalMediaCallbackPtr(NULL),
+      _outputExternalMediaCallbackPtr(NULL),
+      _timeStamp(0),  // This is just an offset, RTP module will add it's own
+                      // random offset
+      _sendTelephoneEventPayloadType(106),
+      ntp_estimator_(Clock::GetRealTimeClock()),
+      jitter_buffer_playout_timestamp_(0),
+      playout_timestamp_rtp_(0),
+      playout_timestamp_rtcp_(0),
+      playout_delay_ms_(0),
+      _numberOfDiscardedPackets(0),
+      send_sequence_number_(0),
+      ts_stats_lock_(CriticalSectionWrapper::CreateCriticalSection()),
+      rtp_ts_wraparound_handler_(new rtc::TimestampWrapAroundHandler()),
+      capture_start_rtp_time_stamp_(-1),
+      capture_start_ntp_time_ms_(-1),
+      _engineStatisticsPtr(NULL),
+      _outputMixerPtr(NULL),
+      _transmitMixerPtr(NULL),
+      _moduleProcessThreadPtr(NULL),
+      _audioDeviceModulePtr(NULL),
+      _voiceEngineObserverPtr(NULL),
+      _callbackCritSectPtr(NULL),
+      _transportPtr(NULL),
+      _rxVadObserverPtr(NULL),
+      _oldVadDecision(-1),
+      _sendFrameType(0),
+      _externalMixing(false),
+      _mixFileWithMicrophone(false),
+      _mute(false),
+      _panLeft(1.0f),
+      _panRight(1.0f),
+      _outputGain(1.0f),
+      _playOutbandDtmfEvent(false),
+      _playInbandDtmfEvent(false),
+      _lastLocalTimeStamp(0),
+      _lastPayloadType(0),
+      _includeAudioLevelIndication(false),
+      _outputSpeechType(AudioFrame::kNormalSpeech),
+      video_sync_lock_(CriticalSectionWrapper::CreateCriticalSection()),
+      _average_jitter_buffer_delay_us(0),
+      _previousTimestamp(0),
+      _recPacketDelayMs(20),
+      _RxVadDetection(false),
+      _rxAgcIsEnabled(false),
+      _rxNsIsEnabled(false),
+      restored_packet_in_use_(false),
+      rtcp_observer_(new VoERtcpObserver(this)),
+      network_predictor_(new NetworkPredictor(Clock::GetRealTimeClock())),
+      assoc_send_channel_lock_(CriticalSectionWrapper::CreateCriticalSection()),
+      associate_send_channel_(ChannelOwner(nullptr)),
+      pacing_enabled_(config.Get<VoicePacing>().enabled),
+      feedback_observer_proxy_(pacing_enabled_ ? new TransportFeedbackProxy()
+                                               : nullptr),
+      seq_num_allocator_proxy_(
+          pacing_enabled_ ? new TransportSequenceNumberProxy() : nullptr),
+      packet_sender_proxy_(pacing_enabled_ ? new PacketSenderProxy()
+                                           : nullptr) {
     WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId,_channelId),
                  "Channel::Channel() - ctor");
     AudioCodingModule::Config acm_config;
@@ -797,6 +909,10 @@ Channel::Channel(int32_t channelId,
     configuration.audio_messages = this;
     configuration.receive_statistics = rtp_receive_statistics_.get();
     configuration.bandwidth_callback = rtcp_observer_.get();
+    configuration.paced_sender = packet_sender_proxy_.get();
+    configuration.transport_sequence_number_allocator =
+        seq_num_allocator_proxy_.get();
+    configuration.transport_feedback_callback = feedback_observer_proxy_.get();
 
     _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
 
@@ -2784,12 +2900,39 @@ int Channel::SetReceiveAbsoluteSenderTimeStatus(bool enable, unsigned char id) {
   return 0;
 }
 
+void Channel::EnableSendTransportSequenceNumber(int id) {
+  int ret =
+      SetSendRtpHeaderExtension(true, kRtpExtensionTransportSequenceNumber, id);
+  RTC_DCHECK_EQ(0, ret);
+}
+
 void Channel::SetRTCPStatus(bool enable) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::SetRTCPStatus()");
   _rtpRtcpModule->SetRTCPStatus(enable ? RtcpMode::kCompound : RtcpMode::kOff);
 }
 
+void Channel::SetCongestionControlObjects(
+    RtpPacketSender* rtp_packet_sender,
+    TransportFeedbackObserver* transport_feedback_observer,
+    PacketRouter* packet_router) {
+  RTC_DCHECK(feedback_observer_proxy_.get());
+  RTC_DCHECK(seq_num_allocator_proxy_.get());
+  RTC_DCHECK(packet_sender_proxy_.get());
+  RTC_DCHECK(packet_router != nullptr || packet_router_ != nullptr);
+  feedback_observer_proxy_->SetTransportFeedbackObserver(
+      transport_feedback_observer);
+  seq_num_allocator_proxy_->SetSequenceNumberAllocator(packet_router);
+  packet_sender_proxy_->SetPacketSender(rtp_packet_sender);
+  _rtpRtcpModule->SetStorePacketsStatus(rtp_packet_sender != nullptr, 600);
+  if (packet_router != nullptr) {
+    packet_router->AddRtpModule(_rtpRtcpModule.get());
+  } else {
+    packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
+  }
+  packet_router_ = packet_router;
+}
+
 int
 Channel::GetRTCPStatus(bool& enabled)
 {
@@ -3162,7 +3305,9 @@ bool Channel::GetCodecFECStatus() {
 
 void Channel::SetNACKStatus(bool enable, int maxNumberOfPackets) {
   // None of these functions can fail.
-  _rtpRtcpModule->SetStorePacketsStatus(enable, maxNumberOfPackets);
+  // If pacing is enabled we always store packets.
+  if (!pacing_enabled_)
+    _rtpRtcpModule->SetStorePacketsStatus(enable, maxNumberOfPackets);
   rtp_receive_statistics_->SetMaxReorderingThreshold(maxNumberOfPackets);
   rtp_receiver_->SetNACKStatus(enable ? kNackRtcp : kNackOff);
   if (enable)