WebRtcVoiceEngine refactoring - moving codec configuration code into Send/RecvStreams

webrtc/media/engine/webrtcvoiceengine.cc

Index: webrtc/media/engine/webrtcvoiceengine.cc
diff --git a/webrtc/media/engine/webrtcvoiceengine.cc b/webrtc/media/engine/webrtcvoiceengine.cc
index 6fe3d86fd98f086fe0622711c7769573d5276a1b..b00269f5f855cea0915e3739e09b0dec77b68d6d 100644
--- a/webrtc/media/engine/webrtcvoiceengine.cc
+++ b/webrtc/media/engine/webrtcvoiceengine.cc
@@ -1140,11 +1140,15 @@ int WebRtcVoiceEngine::CreateVoEChannel() {
 class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     : public AudioSource::Sink {
  public:
-  WebRtcAudioSendStream(int ch, webrtc::AudioTransport* voe_audio_transport,
-                        uint32_t ssrc, const std::string& c_name,
+  WebRtcAudioSendStream(int ch,
+                        WebRtcVoiceEngine* engine,

[the sun, 2016/04/01 08:29:06]

WebRtcAudioReceive/SendStream should be thin proxy classes that help WVoMC to
recreate AudioReceive/SendStream if any of the config settings change.

I'd like to break them out into separate source files, once they have matured.
So, the intention is that WebRtcAudioReceive/SendStream should NOT know about
the engine, and I am not convinced it makes sense for them to take on the
responsibilities of WVoMC in the way proposed here.

Going forward, the streams will be configured with live encoder/decoder objects,
and as I see it, the responsibility of creating those will still fall on WVoMC
(although the calls referencing the voe_channel_id will be gone).

+                        webrtc::AudioTransport* voe_audio_transport,
+                        uint32_t ssrc,
+                        const std::string& c_name,
                         const std::vector<webrtc::RtpExtension>& extensions,
                         webrtc::Call* call)
-      : voe_audio_transport_(voe_audio_transport),
+      : engine_(engine),
+        voe_audio_transport_(voe_audio_transport),
         call_(call),
         config_(nullptr) {
     RTC_DCHECK_GE(ch, 0);
@@ -1257,7 +1261,14 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     return config_.voe_channel_id;
   }
 
+  bool SetSendCodecs(const WebRtcVoiceMediaChannel::SendCodecSpec& spec);
+  bool SetSendCodec(const webrtc::CodecInst& send_codec);
+  void SetNack(bool nack_enabled);
+
  private:
+  WebRtcVoiceEngine* engine() { return engine_; }
+  int GetLastEngineError() { return engine()->GetLastEngineError(); }
+
   void UpdateSendState() {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
     RTC_DCHECK(stream_);
@@ -1270,6 +1281,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
 
   rtc::ThreadChecker worker_thread_checker_;
   rtc::ThreadChecker audio_capture_thread_checker_;
+  WebRtcVoiceEngine* engine_ = nullptr;
   webrtc::AudioTransport* const voe_audio_transport_ = nullptr;
   webrtc::Call* call_ = nullptr;
   webrtc::AudioSendStream::Config config_;
@@ -1283,19 +1295,21 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
   AudioSource* source_ = nullptr;
   bool send_ = false;
 
+
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WebRtcAudioSendStream);
 };
 
 class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
  public:
   WebRtcAudioReceiveStream(int ch,
+                           WebRtcVoiceEngine* engine,
                            uint32_t remote_ssrc,
                            uint32_t local_ssrc,
                            bool use_transport_cc,
                            const std::string& sync_group,
                            const std::vector<webrtc::RtpExtension>& extensions,
                            webrtc::Call* call)
-      : call_(call), config_() {
+      : engine_(engine), call_(call), config_() {
     RTC_DCHECK_GE(ch, 0);
     RTC_DCHECK(call);
     config_.rtp.remote_ssrc = remote_ssrc;
@@ -1336,7 +1350,10 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
     stream_->SetSink(std::move(sink));
   }
 
+  void SetNack(bool nack_enabled);
+
  private:
+  WebRtcVoiceEngine* engine() { return engine_; }
   void RecreateAudioReceiveStream(
       bool use_transport_cc,
       const std::vector<webrtc::RtpExtension>& extensions) {
@@ -1353,6 +1370,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream {
   }
 
   rtc::ThreadChecker worker_thread_checker_;
+  WebRtcVoiceEngine* engine_ = nullptr;
   webrtc::Call* call_ = nullptr;
   webrtc::AudioReceiveStream::Config config_;
   // The stream is owned by WebRtcAudioReceiveStream and may be reallocated if
@@ -1646,15 +1664,19 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
 
   // Cache the codecs in order to configure the channel created later.
   for (const auto& ch : send_streams_) {
-    if (!SetSendCodecs(ch.second->channel())) {
+    if (!ch.second->SetSendCodecs(send_codec_spec_)) {
       return false;
     }
   }
 
+  if (send_bitrate_setting_) {
+    SetSendBitrateInternal(send_bitrate_bps_);
+  }
+
   // Set nack status on receive channels.
   if (!send_streams_.empty()) {
     for (const auto& kv : recv_streams_) {
-      SetNack(kv.second->channel(), send_codec_spec_.nack_enabled);
+      kv.second->SetNack(send_codec_spec_.nack_enabled);
     }
   }
 
@@ -1672,156 +1694,6 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
   return true;
 }
 
-// Apply current codec settings to a single voe::Channel used for sending.
-bool WebRtcVoiceMediaChannel::SetSendCodecs(int channel) {
-  // Disable VAD, FEC, and RED unless we know the other side wants them.
-  engine()->voe()->codec()->SetVADStatus(channel, false);
-  engine()->voe()->rtp()->SetNACKStatus(channel, false, 0);
-  engine()->voe()->rtp()->SetREDStatus(channel, false);
-  engine()->voe()->codec()->SetFECStatus(channel, false);
-
-  if (send_codec_spec_.red_payload_type != -1) {
-    // Enable redundant encoding of the specified codec. Treat any
-    // failure as a fatal internal error.
-    LOG(LS_INFO) << "Enabling RED on channel " << channel;
-    if (engine()->voe()->rtp()->SetREDStatus(channel, true,
-        send_codec_spec_.red_payload_type) == -1) {
-      LOG_RTCERR3(SetREDStatus, channel, true,
-                  send_codec_spec_.red_payload_type);
-      return false;
-    }
-  }
-
-  SetNack(channel, send_codec_spec_.nack_enabled);
-
-  // Set the codec immediately, since SetVADStatus() depends on whether
-  // the current codec is mono or stereo.
-  if (!SetSendCodec(channel, send_codec_spec_.codec_inst)) {
-    return false;
-  }
-
-  // FEC should be enabled after SetSendCodec.
-  if (send_codec_spec_.enable_codec_fec) {
-    LOG(LS_INFO) << "Attempt to enable codec internal FEC on channel "
-                 << channel;
-    if (engine()->voe()->codec()->SetFECStatus(channel, true) == -1) {
-      // Enable codec internal FEC. Treat any failure as fatal internal error.
-      LOG_RTCERR2(SetFECStatus, channel, true);
-      return false;
-    }
-  }
-
-  if (IsCodec(send_codec_spec_.codec_inst, kOpusCodecName)) {
-    // DTX and maxplaybackrate should be set after SetSendCodec. Because current
-    // send codec has to be Opus.
-
-    // Set Opus internal DTX.
-    LOG(LS_INFO) << "Attempt to "
-                 << (send_codec_spec_.enable_opus_dtx ? "enable" : "disable")
-                 << " Opus DTX on channel "
-                 << channel;
-    if (engine()->voe()->codec()->SetOpusDtx(channel,
-        send_codec_spec_.enable_opus_dtx)) {
-      LOG_RTCERR2(SetOpusDtx, channel, send_codec_spec_.enable_opus_dtx);
-      return false;
-    }
-
-    // If opus_max_playback_rate <= 0, the default maximum playback rate
-    // (48 kHz) will be used.
-    if (send_codec_spec_.opus_max_playback_rate > 0) {
-      LOG(LS_INFO) << "Attempt to set maximum playback rate to "
-                   << send_codec_spec_.opus_max_playback_rate
-                   << " Hz on channel "
-                   << channel;
-      if (engine()->voe()->codec()->SetOpusMaxPlaybackRate(
-          channel, send_codec_spec_.opus_max_playback_rate) == -1) {
-        LOG_RTCERR2(SetOpusMaxPlaybackRate, channel,
-                    send_codec_spec_.opus_max_playback_rate);
-        return false;
-      }
-    }
-  }
-
-  if (send_bitrate_setting_) {
-    SetSendBitrateInternal(send_bitrate_bps_);
-  }
-
-  // Set the CN payloadtype and the VAD status.
-  if (send_codec_spec_.cng_payload_type != -1) {
-    // The CN payload type for 8000 Hz clockrate is fixed at 13.
-    if (send_codec_spec_.cng_plfreq != 8000) {
-      webrtc::PayloadFrequencies cn_freq;
-      switch (send_codec_spec_.cng_plfreq) {
-        case 16000:
-          cn_freq = webrtc::kFreq16000Hz;
-          break;
-        case 32000:
-          cn_freq = webrtc::kFreq32000Hz;
-          break;
-        default:
-          RTC_NOTREACHED();
-          return false;
-      }
-      if (engine()->voe()->codec()->SetSendCNPayloadType(
-              channel, send_codec_spec_.cng_payload_type, cn_freq) == -1) {
-        LOG_RTCERR3(SetSendCNPayloadType, channel,
-                    send_codec_spec_.cng_payload_type, cn_freq);
-        // TODO(ajm): This failure condition will be removed from VoE.
-        // Restore the return here when we update to a new enough webrtc.
-        //
-        // Not returning false because the SetSendCNPayloadType will fail if
-        // the channel is already sending.
-        // This can happen if the remote description is applied twice, for
-        // example in the case of ROAP on top of JSEP, where both side will
-        // send the offer.
-      }
-    }
-
-    // Only turn on VAD if we have a CN payload type that matches the
-    // clockrate for the codec we are going to use.
-    if (send_codec_spec_.cng_plfreq == send_codec_spec_.codec_inst.plfreq &&
-        send_codec_spec_.codec_inst.channels == 1) {
-      // TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the
-      // interaction between VAD and Opus FEC.
-      LOG(LS_INFO) << "Enabling VAD";
-      if (engine()->voe()->codec()->SetVADStatus(channel, true) == -1) {
-        LOG_RTCERR2(SetVADStatus, channel, true);
-        return false;
-      }
-    }
-  }
-  return true;
-}
-
-void WebRtcVoiceMediaChannel::SetNack(int channel, bool nack_enabled) {
-  if (nack_enabled) {
-    LOG(LS_INFO) << "Enabling NACK for channel " << channel;
-    engine()->voe()->rtp()->SetNACKStatus(channel, true, kNackMaxPackets);
-  } else {
-    LOG(LS_INFO) << "Disabling NACK for channel " << channel;
-    engine()->voe()->rtp()->SetNACKStatus(channel, false, 0);
-  }
-}
-
-bool WebRtcVoiceMediaChannel::SetSendCodec(
-    int channel, const webrtc::CodecInst& send_codec) {
-  LOG(LS_INFO) << "Send channel " << channel <<  " selected voice codec "
-               << ToString(send_codec) << ", bitrate=" << send_codec.rate;
-
-  webrtc::CodecInst current_codec = {0};
-  if (engine()->voe()->codec()->GetSendCodec(channel, current_codec) == 0 &&
-      (send_codec == current_codec)) {
-    // Codec is already configured, we can return without setting it again.
-    return true;
-  }
-
-  if (engine()->voe()->codec()->SetSendCodec(channel, send_codec) == -1) {
-    LOG_RTCERR2(SetSendCodec, channel, ToString(send_codec));
-    return false;
-  }
-  return true;
-}
-
 bool WebRtcVoiceMediaChannel::SetPlayout(bool playout) {
   desired_playout_ = playout;
   return ChangePlayout(desired_playout_);
@@ -1939,17 +1811,23 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
   // delete the channel in case failure happens below.
   webrtc::AudioTransport* audio_transport =
       engine()->voe()->base()->audio_transport();
-  send_streams_.insert(std::make_pair(ssrc, new WebRtcAudioSendStream(
-      channel, audio_transport, ssrc, sp.cname, send_rtp_extensions_, call_)));
+  WebRtcAudioSendStream* send_stream =
+      new WebRtcAudioSendStream(channel, engine(), audio_transport, ssrc,
+                                sp.cname, send_rtp_extensions_, call_);
+  send_streams_.insert(std::make_pair(ssrc, send_stream));
 
   // Set the current codecs to be used for the new channel. We need to do this
   // after adding the channel to send_channels_, because of how max bitrate is
   // currently being configured by SetSendCodec().
-  if (HasSendCodec() && !SetSendCodecs(channel)) {
+  if (HasSendCodec() && !send_stream->SetSendCodecs(send_codec_spec_)) {
     RemoveSendStream(ssrc);
     return false;
   }
 
+  if (send_bitrate_setting_) {
+    SetSendBitrateInternal(send_bitrate_bps_);
+  }
+
   // At this point the channel's local SSRC has been updated. If the channel is
   // the first send channel make sure that all the receive channels are updated
   // with the same SSRC in order to send receiver reports.
@@ -2066,13 +1944,12 @@ bool WebRtcVoiceMediaChannel::AddRecvStream(const StreamParams& sp) {
                  << " is associated with channel #" << send_channel << ".";
   }
 
-  recv_streams_.insert(std::make_pair(
-      ssrc, new WebRtcAudioReceiveStream(channel, ssrc, receiver_reports_ssrc_,
-                                         recv_transport_cc_enabled_,
-                                         sp.sync_label, recv_rtp_extensions_,
-                                         call_)));
+  WebRtcAudioReceiveStream* recv_stream = new WebRtcAudioReceiveStream(
+      channel, engine(), ssrc, receiver_reports_ssrc_,
+      recv_transport_cc_enabled_, sp.sync_label, recv_rtp_extensions_, call_);
+  recv_streams_.insert(std::make_pair(ssrc, recv_stream));
 
-  SetNack(channel, send_codec_spec_.nack_enabled);
+  recv_stream->SetNack(send_codec_spec_.nack_enabled);
   SetPlayout(channel, playout_);
 
   return true;
@@ -2401,7 +2278,7 @@ bool WebRtcVoiceMediaChannel::SetSendBitrateInternal(int bps) {
     // If codec is multi-rate then just set the bitrate.
     codec.rate = bps;
     for (const auto& ch : send_streams_) {
-      if (!SetSendCodec(ch.second->channel(), codec)) {
+      if (!ch.second->SetSendCodec(codec)) {
         LOG(LS_INFO) << "Failed to set codec " << codec.plname
                      << " to bitrate " << bps << " bps.";
         return false;
@@ -2555,6 +2432,164 @@ bool WebRtcVoiceMediaChannel::SetPlayout(int channel, bool playout) {
   }
   return true;
 }
+
+// Apply current codec settings to a single voe::Channel used for sending.
+bool WebRtcVoiceMediaChannel::WebRtcAudioSendStream::SetSendCodecs(
+    const WebRtcVoiceMediaChannel::SendCodecSpec& send_codec_spec) {
+  // Disable VAD, FEC, and RED unless we know the other side wants them.
+  engine()->voe()->codec()->SetVADStatus(channel(), false);
+  engine()->voe()->rtp()->SetNACKStatus(channel(), false, 0);
+  engine()->voe()->rtp()->SetREDStatus(channel(), false);
+  engine()->voe()->codec()->SetFECStatus(channel(), false);
+
+  if (send_codec_spec.red_payload_type != -1) {
+    // Enable redundant encoding of the specified codec. Treat any
+    // failure as a fatal internal error.
+    LOG(LS_INFO) << "Enabling RED on channel " << channel();
+    if (engine()->voe()->rtp()->SetREDStatus(
+            channel(), true, send_codec_spec.red_payload_type) == -1) {
+      LOG_RTCERR3(SetREDStatus, channel(), true,
+                  send_codec_spec.red_payload_type);
+      return false;
+    }
+  }
+
+  SetNack(send_codec_spec.nack_enabled);
+
+  // Set the codec immediately, since SetVADStatus() depends on whether
+  // the current codec is mono or stereo.
+  if (!SetSendCodec(send_codec_spec.codec_inst)) {
+    return false;
+  }
+
+  // FEC should be enabled after SetSendCodec.
+  if (send_codec_spec.enable_codec_fec) {
+    LOG(LS_INFO) << "Attempt to enable codec internal FEC on channel "
+                 << channel();
+    if (engine()->voe()->codec()->SetFECStatus(channel(), true) == -1) {
+      // Enable codec internal FEC. Treat any failure as fatal internal error.
+      LOG_RTCERR2(SetFECStatus, channel(), true);
+      return false;
+    }
+  }
+
+  if (IsCodec(send_codec_spec.codec_inst, kOpusCodecName)) {
+    // DTX and maxplaybackrate should be set after SetSendCodec. Because current
+    // send codec has to be Opus.
+
+    // Set Opus internal DTX.
+    LOG(LS_INFO) << "Attempt to "
+                 << (send_codec_spec.enable_opus_dtx ? "enable" : "disable")
+                 << " Opus DTX on channel " << channel();
+    if (engine()->voe()->codec()->SetOpusDtx(
+            channel(), send_codec_spec.enable_opus_dtx)) {
+      LOG_RTCERR2(SetOpusDtx, channel(), send_codec_spec.enable_opus_dtx);
+      return false;
+    }
+
+    // If opus_max_playback_rate <= 0, the default maximum playback rate
+    // (48 kHz) will be used.
+    if (send_codec_spec.opus_max_playback_rate > 0) {
+      LOG(LS_INFO) << "Attempt to set maximum playback rate to "
+                   << send_codec_spec.opus_max_playback_rate
+                   << " Hz on channel " << channel();
+      if (engine()->voe()->codec()->SetOpusMaxPlaybackRate(
+              channel(), send_codec_spec.opus_max_playback_rate) == -1) {
+        LOG_RTCERR2(SetOpusMaxPlaybackRate, channel(),
+                    send_codec_spec.opus_max_playback_rate);
+        return false;
+      }
+    }
+  }
+
+  // Set the CN payloadtype and the VAD status.
+  if (send_codec_spec.cng_payload_type != -1) {
+    // The CN payload type for 8000 Hz clockrate is fixed at 13.
+    if (send_codec_spec.cng_plfreq != 8000) {
+      webrtc::PayloadFrequencies cn_freq;
+      switch (send_codec_spec.cng_plfreq) {
+        case 16000:
+          cn_freq = webrtc::kFreq16000Hz;
+          break;
+        case 32000:
+          cn_freq = webrtc::kFreq32000Hz;
+          break;
+        default:
+          RTC_NOTREACHED();
+          return false;
+      }
+      if (engine()->voe()->codec()->SetSendCNPayloadType(
+              channel(), send_codec_spec.cng_payload_type, cn_freq) == -1) {
+        LOG_RTCERR3(SetSendCNPayloadType, channel(),
+                    send_codec_spec.cng_payload_type, cn_freq);
+        // TODO(ajm): This failure condition will be removed from VoE.
+        // Restore the return here when we update to a new enough webrtc.
+        //
+        // Not returning false because the SetSendCNPayloadType will fail if
+        // the channel is already sending.
+        // This can happen if the remote description is applied twice, for
+        // example in the case of ROAP on top of JSEP, where both side will
+        // send the offer.
+      }
+    }
+
+    // Only turn on VAD if we have a CN payload type that matches the
+    // clockrate for the codec we are going to use.
+    if (send_codec_spec.cng_plfreq == send_codec_spec.codec_inst.plfreq &&
+        send_codec_spec.codec_inst.channels == 1) {
+      // TODO(minyue): If CN frequency == 48000 Hz is allowed, consider the
+      // interaction between VAD and Opus FEC.
+      LOG(LS_INFO) << "Enabling VAD";
+      if (engine()->voe()->codec()->SetVADStatus(channel(), true) == -1) {
+        LOG_RTCERR2(SetVADStatus, channel(), true);
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
+bool WebRtcVoiceMediaChannel::WebRtcAudioSendStream::SetSendCodec(
+    const webrtc::CodecInst& send_codec) {
+  LOG(LS_INFO) << "Send channel " << channel() << " selected voice codec "
+               << ToString(send_codec) << ", bitrate=" << send_codec.rate;
+
+  webrtc::CodecInst current_codec = {0};
+  if (engine()->voe()->codec()->GetSendCodec(channel(), current_codec) == 0 &&
+      (send_codec == current_codec)) {
+    // Codec is already configured, we can return without setting it again.
+    return true;
+  }
+
+  if (engine()->voe()->codec()->SetSendCodec(channel(), send_codec) == -1) {
+    LOG_RTCERR2(SetSendCodec, channel(), ToString(send_codec));
+    return false;
+  }
+  return true;
+}
+
+void WebRtcVoiceMediaChannel::WebRtcAudioSendStream::SetNack(
+    bool nack_enabled) {
+  if (nack_enabled) {
+    LOG(LS_INFO) << "Enabling NACK for channel " << channel();
+    engine()->voe()->rtp()->SetNACKStatus(channel(), true, kNackMaxPackets);
+  } else {
+    LOG(LS_INFO) << "Disabling NACK for channel " << channel();
+    engine()->voe()->rtp()->SetNACKStatus(channel(), false, 0);
+  }
+}
+
+void WebRtcVoiceMediaChannel::WebRtcAudioReceiveStream::SetNack(
+    bool nack_enabled) {
+  if (nack_enabled) {
+    LOG(LS_INFO) << "Enabling NACK for channel " << channel();
+    engine()->voe()->rtp()->SetNACKStatus(channel(), true, kNackMaxPackets);
+  } else {
+    LOG(LS_INFO) << "Disabling NACK for channel " << channel();
+    engine()->voe()->rtp()->SetNACKStatus(channel(), false, 0);
+  }
+}
+
 }  // namespace cricket
 
 #endif  // HAVE_WEBRTC_VOICE