Configure VoE NACK through AudioSendStream::Config, for send streams.

BUG=webrtc:4690

Review-Url: https://codereview.webrtc.org/1955363003
Cr-Commit-Position: refs/heads/master@{#13136}

webrtc/audio/audio_send_stream.cc

@@ -39,6 +39,7 @@ std::string AudioSendStream::Config::Rtp::ToString() const {
     }
   }
   ss << ']';
+  ss << ", nack: " << nack.ToString();
   ss << ", c_name: " << c_name;
   ss << '}';
   return ss.str();
@@ -74,6 +75,10 @@ AudioSendStream::AudioSendStream(
   channel_proxy_->SetRTCPStatus(true);
   channel_proxy_->SetLocalSSRC(config.rtp.ssrc);
   channel_proxy_->SetRTCP_CNAME(config.rtp.c_name);
+  // TODO(solenberg): Config NACK history window (which is a packet count),
+  // using the actual packet size for the configured codec.
+  channel_proxy_->SetNACKStatus(config_.rtp.nack.rtp_history_ms != 0,
+                                config_.rtp.nack.rtp_history_ms / 20);
 
   channel_proxy_->RegisterExternalTransport(config.send_transport);
 

webrtc/audio/audio_send_stream_unittest.cc

@@ -74,6 +74,7 @@ struct ConfigHelper {
           EXPECT_CALL(*channel_proxy_, SetRTCPStatus(true)).Times(1);
           EXPECT_CALL(*channel_proxy_, SetLocalSSRC(kSsrc)).Times(1);
           EXPECT_CALL(*channel_proxy_, SetRTCP_CNAME(StrEq(kCName))).Times(1);
+          EXPECT_CALL(*channel_proxy_, SetNACKStatus(true, 10)).Times(1);
           EXPECT_CALL(*channel_proxy_,
               SetSendAbsoluteSenderTimeStatus(true, kAbsSendTimeId)).Times(1);
           EXPECT_CALL(*channel_proxy_,
@@ -97,6 +98,7 @@ struct ConfigHelper {
         }));
     stream_config_.voe_channel_id = kChannelId;
     stream_config_.rtp.ssrc = kSsrc;
+    stream_config_.rtp.nack.rtp_history_ms = 200;
     stream_config_.rtp.c_name = kCName;
     stream_config_.rtp.extensions.push_back(
         RtpExtension(RtpExtension::kAudioLevelUri, kAudioLevelId));
@@ -178,7 +180,8 @@ TEST(AudioSendStreamTest, ConfigToString) {
   EXPECT_EQ(
       "{rtp: {ssrc: 1234, extensions: [{uri: "
       "http://www.webrtc.org/experiments/rtp-hdrext/abs-send-time, id: 3}], "
-      "c_name: foo_name}, voe_channel_id: 1, cng_payload_type: 42}",
+      "nack: {rtp_history_ms: 0}, c_name: foo_name}, voe_channel_id: 1, "
+      "cng_payload_type: 42}",
       config.ToString());
 }
 

webrtc/audio_send_stream.h

@@ -56,7 +56,7 @@ class AudioSendStream {
 
     std::string ToString() const;
 
-    // Receive-stream specific RTP settings.
+    // Send-stream specific RTP settings.
     struct Rtp {
       std::string ToString() const;
 
@@ -66,6 +66,9 @@ class AudioSendStream {
       // RTP header extensions used for the sent stream.
       std::vector<RtpExtension> extensions;
 
+      // See NackConfig for description.
+      NackConfig nack;
+
       // RTCP CNAME, see RFC 3550.
       std::string c_name;
     } rtp;

webrtc/config.cc

@@ -13,6 +13,13 @@
 #include <string>
 
 namespace webrtc {
+std::string NackConfig::ToString() const {
+  std::stringstream ss;
+  ss << "{rtp_history_ms: " << rtp_history_ms;
+  ss << '}';
+  return ss.str();
+}
+
 std::string FecConfig::ToString() const {
   std::stringstream ss;
   ss << "{ulpfec_payload_type: " << ulpfec_payload_type;

webrtc/config.h

@@ -25,6 +25,7 @@ namespace webrtc {
 // Settings for NACK, see RFC 4585 for details.
 struct NackConfig {
   NackConfig() : rtp_history_ms(0) {}
+  std::string ToString() const;
   // Send side: the time RTP packets are stored for retransmissions.
   // Receive side: the time the receiver is prepared to wait for
   // retransmissions.

webrtc/media/engine/webrtcvoiceengine.cc

@@ -64,6 +64,7 @@ const int kDefaultAudioDeviceId = 0;
 // Parameter used for NACK.
 // This value is equivalent to 5 seconds of audio data at 20 ms per packet.
 const int kNackMaxPackets = 250;
+constexpr int kNackRtpHistoryMs = 5000;
 
 // Codec parameters for Opus.
 // draft-spittka-payload-rtp-opus-03
@@ -461,6 +462,41 @@ const WebRtcVoiceCodecs::CodecPref WebRtcVoiceCodecs::kCodecPrefs[11] = {
 };
 } // namespace {
 
+bool SendCodecSpec::operator==(const SendCodecSpec& rhs) const {
+  if (nack_enabled != rhs.nack_enabled) {
+    return false;
+  }
+  if (transport_cc_enabled != rhs.transport_cc_enabled) {
+    return false;
+  }
+  if (enable_codec_fec != rhs.enable_codec_fec) {
+    return false;
+  }
+  if (enable_opus_dtx != rhs.enable_opus_dtx) {
+    return false;
+  }
+  if (opus_max_playback_rate != rhs.opus_max_playback_rate) {
+    return false;
+  }
+  if (red_payload_type != rhs.red_payload_type) {
+    return false;
+  }
+  if (cng_payload_type != rhs.cng_payload_type) {
+    return false;
+  }
+  if (cng_plfreq != rhs.cng_plfreq) {
+    return false;
+  }
+  if (codec_inst != rhs.codec_inst) {
+    return false;
+  }
+  return true;
+}
+
+bool SendCodecSpec::operator!=(const SendCodecSpec& rhs) const {
+  return !(*this == rhs);
+}
+
 bool WebRtcVoiceEngine::ToCodecInst(const AudioCodec& in,
                                     webrtc::CodecInst* out) {
   return WebRtcVoiceCodecs::ToCodecInst(in, out);
@@ -1048,6 +1084,7 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
                         webrtc::AudioTransport* voe_audio_transport,
                         uint32_t ssrc,
                         const std::string& c_name,
+                        const SendCodecSpec& send_codec_spec,
                         const std::vector<webrtc::RtpExtension>& extensions,
                         webrtc::Call* call,
                         webrtc::Transport* send_transport)
@@ -1063,7 +1100,8 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     config_.rtp.ssrc = ssrc;
     config_.rtp.c_name = c_name;
     config_.voe_channel_id = ch;
-    RecreateAudioSendStream(extensions);
+    config_.rtp.extensions = extensions;
+    RecreateAudioSendStream(send_codec_spec);
   }
 
   ~WebRtcAudioSendStream() override {
@@ -1072,6 +1110,20 @@ class WebRtcVoiceMediaChannel::WebRtcAudioSendStream
     call_->DestroyAudioSendStream(stream_);
   }
 
+  void RecreateAudioSendStream(const SendCodecSpec& send_codec_spec) {
+    RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
+    if (stream_) {
+      call_->DestroyAudioSendStream(stream_);
+      stream_ = nullptr;
+    }
+    config_.rtp.nack.rtp_history_ms =
+        send_codec_spec.nack_enabled ? kNackRtpHistoryMs : 0;
+    RTC_DCHECK(!stream_);
+    stream_ = call_->CreateAudioSendStream(config_);
+    RTC_CHECK(stream_);
+    UpdateSendState();
+  }
+
   void RecreateAudioSendStream(
       const std::vector<webrtc::RtpExtension>& extensions) {
     RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
@@ -1598,8 +1650,8 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
   // with the proper configuration for VAD, CNG, NACK and Opus-specific
   // parameters.
   // TODO(solenberg): Refactor this logic once we create AudioEncoders here.
+  SendCodecSpec send_codec_spec;
   {
-    SendCodecSpec send_codec_spec;
     send_codec_spec.nack_enabled = send_codec_spec_.nack_enabled;
 
     // Find send codec (the first non-telephone-event/CN codec).
@@ -1665,15 +1717,16 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
         break;
       }
     }
-
-    // Latch in the new state.
-    send_codec_spec_ = std::move(send_codec_spec);
   }
 
-  // Cache the codecs in order to configure the channel created later.
-  for (const auto& ch : send_streams_) {
-    if (!SetSendCodecs(ch.second->channel(), ch.second->rtp_parameters())) {
-      return false;
+  // Apply new settings to all streams.
+  if (send_codec_spec_ != send_codec_spec) {
+    send_codec_spec_ = std::move(send_codec_spec);
+    for (const auto& kv : send_streams_) {
+      kv.second->RecreateAudioSendStream(send_codec_spec_);
+      if (!SetSendCodecs(kv.second->channel(), kv.second->rtp_parameters())) {
+        return false;
+      }
     }
   }
 
@@ -1701,13 +1754,10 @@ bool WebRtcVoiceMediaChannel::SetSendCodecs(
 bool WebRtcVoiceMediaChannel::SetSendCodecs(
     int channel,
     const webrtc::RtpParameters& rtp_parameters) {
-  // Disable VAD, NACK and FEC unless we know the other side wants them.
+  // Disable VAD and FEC unless we know the other side wants them.
   engine()->voe()->codec()->SetVADStatus(channel, false);
-  engine()->voe()->rtp()->SetNACKStatus(channel, false, 0);
   engine()->voe()->codec()->SetFECStatus(channel, 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)) {
@@ -1956,8 +2006,8 @@ bool WebRtcVoiceMediaChannel::AddSendStream(const StreamParams& sp) {
       engine()->voe()->base()->audio_transport();
 
   WebRtcAudioSendStream* stream = new WebRtcAudioSendStream(
-      channel, audio_transport, ssrc, sp.cname, send_rtp_extensions_, call_,
-      this);
+      channel, audio_transport, ssrc, sp.cname, send_codec_spec_,
+      send_rtp_extensions_, call_, this);
   send_streams_.insert(std::make_pair(ssrc, stream));
 
   // Set the current codecs to be used for the new channel. We need to do this

webrtc/media/engine/webrtcvoiceengine.h

@@ -38,6 +38,26 @@ class AudioSource;
 class VoEWrapper;
 class WebRtcVoiceMediaChannel;
 
+struct SendCodecSpec {
+  SendCodecSpec() {
+    webrtc::CodecInst empty_inst = {0};
+    codec_inst = empty_inst;
+    codec_inst.pltype = -1;
+  }
+  bool operator==(const SendCodecSpec& rhs) const;
+  bool operator!=(const SendCodecSpec& rhs) const;
+
+  bool nack_enabled = false;
+  bool transport_cc_enabled = false;
+  bool enable_codec_fec = false;
+  bool enable_opus_dtx = false;
+  int opus_max_playback_rate = 0;
+  int red_payload_type = -1;
+  int cng_payload_type = -1;
+  int cng_plfreq = -1;
+  webrtc::CodecInst codec_inst;
+};
+
 // WebRtcVoiceEngine is a class to be used with CompositeMediaEngine.
 // It uses the WebRtc VoiceEngine library for audio handling.
 class WebRtcVoiceEngine final : public webrtc::TraceCallback  {
@@ -282,21 +302,7 @@ class WebRtcVoiceMediaChannel final : public VoiceMediaChannel,
   std::map<uint32_t, WebRtcAudioReceiveStream*> recv_streams_;
   std::vector<webrtc::RtpExtension> recv_rtp_extensions_;
 
-  struct SendCodecSpec {
-    SendCodecSpec() {
-      webrtc::CodecInst empty_inst = {0};
-      codec_inst = empty_inst;
-      codec_inst.pltype = -1;
-    }
-    bool nack_enabled = false;
-    bool transport_cc_enabled = false;
-    bool enable_codec_fec = false;
-    bool enable_opus_dtx = false;
-    int opus_max_playback_rate = 0;
-    int cng_payload_type = -1;
-    int cng_plfreq = -1;
-    webrtc::CodecInst codec_inst;
-  } send_codec_spec_;
+  SendCodecSpec send_codec_spec_;
 
   RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WebRtcVoiceMediaChannel);
 };

webrtc/media/engine/webrtcvoiceengine_unittest.cc

@@ -50,6 +50,8 @@ const uint32_t kSsrc2 = 2;
 const uint32_t kSsrc3 = 3;
 const uint32_t kSsrcs4[] = { 1, 2, 3, 4 };
 
+constexpr int kRtpHistoryMs = 5000;
+
 class FakeVoEWrapper : public cricket::VoEWrapper {
  public:
   explicit FakeVoEWrapper(cricket::FakeWebRtcVoiceEngine* engine)
@@ -1358,15 +1360,14 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecOpusMaxAverageBitrate) {
 // Test that we can enable NACK with opus as caller.
 TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackAsCaller) {
   EXPECT_TRUE(SetupSendStream());
-  int channel_num = voe_.GetLastChannel();
   cricket::AudioSendParameters parameters;
   parameters.codecs.push_back(kOpusCodec);
   parameters.codecs[0].AddFeedbackParam(
       cricket::FeedbackParam(cricket::kRtcpFbParamNack,
                              cricket::kParamValueEmpty));
-  EXPECT_FALSE(voe_.GetNACK(channel_num));
+  EXPECT_EQ(0, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_TRUE(voe_.GetNACK(channel_num));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
 }
 
 // Test that we can enable NACK with opus as callee.
@@ -1385,13 +1386,12 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackAsCallee) {
 
   EXPECT_TRUE(channel_->AddSendStream(
       cricket::StreamParams::CreateLegacy(kSsrc1)));
-  EXPECT_TRUE(voe_.GetNACK(voe_.GetLastChannel()));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
 }
 
 // Test that we can enable NACK on receive streams.
 TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackRecvStreams) {
   EXPECT_TRUE(SetupSendStream());
-  int channel_num1 = voe_.GetLastChannel();
   EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(2)));
   int channel_num2 = voe_.GetLastChannel();
   cricket::AudioSendParameters parameters;
@@ -1399,35 +1399,33 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecEnableNackRecvStreams) {
   parameters.codecs[0].AddFeedbackParam(
       cricket::FeedbackParam(cricket::kRtcpFbParamNack,
                              cricket::kParamValueEmpty));
-  EXPECT_FALSE(voe_.GetNACK(channel_num1));
+  EXPECT_EQ(0, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
   EXPECT_FALSE(voe_.GetNACK(channel_num2));
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_TRUE(voe_.GetNACK(channel_num1));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
   EXPECT_TRUE(voe_.GetNACK(channel_num2));
 }
 
 // Test that we can disable NACK.
 TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecDisableNack) {
   EXPECT_TRUE(SetupSendStream());
-  int channel_num = voe_.GetLastChannel();
   cricket::AudioSendParameters parameters;
   parameters.codecs.push_back(kOpusCodec);
   parameters.codecs[0].AddFeedbackParam(
       cricket::FeedbackParam(cricket::kRtcpFbParamNack,
                              cricket::kParamValueEmpty));
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_TRUE(voe_.GetNACK(channel_num));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
 
   parameters.codecs.clear();
   parameters.codecs.push_back(kOpusCodec);
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_FALSE(voe_.GetNACK(channel_num));
+  EXPECT_EQ(0, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
 }
 
 // Test that we can disable NACK on receive streams.
 TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecDisableNackRecvStreams) {
   EXPECT_TRUE(SetupSendStream());
-  int channel_num1 = voe_.GetLastChannel();
   EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(2)));
   int channel_num2 = voe_.GetLastChannel();
   cricket::AudioSendParameters parameters;
@@ -1436,20 +1434,19 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendCodecDisableNackRecvStreams) {
       cricket::FeedbackParam(cricket::kRtcpFbParamNack,
                              cricket::kParamValueEmpty));
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_TRUE(voe_.GetNACK(channel_num1));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
   EXPECT_TRUE(voe_.GetNACK(channel_num2));
 
   parameters.codecs.clear();
   parameters.codecs.push_back(kOpusCodec);
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_FALSE(voe_.GetNACK(channel_num1));
+  EXPECT_EQ(0, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
   EXPECT_FALSE(voe_.GetNACK(channel_num2));
 }
 
 // Test that NACK is enabled on a new receive stream.
 TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamEnableNack) {
   EXPECT_TRUE(SetupSendStream());
-  int channel_num = voe_.GetLastChannel();
   cricket::AudioSendParameters parameters;
   parameters.codecs.push_back(kIsacCodec);
   parameters.codecs.push_back(kCn16000Codec);
@@ -1457,10 +1454,10 @@ TEST_F(WebRtcVoiceEngineTestFake, AddRecvStreamEnableNack) {
       cricket::FeedbackParam(cricket::kRtcpFbParamNack,
                              cricket::kParamValueEmpty));
   EXPECT_TRUE(channel_->SetSendParameters(parameters));
-  EXPECT_TRUE(voe_.GetNACK(channel_num));
+  EXPECT_EQ(kRtpHistoryMs, GetSendStreamConfig(kSsrc1).rtp.nack.rtp_history_ms);
 
   EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(2)));
