Injectable audio encoders: WebRtcVoiceEngine and company

webrtc/voice_engine/channel.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 1086 matching lines @@
 void Channel::SetSink(std::unique_ptr<AudioSinkInterface> sink) {
   rtc::CritScope cs(&_callbackCritSect);
   audio_sink_ = std::move(sink);
 }
 
 const rtc::scoped_refptr<AudioDecoderFactory>&
 Channel::GetAudioDecoderFactory() const {
   return decoder_factory_;
 }
 
+AudioCodingModule& Channel::GetAudioCodingModule() {
+  return *audio_coding_;
+}
+
 int32_t Channel::StartPlayout() {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::StartPlayout()");
   if (channel_state_.Get().playing) {
     return 0;
   }
 
   // Add participant as candidates for mixing.
   if (_outputMixerPtr->SetMixabilityStatus(*this, true) != 0) {
     _engineStatisticsPtr->SetLastError(
@@ skipping 78 matching lines @@
   if (_rtpRtcpModule->SetSendingStatus(false) == -1) {
     _engineStatisticsPtr->SetLastError(
         VE_RTP_RTCP_MODULE_ERROR, kTraceWarning,
         "StartSend() RTP/RTCP failed to stop sending");
   }
   _rtpRtcpModule->SetSendingMediaStatus(false);
 
   return 0;
 }
 
+bool Channel::SetSendFormat(int payload_type,
+                            const SdpAudioFormat& format,
+                            AudioEncoderFactory* factory) {
+  WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
+               "Channel::SetSendFormat()");
+
+  auto encoder = factory->MakeAudioEncoder(payload_type, format);

[ossu, 2017/02/22 10:24:23]

I put the MakeAudioEncoder call here, rather than webrtcvoiceengine or
audio_send_stream, because we need to call RegisterSendPayload with stuff in
format, so we'll need to pass format along anyway. This could be changed, i.e.
by being able to ask the encoder about its format, or maybe RegisterSendPayload
could be redesigned?

[kwiberg-webrtc, 2017/02/22 10:42:06]

We want RegisterSendPayload to not take a CodecInst, at any rate. If we can
figure out what data it should actually take, it'll hopefully become clearer
whether AudioEncoder instances should reasonably carry it.

+  if (!encoder) {
+    WEBRTC_TRACE(kTraceError, kTraceVoice, VoEId(_instanceId, _channelId),
+                 "SetSendFormat() failed to create encoder");
+    return false;
+  }
+
+  // TODO(ossu): Make a CodecInst up for now.
+  CodecInst lies;
+  lies.pltype = payload_type;
+  strncpy(lies.plname, format.name.c_str(), RTP_PAYLOAD_NAME_SIZE);

[kwiberg-webrtc, 2017/02/21 23:35:04]

The length limiter should also involve sizeof(lies.plname).

[ossu, 2017/02/22 10:24:23]

But sizeof(lies.plname) _is_ RTP_PAYLOAD_NAME_SIZE.

[kwiberg-webrtc, 2017/02/22 10:42:06]

You know that, and I know that (obviously!), but I have a friend who didn't know
this for certain and would have had to look it up, but was too lazy.

So I'd still suggest you use sizeof(lies.plname). If you want to use
RTP_PAYLOAD_NAME_SIZE, please static_assert that they're the same.

[ossu, 2017/02/22 10:47:07]

You're right. I'll use the sizeof instead. I think I just copied this phrasing
from somewhere else.

+  lies.plname[RTP_PAYLOAD_NAME_SIZE - 1] = 0;
+  // Seems unclear if it should be clock rate or sample rate. CodecInst used to
+  // carry sample rate, but only clock rate seems sensible to send to the

[kwiberg-webrtc, 2017/02/21 23:35:04]

"CodecInst supposedly carries the sample rate,"

[ossu, 2017/02/22 10:24:23]

Alright. I think this comment is more for the review, rather than being left in.
But maybe nobody really knows which one should be in there? It SeemsToWork(tm)
regardless of the value I choose. If that's the case, then the comment should
remain and I'll rephrase it.

[kwiberg-webrtc, 2017/02/22 10:42:06]

Acknowledged.

+  // RTP/RTCP module.
+  // lies.plfreq = encoder->SampleRateHz();
+  lies.plfreq = format.clockrate_hz;
+  lies.pacsize = 0;
+  lies.channels = encoder->NumChannels();
+  lies.rate = 0;
+
+  if (_rtpRtcpModule->RegisterSendPayload(lies) != 0) {
+    _rtpRtcpModule->DeRegisterSendPayload(payload_type);
+    if (_rtpRtcpModule->RegisterSendPayload(lies) != 0) {
+      WEBRTC_TRACE(
+          kTraceError, kTraceVoice, VoEId(_instanceId, _channelId),
+          "SetSendFormat() failed to register codec to RTP/RTCP module");
+      return false;
+    }
+  }
+
+  audio_coding_->SetEncoder(std::move(encoder));
+
+  return true;
+}
+
 int32_t Channel::RegisterVoiceEngineObserver(VoiceEngineObserver& observer) {
   WEBRTC_TRACE(kTraceInfo, kTraceVoice, VoEId(_instanceId, _channelId),
                "Channel::RegisterVoiceEngineObserver()");
   rtc::CritScope cs(&_callbackCritSect);
 
   if (_voiceEngineObserverPtr) {
     _engineStatisticsPtr->SetLastError(
         VE_INVALID_OPERATION, kTraceError,
         "RegisterVoiceEngineObserver() observer already enabled");
     return -1;
@@ skipping 1796 matching lines @@
   int64_t min_rtt = 0;
   if (_rtpRtcpModule->RTT(remoteSSRC, &rtt, &avg_rtt, &min_rtt, &max_rtt) !=
       0) {
     return 0;
   }
   return rtt;
 }
 
 }  // namespace voe
 }  // namespace webrtc