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..b2b081c886b64cb64aa0975bb19312de5ca55dc6 100644
--- a/webrtc/voice_engine/channel.cc
+++ b/webrtc/voice_engine/channel.cc
@@ -21,6 +21,7 @@
 #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/paced_sender.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"
@@ -43,6 +44,100 @@
 
 namespace webrtc {
 namespace voe {
+namespace {
+
+class PacketSenderProxy : public RtpPacketSender,
+                          public TransportFeedbackObserver,
+                          public TransportSequenceNumberAllocator {
+ public:
+  PacketSenderProxy()
+      : packet_sender_(nullptr),
+        feedback_observer_(nullptr),
+        seq_num_allocator_(nullptr) {
+    encoder_thread_.DetachFromThread();
+    pacer_thread_.DetachFromThread();
+    network_thread_.DetachFromThread();
+  }
+
+  void SetCongestionControlObjects(
+      RtpPacketSender* rtp_packet_sender,
+      TransportFeedbackObserver* transport_feedback_observer,
+      TransportSequenceNumberAllocator* seq_num_allocator) {
+    RTC_DCHECK(thread_checker_.CalledOnValidThread());
+    rtc::CritScope lock(&crit_);
+    packet_sender_ = rtp_packet_sender;
+    feedback_observer_ = transport_feedback_observer;
+    seq_num_allocator_ = seq_num_allocator;
+  }
+
+  // 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,
+                                capture_time_ms, bytes, retransmission);
+  }
+
+  // Implements TransportFeedbackObserver.
+  void AddPacket(uint16_t sequence_number,
+                 size_t length,
+                 bool was_paced) override {
+    RTC_DCHECK(pacer_thread_.CalledOnValidThread());
+    TransportFeedbackObserver* feedback_observer;
+    {
+      rtc::CritScope lock(&crit_);
+      if (feedback_observer_ == nullptr)
+        return;
+      feedback_observer = feedback_observer_;
+    }
+    feedback_observer->AddPacket(sequence_number, length, was_paced);
+  }
+  void OnTransportFeedback(const rtcp::TransportFeedback& feedback) override {
+    RTC_DCHECK(network_thread_.CalledOnValidThread());
+    TransportFeedbackObserver* feedback_observer;
+    {
+      rtc::CritScope lock(&crit_);
+      if (feedback_observer_ == nullptr)
+        return;
+      feedback_observer = feedback_observer_;
+    }
+    feedback_observer->OnTransportFeedback(feedback);
+  }
+
+  // Implements TransportSequenceNumberAllocator.
+  uint16_t AllocateSequenceNumber() override {
+    RTC_DCHECK(pacer_thread_.CalledOnValidThread());
+    TransportSequenceNumberAllocator* seq_num_allocator;
+    {
+      rtc::CritScope lock(&crit_);
+      RTC_DCHECK(seq_num_allocator_ != nullptr);
+      seq_num_allocator = seq_num_allocator_;
+    }
+    return seq_num_allocator->AllocateSequenceNumber();
+  }
+
+ private:

[the sun, 2015/12/01 10:25:36]

Wow! So with this critsect, this class becomes a common choke for contention
across all of these 4 threads?

To me, that is sign that the rtp module should instead be blessed with separate
CSs for the three pointers that can be mutated. Then there'd only be contention
between the worker thread and either of the others. Not between all of them.

Another option would be to recreate the rtp module (and keep it in Channel as a
scoped_ptr). The recreation will happen very shortly after the channel is
created, so any state the rtp module has collected won't matter much, if at all.

And thanks for adding the TCs, they really revealed stuff I wasn't fully aware
of.

[stefan-webrtc, 2015/12/01 16:19:33]

Right, not very nice. I redid it to only copy the pointers in the choke, but
that will only work if we know that audio stops being encoded before we tear
down the audio send stream. Do we know that? In that case this choke is cheap.
Basically same thing but with three locks? I'd prefer not to introduce
complexity in the rtp module which we have worked hard to get rid of.
I like this option, and that is basically what we do in video engine. If you
think that should be fine for voice I wouldn't mind doing that. But won't there
be a problem where the channel can be started (and therefore is already sending
packets) when we try to tear down the rtp module? I could possibly stop and
restart the channel, but I'm not very comfortable doing things like that since I
don't know voice engine too well...

[the sun, 2015/12/01 16:48:23]

I think we might be able to get away with it in this case. Look at
WebRtcVoiceMediaChannel::AddSendStream().
 
The VoE channel is created:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...
then just a few lines after the AudioSendStream is created:
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/libjin...

The stream has not been started yet. So if you could break out the
creation/config of the RtpRtcp module and call that from the ctor and
SetCongestionControlObjects() I *think* we should be fine. Just leave a BIG TODO
for me, and RTC_DCHECK(!channel_state_.Get().sending).

[stefan-webrtc, 2015/12/02 16:14:17]

Done.

+  rtc::ThreadChecker thread_checker_;
+  rtc::ThreadChecker encoder_thread_;
+  rtc::ThreadChecker pacer_thread_;
+  rtc::ThreadChecker network_thread_;
+  rtc::CriticalSection crit_;
+  RtpPacketSender* packet_sender_ GUARDED_BY(&crit_);
+  TransportFeedbackObserver* feedback_observer_ GUARDED_BY(&crit_);
+  TransportSequenceNumberAllocator* seq_num_allocator_ GUARDED_BY(&crit_);
+};
+}  // namespace
 
 // Extend the default RTCP statistics struct with max_jitter, defined as the
 // maximum jitter value seen in an RTCP report block.
@@ -690,89 +785,94 @@ 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)),
+      packet_sender_proxy_(config.Get<VoicePacing>().enabled
+                               ? new PacketSenderProxy()
+                               : nullptr),
+      packet_router_(nullptr) {
     WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId,_channelId),
                  "Channel::Channel() - ctor");
     AudioCodingModule::Config acm_config;
@@ -797,6 +897,7 @@ 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();
 
     _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
 
@@ -824,6 +925,8 @@ Channel::~Channel()
         DeRegisterExternalMediaProcessing(kRecordingPerChannel);
     }
     StopSend();
+    if (packet_router_ != nullptr)
+      packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
     StopPlayout();
 
     {
@@ -2784,12 +2887,35 @@ int Channel::SetReceiveAbsoluteSenderTimeStatus(bool enable, unsigned char id) {
   return 0;
 }
 
+void Channel::SetSendTransportSequenceNumber(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(packet_sender_proxy_.get());
+  RTC_DCHECK(packet_router != nullptr || packet_router_ != nullptr);
+  packet_sender_proxy_->SetCongestionControlObjects(
+      rtp_packet_sender, transport_feedback_observer, packet_router);
+  _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)
 {