RTCP: implement reduced size RTCP for audio

reduced-size RTCP, i.e. not prefixing RTCP packets with either a sender report or receiver report has been implemented for a long time but only for video.

This CL adds it for audio as well. This reduces the size of audio NACKs (16 bytes, typically one NACK per packet) sent by not prefixing it with a receiver report (32 bytes).
Other packets are not affected as e.g. transport-cc feedback does not add a RR even though that is technically required.

The effect on NACK can be tested by running Chromium with
  --disable-webrtc-encryption --force-fieldtrials=WebRTC-FakeNetworkReceiveConfig/loss_percent:5/
against this fiddle negotiating audio nack:
https://jsfiddle.net/fippo/8ubtLnfx/1/

BUG=webrtc:340041654

Change-Id: I06fb94742ff1b6f9a464c404bfc53913f23498d8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/350269
Commit-Queue: Philipp Hancke <phancke@meta.com>
Reviewed-by: Jakob Ivarsson‎ <jakobi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#42330}

audio/audio_receive_stream.cc

@@ -39,6 +39,10 @@ std::string AudioReceiveStreamInterface::Config::Rtp::ToString() const {
   ss << "{remote_ssrc: " << remote_ssrc;
   ss << ", local_ssrc: " << local_ssrc;
   ss << ", nack: " << nack.ToString();
+  ss << ", rtcp: "
+     << (rtcp_mode == RtcpMode::kCompound
+             ? "compound"
+             : (rtcp_mode == RtcpMode::kReducedSize ? "reducedSize" : "off"));
   ss << '}';
   return ss.str();
 }
@@ -126,6 +130,7 @@ AudioReceiveStreamImpl::AudioReceiveStreamImpl(
   // using the actual packet size for the configured codec.
   channel_receive_->SetNACKStatus(config.rtp.nack.rtp_history_ms != 0,
                                   config.rtp.nack.rtp_history_ms / 20);
+  channel_receive_->SetRtcpMode(config.rtp.rtcp_mode);
   channel_receive_->SetReceiveCodecs(config.decoder_map);
   // `frame_transformer` and `frame_decryptor` have been given to
   // `channel_receive_` already.
@@ -232,6 +237,16 @@ void AudioReceiveStreamImpl::SetNackHistory(int history_ms) {
   channel_receive_->SetNACKStatus(history_ms != 0, history_ms / 20);
 }
 
+void AudioReceiveStreamImpl::SetRtcpMode(webrtc::RtcpMode mode) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+
+  if (config_.rtp.rtcp_mode == mode)
+    return;
+
+  config_.rtp.rtcp_mode = mode;
+  channel_receive_->SetRtcpMode(mode);
+}
+
 void AudioReceiveStreamImpl::SetNonSenderRttMeasurement(bool enabled) {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);
   config_.enable_non_sender_rtt = enabled;

audio/audio_receive_stream.h

@@ -91,6 +91,7 @@ class AudioReceiveStreamImpl final : public webrtc::AudioReceiveStreamInterface,
       override;
   void SetDecoderMap(std::map<int, SdpAudioFormat> decoder_map) override;
   void SetNackHistory(int history_ms) override;
+  void SetRtcpMode(RtcpMode mode) override;
   void SetNonSenderRttMeasurement(bool enabled) override;
   void SetFrameDecryptor(rtc::scoped_refptr<webrtc::FrameDecryptorInterface>
                              frame_decryptor) override;

audio/audio_receive_stream_unittest.cc

