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}
diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc
index 415ad06..31e93b3 100644
--- a/audio/audio_receive_stream.cc
+++ b/audio/audio_receive_stream.cc
@@ -39,6 +39,10 @@
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 @@
// 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 @@
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;
diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h
index db49631..0d3d946 100644
--- a/audio/audio_receive_stream.h
+++ b/audio/audio_receive_stream.h
@@ -91,6 +91,7 @@
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;
diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc
index 451d5f9..25e7e7b 100644
--- a/audio/audio_receive_stream_unittest.cc
+++ b/audio/audio_receive_stream_unittest.cc
@@ -120,6 +120,7 @@
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 @@
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());
}
diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc
index 6f0553c..1fa396e 100644
--- a/audio/channel_receive.cc
+++ b/audio/channel_receive.cc
@@ -162,6 +162,7 @@
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::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);
diff --git a/audio/channel_receive.h b/audio/channel_receive.h
index ab69103..61212f6 100644
--- a/audio/channel_receive.h
+++ b/audio/channel_receive.h
@@ -140,6 +140,7 @@
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(
diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h
index 71ef5d1..5836d88 100644
--- a/audio/mock_voe_channel_proxy.h
+++ b/audio/mock_voe_channel_proxy.h
@@ -30,6 +30,7 @@
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,
diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h
index 4879311..344a4b6 100644
--- a/call/audio_receive_stream.h
+++ b/call/audio_receive_stream.h
@@ -117,6 +117,7 @@
// See NackConfig for description.
NackConfig nack;
+ RtcpMode rtcp_mode = RtcpMode::kCompound;
} rtp;
// Receive-side RTT.
diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h
index 798b1cd..1b7a906 100644
--- a/call/audio_send_stream.h
+++ b/call/audio_send_stream.h
@@ -106,6 +106,9 @@
// 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
diff --git a/call/receive_stream.h b/call/receive_stream.h
index 8a99059..287ddc4 100644
--- a/call/receive_stream.h
+++ b/call/receive_stream.h
@@ -65,6 +65,8 @@
rtc::scoped_refptr<webrtc::FrameDecryptorInterface> frame_decryptor) = 0;
virtual std::vector<RtpSource> GetSources() const = 0;
+
+ virtual void SetRtcpMode(RtcpMode mode) = 0;
};
} // namespace webrtc
diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h
index 20e9245..59ffd39 100644
--- a/call/video_receive_stream.h
+++ b/call/video_receive_stream.h
@@ -301,8 +301,6 @@
// 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.
diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h
index b56af0e..64af78d 100644
--- a/media/base/fake_media_engine.h
+++ b/media/base/fake_media_engine.h
@@ -495,6 +495,7 @@
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:
diff --git a/media/base/media_channel.h b/media/base/media_channel.h
index 3721e5f..9e024d1 100644
--- a/media/base/media_channel.h
+++ b/media/base/media_channel.h
@@ -819,7 +819,7 @@
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 @@
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 @@
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;
};
diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc
index d4c6b23..f5a1f69 100644
--- a/media/engine/fake_webrtc_call.cc
+++ b/media/engine/fake_webrtc_call.cc
@@ -123,6 +123,10 @@
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;
}
diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h
index 9a6bc0a..7b76e10 100644
--- a/media/engine/fake_webrtc_call.h
+++ b/media/engine/fake_webrtc_call.h
@@ -122,6 +122,7 @@
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;
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index db43848..e76cebc 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -283,6 +283,7 @@
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 @@
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 @@
}
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 @@
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 @@
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 @@
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 @@
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::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.
diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h
index 379e91d..13ec681 100644
--- a/media/engine/webrtc_voice_engine.h
+++ b/media/engine/webrtc_voice_engine.h
@@ -316,6 +316,7 @@
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 @@
override;
void SetReceiveNackEnabled(bool enabled) override;
+ void SetRtcpMode(webrtc::RtcpMode mode) override;
void SetReceiveNonSenderRttEnabled(bool enabled) override;
private:
@@ -457,6 +459,7 @@
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;
diff --git a/pc/media_session.cc b/pc/media_session.cc
index 921749e..e3aa032 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -391,9 +391,7 @@
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);
- }
+ offer->set_rtcp_reduced_size(true);
// Build the vector of header extensions with directions for this
// media_description's options.
@@ -1103,10 +1101,7 @@
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_rtcp_reduced_size(offer->rtcp_reduced_size());
answer->set_remote_estimate(offer->remote_estimate());
AddSimulcastToMediaDescription(media_description_options, answer);
diff --git a/pc/sdp_offer_answer_unittest.cc b/pc/sdp_offer_answer_unittest.cc
index 3a6f510..bef1815 100644
--- a/pc/sdp_offer_answer_unittest.cc
+++ b/pc/sdp_offer_answer_unittest.cc
@@ -1338,4 +1338,59 @@
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
diff --git a/pc/test/mock_voice_media_receive_channel_interface.h b/pc/test/mock_voice_media_receive_channel_interface.h
index adb1201..8c0f837 100644
--- a/pc/test/mock_voice_media_receive_channel_interface.h
+++ b/pc/test/mock_voice_media_receive_channel_interface.h
@@ -68,6 +68,7 @@
(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
