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..86c0c554ba85d2d3af1128bdd4a5356700825c00 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"
@@ -690,89 +691,91 @@ 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_router_(nullptr) {
     WEBRTC_TRACE(kTraceMemory, kTraceVoice, VoEId(_instanceId,_channelId),
                  "Channel::Channel() - ctor");
     AudioCodingModule::Config acm_config;
@@ -791,14 +794,7 @@ Channel::Channel(int32_t channelId,
     _inbandDtmfGenerator.Init();
     _outputAudioLevel.Clear();
 
-    RtpRtcp::Configuration configuration;
-    configuration.audio = true;
-    configuration.outgoing_transport = this;
-    configuration.audio_messages = this;
-    configuration.receive_statistics = rtp_receive_statistics_.get();
-    configuration.bandwidth_callback = rtcp_observer_.get();
-
-    _rtpRtcpModule.reset(RtpRtcp::CreateRtpRtcp(configuration));
+    _rtpRtcpModule.reset(CreateRtpRtcp(nullptr, nullptr, nullptr));
 
     statistics_proxy_.reset(new StatisticsProxy(_rtpRtcpModule->SSRC()));
     rtp_receive_statistics_->RegisterRtcpStatisticsCallback(
@@ -824,6 +820,8 @@ Channel::~Channel()
         DeRegisterExternalMediaProcessing(kRecordingPerChannel);
     }
     StopSend();
+    if (packet_router_ != nullptr)

[mflodman, 2015/12/03 09:24:21]

I prefer 'if (packet_router)', but I'm not sure about the gerneal usage of this
expression in the voice code.

Same further down in this file.

[stefan-webrtc, 2015/12/03 10:18:27]

I also prefer that actually.

[the sun, 2015/12/03 11:10:28]

+1

+      packet_router_->RemoveRtpModule(_rtpRtcpModule.get());
     StopPlayout();
 
     {
@@ -880,6 +878,23 @@ Channel::~Channel()
     delete &volume_settings_critsect_;
 }
 
+RtpRtcp* Channel::CreateRtpRtcp(
+    RtpPacketSender* packet_sender,
+    TransportSequenceNumberAllocator* sequence_number_allocator,
+    TransportFeedbackObserver* transport_feedback_callback) {
+  RtpRtcp::Configuration configuration;
+  configuration.audio = true;
+  configuration.outgoing_transport = this;
+  configuration.audio_messages = this;
+  configuration.receive_statistics = rtp_receive_statistics_.get();
+  configuration.bandwidth_callback = rtcp_observer_.get();
+  configuration.paced_sender = packet_sender;
+  configuration.transport_sequence_number_allocator = sequence_number_allocator;
+  configuration.transport_feedback_callback = transport_feedback_callback;
+
+  return RtpRtcp::CreateRtpRtcp(configuration);
+}
+
 int32_t
 Channel::Init()
 {
@@ -2784,12 +2799,37 @@ int Channel::SetReceiveAbsoluteSenderTimeStatus(bool enable, unsigned char id) {
   return 0;
 }
 
+void Channel::SetSendTransportSequenceNumber(int id) {

[mflodman, 2015/12/03 09:24:21]

This is a bit ambiguous, might be interpreted as settting a sequence number and
not enabling the extension. And since there is no way to disable, only enable,
maybe change 'Set' to 'Enable'?

[stefan-webrtc, 2015/12/03 10:18:27]

Done.

+  int ret =
+      SetSendRtpHeaderExtension(true, kRtpExtensionTransportSequenceNumber, id);
+  RTC_DCHECK_EQ(0, ret);

[mflodman, 2015/12/03 09:24:21]

Do we expect errors for other extensions?

If not, it would be a nicer to make SetSendRtpHeaderExtension a void-function
and DCHECK there.

[stefan-webrtc, 2015/12/03 10:18:27]

I'd prefer to not touch too much of the existing VoE API since it's supposed to
still be supported. For instance, I don't know if other code relies on voice
engine checking the range of the header extension id (libjingle does, but what
other code is there?). I'll leave it as is for now if you don't mind.

[mflodman, 2015/12/03 10:38:33]

Acknowledged.

[the sun, 2015/12/03 11:10:28]

+1

+}
+
 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(!channel_state_.Get().sending);
+  RTC_DCHECK(packet_router != nullptr || packet_router_ != nullptr);
+  _moduleProcessThreadPtr->DeRegisterModule(_rtpRtcpModule.get());
+  _rtpRtcpModule.reset(CreateRtpRtcp(rtp_packet_sender, packet_router,

[mflodman, 2015/12/03 09:24:21]

Can we check we haven't sent anything before this point?

Just resetting the RTP module might cause SRTP issues.

[stefan-webrtc, 2015/12/03 10:18:27]

We check that the channel_state_ is not sending on line 2818. I don't think
that's enough to be 100% sure this is safe, but maybe it's a good enough check?
Otherwise we'll have to add a lock to check if a packet has been sent or not.

[mflodman, 2015/12/03 10:38:33]

I was thinking of checking number of sent frames, or similar, in the RTP module
we're about to delete, but I didn't see a good counter for audio, only video. 

[the sun, 2015/12/03 11:10:28]

We should be fine; the ambiguity here is transient - once AudioSendStream
creates its own VoE::Channel there will be no way for a client to mess things
up.

+                                     transport_feedback_observer));
+  _moduleProcessThreadPtr->RegisterModule(_rtpRtcpModule.get());
+  _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)
 {