@@ -120,6 +120,7 @@ struct ConfigHelper {
 
     channel_receive_ = new ::testing::StrictMock<MockChannelReceive>();
     EXPECT_CALL(*channel_receive_, SetNACKStatus(true, 15)).Times(1);
+    EXPECT_CALL(*channel_receive_, SetRtcpMode(_)).Times(1);
     EXPECT_CALL(*channel_receive_,
                 RegisterReceiverCongestionControlObjects(&packet_router_))
         .Times(1);
@@ -208,9 +209,10 @@ TEST(AudioReceiveStreamTest, ConfigToString) {
   AudioReceiveStreamInterface::Config config;
   config.rtp.remote_ssrc = kRemoteSsrc;
   config.rtp.local_ssrc = kLocalSsrc;
+  config.rtp.rtcp_mode = RtcpMode::kOff;
   EXPECT_EQ(
       "{rtp: {remote_ssrc: 1234, local_ssrc: 5678, nack: "
-      "{rtp_history_ms: 0}}, "
+      "{rtp_history_ms: 0}, rtcp: off}, "
       "rtcp_send_transport: null}",
       config.ToString());
 }

audio/channel_receive.cc

@@ -162,6 +162,7 @@ class ChannelReceive : public ChannelReceiveInterface,
 
   CallReceiveStatistics GetRTCPStatistics() const override;
   void SetNACKStatus(bool enable, int maxNumberOfPackets) override;
+  void SetRtcpMode(webrtc::RtcpMode mode) override;
   void SetNonSenderRttMeasurement(bool enabled) override;
 
   AudioMixer::Source::AudioFrameInfo GetAudioFrameWithInfo(
@@ -879,6 +880,11 @@ void ChannelReceive::SetNACKStatus(bool enable, int max_packets) {
   }
 }
 
+void ChannelReceive::SetRtcpMode(webrtc::RtcpMode mode) {
+  RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+  rtp_rtcp_->SetRTCPStatus(mode);
+}
+
 void ChannelReceive::SetNonSenderRttMeasurement(bool enabled) {
   RTC_DCHECK_RUN_ON(&worker_thread_checker_);
   rtp_rtcp_->SetNonSenderRttMeasurement(enabled);

audio/channel_receive.h

@@ -140,6 +140,7 @@ class ChannelReceiveInterface : public RtpPacketSinkInterface {
 
   virtual CallReceiveStatistics GetRTCPStatistics() const = 0;
   virtual void SetNACKStatus(bool enable, int max_packets) = 0;
+  virtual void SetRtcpMode(webrtc::RtcpMode mode) = 0;
   virtual void SetNonSenderRttMeasurement(bool enabled) = 0;
 
   virtual AudioMixer::Source::AudioFrameInfo GetAudioFrameWithInfo(

audio/mock_voe_channel_proxy.h

@@ -30,6 +30,7 @@ namespace test {
 class MockChannelReceive : public voe::ChannelReceiveInterface {
  public:
   MOCK_METHOD(void, SetNACKStatus, (bool enable, int max_packets), (override));
+  MOCK_METHOD(void, SetRtcpMode, (RtcpMode mode), (override));
   MOCK_METHOD(void, SetNonSenderRttMeasurement, (bool enabled), (override));
   MOCK_METHOD(void,
               RegisterReceiverCongestionControlObjects,

call/audio_receive_stream.h

@@ -117,6 +117,7 @@ class AudioReceiveStreamInterface : public MediaReceiveStreamInterface {
 
       // See NackConfig for description.
       NackConfig nack;
+      RtcpMode rtcp_mode = RtcpMode::kCompound;
     } rtp;
 
     // Receive-side RTT.

call/audio_send_stream.h

@@ -106,6 +106,9 @@ class AudioSendStream : public AudioSender {
 
       // RTCP CNAME, see RFC 3550.
       std::string c_name;
+
+      // Compound or reduced size RTCP.
+      RtcpMode rtcp_mode = RtcpMode::kCompound;
     } rtp;
 
     // Time interval between RTCP report for audio

call/receive_stream.h

@@ -65,6 +65,8 @@ class MediaReceiveStreamInterface : public ReceiveStreamInterface {
       rtc::scoped_refptr<webrtc::FrameDecryptorInterface> frame_decryptor) = 0;
 
   virtual std::vector<RtpSource> GetSources() const = 0;
+
+  virtual void SetRtcpMode(RtcpMode mode) = 0;
 };
 
 }  // namespace webrtc

call/video_receive_stream.h

@@ -301,8 +301,6 @@ class VideoReceiveStreamInterface : public MediaReceiveStreamInterface {
   // Cause eventual generation of a key frame from the sender.
   virtual void GenerateKeyFrame() = 0;
 
-  virtual void SetRtcpMode(RtcpMode mode) = 0;
-
   // Sets or clears a flexfec RTP sink. This affects `rtp.packet_sink_` and
   // `rtp.protected_by_flexfec` parts of the configuration. Must be called on
   // the packet delivery thread.

media/base/fake_media_engine.h

@@ -495,6 +495,7 @@ class FakeVoiceMediaReceiveChannel
 
   std::vector<webrtc::RtpSource> GetSources(uint32_t ssrc) const override;
   void SetReceiveNackEnabled(bool enabled) override {}
+  void SetRtcpMode(webrtc::RtcpMode mode) override {}
   void SetReceiveNonSenderRttEnabled(bool enabled) override {}
 
  private:

media/base/media_channel.h

@@ -819,7 +819,7 @@ struct MediaChannelParameters {
 
   std::vector<Codec> codecs;
   std::vector<webrtc::RtpExtension> extensions;
-  // For a send stream this is true if we've neogtiated a send direction,
+  // For a send stream this is true if we've negotiated a send direction,
   // for a receive stream this is true if we've negotiated a receive direction.
   bool is_stream_active = true;
 
@@ -841,7 +841,10 @@ struct MediaChannelParameters {
  protected:
   virtual std::map<std::string, std::string> ToStringMap() const {
     return {{"codecs", VectorToString(codecs)},
-            {"extensions", VectorToString(extensions)}};
+            {"extensions", VectorToString(extensions)},
+            {"rtcp", "{reduced_size:" + rtc::ToString(rtcp.reduced_size) +
+                         ", remote_estimate:" +
+                         rtc::ToString(rtcp.remote_estimate) + "}"}};
   }
 };
 
@@ -918,6 +921,7 @@ class VoiceMediaReceiveChannelInterface : public MediaReceiveChannelInterface {
       std::unique_ptr<webrtc::AudioSinkInterface> sink) = 0;
   virtual bool GetStats(VoiceMediaReceiveInfo* stats, bool reset_legacy) = 0;
   virtual void SetReceiveNackEnabled(bool enabled) = 0;
+  virtual void SetRtcpMode(webrtc::RtcpMode mode) = 0;
   virtual void SetReceiveNonSenderRttEnabled(bool enabled) = 0;
 };
 

media/engine/fake_webrtc_call.cc

@@ -123,6 +123,10 @@ void FakeAudioReceiveStream::SetNackHistory(int history_ms) {
   config_.rtp.nack.rtp_history_ms = history_ms;
 }
 
+void FakeAudioReceiveStream::SetRtcpMode(webrtc::RtcpMode mode) {
+  config_.rtp.rtcp_mode = mode;
+}
+
 void FakeAudioReceiveStream::SetNonSenderRttMeasurement(bool enabled) {
   config_.enable_non_sender_rtt = enabled;
 }

media/engine/fake_webrtc_call.h

@@ -122,6 +122,7 @@ class FakeAudioReceiveStream final
   void SetDecoderMap(
       std::map<int, webrtc::SdpAudioFormat> decoder_map) override;
   void SetNackHistory(int history_ms) override;
+  void SetRtcpMode(webrtc::RtcpMode mode) override;
   void SetNonSenderRttMeasurement(bool enabled) override;
   void SetFrameDecryptor(rtc::scoped_refptr<webrtc::FrameDecryptorInterface>
                              frame_decryptor) override;

media/engine/webrtc_voice_engine.cc

@@ -283,6 +283,7 @@ webrtc::AudioReceiveStreamInterface::Config BuildReceiveStreamConfig(
   config.rtp.remote_ssrc = remote_ssrc;
   config.rtp.local_ssrc = local_ssrc;
   config.rtp.nack.rtp_history_ms = use_nack ? kNackRtpHistoryMs : 0;
+  config.rtp.rtcp_mode = webrtc::RtcpMode::kCompound;
   if (!stream_ids.empty()) {
     config.sync_group = stream_ids[0];
   }
@@ -879,6 +880,17 @@ class WebRtcVoiceSendChannel::WebRtcAudioSendStream : public AudioSource::Sink {
     ReconfigureAudioSendStream(nullptr);
   }
 
+  void SetRtcpMode(webrtc::RtcpMode mode) {
+    bool reduced_size = mode == webrtc::RtcpMode::kReducedSize;
+    if (rtp_parameters_.rtcp.reduced_size == reduced_size) {
+      return;
+    }
+    rtp_parameters_.rtcp.reduced_size = reduced_size;
+    // Note: this is not wired up beyond this point. For all audio
+    // RTCP packets sent by a sender there is no difference.
+    ReconfigureAudioSendStream(nullptr);
+  }
+
   void SetFrameEncryptor(
       rtc::scoped_refptr<webrtc::FrameEncryptorInterface> frame_encryptor) {
     RTC_DCHECK_RUN_ON(&worker_thread_checker_);
@@ -1075,7 +1087,8 @@ class WebRtcVoiceSendChannel::WebRtcAudioSendStream : public AudioSource::Sink {
     }
 
     rtp_parameters_.rtcp.cname = config_.rtp.c_name;
-    rtp_parameters_.rtcp.reduced_size = false;
+    rtp_parameters_.rtcp.reduced_size =
+        config_.rtp.rtcp_mode == webrtc::RtcpMode::kReducedSize;
 
     // parameters.encodings[0].active could have changed.
     UpdateSendState();
@@ -1322,6 +1335,11 @@ bool WebRtcVoiceSendChannel::SetSenderParameters(
   if (send_codec_spec_ && !SetMaxSendBitrate(params.max_bandwidth_bps)) {
     return false;
   }
+  rtcp_mode_ = params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
+                                        : webrtc::RtcpMode::kCompound;
+  for (auto& it : send_streams_) {
+    it.second->SetRtcpMode(rtcp_mode_);
+  }
   return SetOptions(params.options);
 }
 
@@ -1935,6 +1953,11 @@ class WebRtcVoiceReceiveChannel::WebRtcAudioReceiveStream {
     stream_->SetNackHistory(use_nack ? kNackRtpHistoryMs : 0);
   }
 
+  void SetRtcpMode(webrtc::RtcpMode mode) {
+    RTC_DCHECK_RUN_ON(&worker_thread_checker_);
+    stream_->SetRtcpMode(mode);
+  }
+
   void SetNonSenderRttMeasurement(bool enabled) {
     RTC_DCHECK_RUN_ON(&worker_thread_checker_);
     stream_->SetNonSenderRttMeasurement(enabled);
@@ -2065,6 +2088,8 @@ bool WebRtcVoiceReceiveChannel::SetReceiverParameters(
     recv_rtp_extension_map_ =
         webrtc::RtpHeaderExtensionMap(recv_rtp_extensions_);
   }
+  SetRtcpMode(params.rtcp.reduced_size ? webrtc::RtcpMode::kReducedSize
+                                       : webrtc::RtcpMode::kCompound);
   return true;
 }
 
@@ -2087,6 +2112,8 @@ webrtc::RtpParameters WebRtcVoiceReceiveChannel::GetRtpReceiverParameters(
   for (const Codec& codec : recv_codecs_) {
     rtp_params.codecs.push_back(codec.ToCodecParameters());
   }
+  rtp_params.rtcp.reduced_size =
+      recv_rtcp_mode_ == webrtc::RtcpMode::kReducedSize;
   return rtp_params;
 }
 
@@ -2212,6 +2239,18 @@ void WebRtcVoiceReceiveChannel::SetReceiveNackEnabled(bool enabled) {
   }
 }
 
+void WebRtcVoiceReceiveChannel::SetRtcpMode(webrtc::RtcpMode mode) {
+  // Check if the reduced size RTCP status changed on the
+  // preferred send codec, and in that case reconfigure all receive streams.
+  if (recv_rtcp_mode_ != mode) {
+    RTC_LOG(LS_INFO) << "Changing RTCP mode on receive streams.";
+    recv_rtcp_mode_ = mode;
+    for (auto& kv : recv_streams_) {
+      kv.second->SetRtcpMode(recv_rtcp_mode_);
+    }
+  }
+}
+
 void WebRtcVoiceReceiveChannel::SetReceiveNonSenderRttEnabled(bool enabled) {
   // Check if the receive-side RTT status has changed on the preferred send
   // codec, in that case reconfigure all receive streams.

media/engine/webrtc_voice_engine.h

@@ -316,6 +316,7 @@ class WebRtcVoiceSendChannel final : public MediaChannelUtil,
   std::map<uint32_t, WebRtcAudioSendStream*> send_streams_;
   std::vector<webrtc::RtpExtension> send_rtp_extensions_;
   std::string mid_;
+  webrtc::RtcpMode rtcp_mode_;
 
   absl::optional<webrtc::AudioSendStream::Config::SendCodecSpec>
       send_codec_spec_;
@@ -424,6 +425,7 @@ class WebRtcVoiceReceiveChannel final
       override;
 
   void SetReceiveNackEnabled(bool enabled) override;
+  void SetRtcpMode(webrtc::RtcpMode mode) override;
   void SetReceiveNonSenderRttEnabled(bool enabled) override;
 
  private:
@@ -457,6 +459,7 @@ class WebRtcVoiceReceiveChannel final
 
   AudioOptions options_;
   bool recv_nack_enabled_ = false;
+  webrtc::RtcpMode recv_rtcp_mode_ = webrtc::RtcpMode::kCompound;
   bool enable_non_sender_rtt_ = false;
   bool playout_ = false;
   webrtc::Call* const call_ = nullptr;

pc/media_session.cc

@@ -391,9 +391,7 @@ RTCError CreateContentOffer(
     StreamParamsVec* current_streams,
     MediaContentDescription* offer) {
   offer->set_rtcp_mux(session_options.rtcp_mux_enabled);
-  if (offer->type() == cricket::MEDIA_TYPE_VIDEO) {
   offer->set_rtcp_reduced_size(true);
-  }
 
   // Build the vector of header extensions with directions for this
   // media_description's options.
@@ -1103,10 +1101,7 @@ bool CreateMediaContentAnswer(
   answer->set_rtp_header_extensions(negotiated_rtp_extensions);
 
   answer->set_rtcp_mux(session_options.rtcp_mux_enabled && offer->rtcp_mux());
-  if (answer->type() == cricket::MEDIA_TYPE_VIDEO) {
   answer->set_rtcp_reduced_size(offer->rtcp_reduced_size());
-  }
-
   answer->set_remote_estimate(offer->remote_estimate());
 
   AddSimulcastToMediaDescription(media_description_options, answer);

pc/sdp_offer_answer_unittest.cc

@@ -1338,4 +1338,59 @@ TEST_F(SdpOfferAnswerTest, MidBackfillDoesNotCheckAgainstBundleGroup) {
   EXPECT_TRUE(pc->CreateAnswerAndSetAsLocal());
 }
 
+TEST_F(SdpOfferAnswerTest, ReducedSizeNegotiated) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+
+  auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
+  auto video_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
+
+  ASSERT_TRUE(caller->ExchangeOfferAnswerWith(callee.get()));
+  auto receivers = callee->pc()->GetReceivers();
+  ASSERT_EQ(receivers.size(), 2u);
+  auto audio_recv_param = receivers[0]->GetParameters();
+  EXPECT_TRUE(audio_recv_param.rtcp.reduced_size);
+  auto video_recv_param = receivers[1]->GetParameters();
+  EXPECT_TRUE(video_recv_param.rtcp.reduced_size);
+
+  auto senders = caller->pc()->GetSenders();
+  ASSERT_EQ(senders.size(), 2u);
+  auto audio_send_param = senders[0]->GetParameters();
+  EXPECT_TRUE(audio_send_param.rtcp.reduced_size);
+  auto video_send_param = senders[1]->GetParameters();
+  EXPECT_TRUE(video_send_param.rtcp.reduced_size);
+}
+
+TEST_F(SdpOfferAnswerTest, ReducedSizeNotNegotiated) {
+  auto caller = CreatePeerConnection();
+  auto callee = CreatePeerConnection();
+
+  auto audio_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
+  auto video_transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
+
+  EXPECT_TRUE(caller->CreateOfferAndSetAsLocal());
+  std::string sdp;
+  caller->pc()->local_description()->ToString(&sdp);
+  // Remove rtcp-rsize attribute.
+  auto offer = CreateSessionDescription(
+      SdpType::kOffer, absl::StrReplaceAll(sdp, {{"a=rtcp-rsize\r\n", ""}}));
+  EXPECT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  auto answer = callee->CreateAnswerAndSetAsLocal();
+  EXPECT_TRUE(caller->SetRemoteDescription(std::move(answer)));
+
+  auto receivers = callee->pc()->GetReceivers();
+  ASSERT_EQ(receivers.size(), 2u);
+  auto audio_recv_param = receivers[0]->GetParameters();
+  EXPECT_FALSE(audio_recv_param.rtcp.reduced_size);
+  auto video_recv_param = receivers[1]->GetParameters();
+  EXPECT_FALSE(video_recv_param.rtcp.reduced_size);
+
+  auto senders = caller->pc()->GetSenders();
+  ASSERT_EQ(senders.size(), 2u);
+  auto audio_send_param = senders[0]->GetParameters();
+  EXPECT_FALSE(audio_send_param.rtcp.reduced_size);
+  auto video_send_param = senders[1]->GetParameters();
+  EXPECT_FALSE(video_send_param.rtcp.reduced_size);
+}
+
 }  // namespace webrtc

pc/test/mock_voice_media_receive_channel_interface.h

@@ -68,6 +68,7 @@ class MockVoiceMediaReceiveChannelInterface
               (VoiceMediaReceiveInfo * stats, bool reset_legacy),
               (override));
   MOCK_METHOD(void, SetReceiveNackEnabled, (bool enabled), (override));
+  MOCK_METHOD(void, SetRtcpMode, (webrtc::RtcpMode mode), (override));
   MOCK_METHOD(void, SetReceiveNonSenderRttEnabled, (bool enabled), (override));
 
   // MediaReceiveChannelInterface