-  channel_num = voe_.GetLastChannel();
+  int channel_num = voe_.GetLastChannel();
   EXPECT_TRUE(voe_.GetNACK(channel_num));
   EXPECT_TRUE(channel_->AddRecvStream(cricket::StreamParams::CreateLegacy(3)));
   channel_num = voe_.GetLastChannel();
@@ -2276,15 +2273,16 @@ TEST_F(WebRtcVoiceEngineTestFake, GetStatsWithMultipleSendStreams) {
     EXPECT_TRUE(channel_->AddSendStream(
         cricket::StreamParams::CreateLegacy(ssrc)));
   }
-  SetAudioSendStreamStats();
 
   // Create a receive stream to check that none of the send streams end up in
   // the receive stream stats.
   EXPECT_TRUE(channel_->AddRecvStream(
       cricket::StreamParams::CreateLegacy(kSsrc2)));
+
   // We need send codec to be set to get all stats.
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
   EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_));
+  SetAudioSendStreamStats();
 
   // Check stats for the added streams.
   {
@@ -2442,13 +2440,13 @@ TEST_F(WebRtcVoiceEngineTestFake, SetSendSsrc) {
 TEST_F(WebRtcVoiceEngineTestFake, GetStats) {
   // Setup. We need send codec to be set to get all stats.
   EXPECT_TRUE(SetupSendStream());
-  SetAudioSendStreamStats();
   // SetupSendStream adds a send stream with kSsrc1, so the receive
   // stream has to use a different SSRC.
   EXPECT_TRUE(channel_->AddRecvStream(
       cricket::StreamParams::CreateLegacy(kSsrc2)));
   EXPECT_TRUE(channel_->SetSendParameters(send_parameters_));
   EXPECT_TRUE(channel_->SetRecvParameters(recv_parameters_));
+  SetAudioSendStreamStats();
 
   // Check stats for the added streams.
   {

webrtc/test/mock_voe_channel_proxy.h

@@ -23,6 +23,7 @@ class MockVoEChannelProxy : public voe::ChannelProxy {
   MOCK_METHOD1(SetRTCPStatus, void(bool enable));
   MOCK_METHOD1(SetLocalSSRC, void(uint32_t ssrc));
   MOCK_METHOD1(SetRTCP_CNAME, void(const std::string& c_name));
+  MOCK_METHOD2(SetNACKStatus, void(bool enable, int max_packets));
   MOCK_METHOD2(SetSendAbsoluteSenderTimeStatus, void(bool enable, int id));
   MOCK_METHOD2(SetSendAudioLevelIndicationStatus, void(bool enable, int id));
   MOCK_METHOD2(SetReceiveAbsoluteSenderTimeStatus, void(bool enable, int id));

webrtc/voice_engine/channel_proxy.cc

@@ -45,6 +45,11 @@ void ChannelProxy::SetRTCP_CNAME(const std::string& c_name) {
   RTC_DCHECK_EQ(0, error);
 }
 
+void ChannelProxy::SetNACKStatus(bool enable, int max_packets) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  channel()->SetNACKStatus(enable, max_packets);
+}
+
 void ChannelProxy::SetSendAbsoluteSenderTimeStatus(bool enable, int id) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   int error = channel()->SetSendAbsoluteSenderTimeStatus(enable, id);

webrtc/voice_engine/channel_proxy.h

@@ -48,6 +48,7 @@ class ChannelProxy {
   virtual void SetRTCPStatus(bool enable);
   virtual void SetLocalSSRC(uint32_t ssrc);
   virtual void SetRTCP_CNAME(const std::string& c_name);
+  virtual void SetNACKStatus(bool enable, int max_packets);
   virtual void SetSendAbsoluteSenderTimeStatus(bool enable, int id);
   virtual void SetSendAudioLevelIndicationStatus(bool enable, int id);
   virtual void SetReceiveAbsoluteSenderTimeStatus(bool enable, int id);