diff --git a/audio/audio_receive_stream.cc b/audio/audio_receive_stream.cc index 415ad0640a..31e93b3c7d 100644 --- a/audio/audio_receive_stream.cc +++ b/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; diff --git a/audio/audio_receive_stream.h b/audio/audio_receive_stream.h index db49631638..0d3d9460cf 100644 --- a/audio/audio_receive_stream.h +++ b/audio/audio_receive_stream.h @@ -91,6 +91,7 @@ class AudioReceiveStreamImpl final : public webrtc::AudioReceiveStreamInterface, override; void SetDecoderMap(std::map decoder_map) override; void SetNackHistory(int history_ms) override; + void SetRtcpMode(RtcpMode mode) override; void SetNonSenderRttMeasurement(bool enabled) override; void SetFrameDecryptor(rtc::scoped_refptr frame_decryptor) override; diff --git a/audio/audio_receive_stream_unittest.cc b/audio/audio_receive_stream_unittest.cc index 451d5f9b91..25e7e7bfa6 100644 --- a/audio/audio_receive_stream_unittest.cc +++ b/audio/audio_receive_stream_unittest.cc @@ -120,6 +120,7 @@ struct ConfigHelper { channel_receive_ = new ::testing::StrictMock(); 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()); } diff --git a/audio/channel_receive.cc b/audio/channel_receive.cc index 6f0553cb61..1fa396e482 100644 --- a/audio/channel_receive.cc +++ b/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); diff --git a/audio/channel_receive.h b/audio/channel_receive.h index ab69103269..61212f60fe 100644 --- a/audio/channel_receive.h +++ b/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( diff --git a/audio/mock_voe_channel_proxy.h b/audio/mock_voe_channel_proxy.h index 71ef5d12fb..5836d8838a 100644 --- a/audio/mock_voe_channel_proxy.h +++ b/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, diff --git a/call/audio_receive_stream.h b/call/audio_receive_stream.h index 4879311fdb..344a4b64e4 100644 --- a/call/audio_receive_stream.h +++ b/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. diff --git a/call/audio_send_stream.h b/call/audio_send_stream.h index 798b1cdb9f..1b7a9065d5 100644 --- a/call/audio_send_stream.h +++ b/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 diff --git a/call/receive_stream.h b/call/receive_stream.h index 8a99059ec5..287ddc47a8 100644 --- a/call/receive_stream.h +++ b/call/receive_stream.h @@ -65,6 +65,8 @@ class MediaReceiveStreamInterface : public ReceiveStreamInterface { rtc::scoped_refptr frame_decryptor) = 0; virtual std::vector 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 20e9245584..59ffd39548 100644 --- a/call/video_receive_stream.h +++ b/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. diff --git a/media/base/fake_media_engine.h b/media/base/fake_media_engine.h index b56af0e99e..64af78d165 100644 --- a/media/base/fake_media_engine.h +++ b/media/base/fake_media_engine.h @@ -495,6 +495,7 @@ class FakeVoiceMediaReceiveChannel std::vector 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 3721e5f3b1..9e024d1ae3 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -819,7 +819,7 @@ struct MediaChannelParameters { std::vector codecs; std::vector 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 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 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 d4c6b23767..f5a1f693e4 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/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; } diff --git a/media/engine/fake_webrtc_call.h b/media/engine/fake_webrtc_call.h index 9a6bc0a246..7b76e10fbf 100644 --- a/media/engine/fake_webrtc_call.h +++ b/media/engine/fake_webrtc_call.h @@ -122,6 +122,7 @@ class FakeAudioReceiveStream final void SetDecoderMap( std::map 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 frame_decryptor) override; diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc index db43848224..e76cebc8ff 100644 --- a/media/engine/webrtc_voice_engine.cc +++ b/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 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. diff --git a/media/engine/webrtc_voice_engine.h b/media/engine/webrtc_voice_engine.h index 379e91d56b..13ec681c6a 100644 --- a/media/engine/webrtc_voice_engine.h +++ b/media/engine/webrtc_voice_engine.h @@ -316,6 +316,7 @@ class WebRtcVoiceSendChannel final : public MediaChannelUtil, std::map send_streams_; std::vector send_rtp_extensions_; std::string mid_; + webrtc::RtcpMode rtcp_mode_; absl::optional 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; diff --git a/pc/media_session.cc b/pc/media_session.cc index 921749e65b..e3aa0320ea 100644 --- a/pc/media_session.cc +++ b/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); - } + 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_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 3a6f510864..bef18154ee 100644 --- a/pc/sdp_offer_answer_unittest.cc +++ b/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 diff --git a/pc/test/mock_voice_media_receive_channel_interface.h b/pc/test/mock_voice_media_receive_channel_interface.h index adb1201239..8c0f837765 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 @@ 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