Reland "Remove use of ReceiveStreamRtpConfig:transport_cc"
This is a reland of commit 97ba853295578975a04fc504315cccd465f9f0bd
This cl did not cause the regression in Chrome rolls https://chromium-review.googlesource.com/c/chromium/src/+/4132644?tab=checks. Real culprit reverted in https://webrtc-review.googlesource.com/c/src/+/290502.
Original change's description:
> Remove use of ReceiveStreamRtpConfig:transport_cc
>
> With this change, webrtc will send RTCP transport feedback for all received packets that have a transport sequence number, if the header extension
> http://www.ietf.org/id/draft-holmer-rmcat-transport-wide-cc-extensions is negotiated.
> I.e the SDP attribute a=rtcp-fb:96 transport-cc is ignored.
>
>
> Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
>
> Bug: webrtc:14802
> Change-Id: I95d8d4405dc86a2f872f7883b7bafd623d5f7841
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290403
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Commit-Queue: Per Kjellander <perkj@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#38980}
Bug: webrtc:14802
Change-Id: Ib98e61413161d462da60144942cdb0140e12bc42
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/290503
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38997}
diff --git a/video/end_to_end_tests/bandwidth_tests.cc b/video/end_to_end_tests/bandwidth_tests.cc
index 986ced4..200b6fc 100644
--- a/video/end_to_end_tests/bandwidth_tests.cc
+++ b/video/end_to_end_tests/bandwidth_tests.cc
@@ -55,7 +55,6 @@
send_config->rtp.extensions.clear();
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeExtensionId));
- (*receive_configs)[0].rtp.transport_cc = false;
}
Action OnReceiveRtcp(const uint8_t* packet, size_t length) override {
@@ -106,12 +105,10 @@
if (!send_side_bwe_) {
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kAbsSendTimeUri, kAbsSendTimeExtensionId));
- (*receive_configs)[0].rtp.transport_cc = false;
} else {
send_config->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
kTransportSequenceNumberId));
- (*receive_configs)[0].rtp.transport_cc = true;
}
// Force a too high encoder bitrate to make sure we get pacer delay.
diff --git a/video/end_to_end_tests/rtp_rtcp_tests.cc b/video/end_to_end_tests/rtp_rtcp_tests.cc
index c7a3448..ea9b399 100644
--- a/video/end_to_end_tests/rtp_rtcp_tests.cc
+++ b/video/end_to_end_tests/rtp_rtcp_tests.cc
@@ -540,7 +540,6 @@
flexfec_receive_config.protected_media_ssrcs =
GetVideoSendConfig()->rtp.flexfec.protected_media_ssrcs;
flexfec_receive_config.rtp.local_ssrc = kReceiverLocalVideoSsrc;
- flexfec_receive_config.rtp.transport_cc = true;
flexfec_receive_config.rtp.extensions.emplace_back(
RtpExtension::kTransportSequenceNumberUri,
kTransportSequenceNumberExtensionId);
diff --git a/video/end_to_end_tests/transport_feedback_tests.cc b/video/end_to_end_tests/transport_feedback_tests.cc
index 1e95140..dbe3f0c8 100644
--- a/video/end_to_end_tests/transport_feedback_tests.cc
+++ b/video/end_to_end_tests/transport_feedback_tests.cc
@@ -244,11 +244,8 @@
class TransportFeedbackTester : public test::EndToEndTest {
public:
- TransportFeedbackTester(bool feedback_enabled,
- size_t num_video_streams,
- size_t num_audio_streams)
+ TransportFeedbackTester(size_t num_video_streams, size_t num_audio_streams)
: EndToEndTest(::webrtc::TransportFeedbackEndToEndTest::kDefaultTimeout),
- feedback_enabled_(feedback_enabled),
num_video_streams_(num_video_streams),
num_audio_streams_(num_audio_streams),
receiver_call_(nullptr) {
@@ -276,11 +273,7 @@
}
void PerformTest() override {
- constexpr TimeDelta kDisabledFeedbackTimeout = TimeDelta::Seconds(5);
- EXPECT_EQ(feedback_enabled_,
- observation_complete_.Wait(feedback_enabled_
- ? test::CallTest::kDefaultTimeout
- : kDisabledFeedbackTimeout));
+ EXPECT_TRUE(observation_complete_.Wait(test::CallTest::kDefaultTimeout));
}
void OnCallsCreated(Call* sender_call, Call* receiver_call) override {
@@ -290,13 +283,6 @@
size_t GetNumVideoStreams() const override { return num_video_streams_; }
size_t GetNumAudioStreams() const override { return num_audio_streams_; }
- void ModifyVideoConfigs(
- VideoSendStream::Config* send_config,
- std::vector<VideoReceiveStreamInterface::Config>* receive_configs,
- VideoEncoderConfig* encoder_config) override {
- (*receive_configs)[0].rtp.transport_cc = feedback_enabled_;
- }
-
void ModifyAudioConfigs(AudioSendStream::Config* send_config,
std::vector<AudioReceiveStreamInterface::Config>*
receive_configs) override {
@@ -306,38 +292,25 @@
kTransportSequenceNumberExtensionId));
(*receive_configs)[0].rtp.extensions.clear();
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
- (*receive_configs)[0].rtp.transport_cc = feedback_enabled_;
}
private:
- const bool feedback_enabled_;
const size_t num_video_streams_;
const size_t num_audio_streams_;
Call* receiver_call_;
};
TEST_F(TransportFeedbackEndToEndTest, VideoReceivesTransportFeedback) {
- TransportFeedbackTester test(true, 1, 0);
+ TransportFeedbackTester test(1, 0);
RunBaseTest(&test);
}
-
-TEST_F(TransportFeedbackEndToEndTest, VideoTransportFeedbackNotConfigured) {
- TransportFeedbackTester test(false, 1, 0);
- RunBaseTest(&test);
-}
-
TEST_F(TransportFeedbackEndToEndTest, AudioReceivesTransportFeedback) {
- TransportFeedbackTester test(true, 0, 1);
- RunBaseTest(&test);
-}
-
-TEST_F(TransportFeedbackEndToEndTest, AudioTransportFeedbackNotConfigured) {
- TransportFeedbackTester test(false, 0, 1);
+ TransportFeedbackTester test(0, 1);
RunBaseTest(&test);
}
TEST_F(TransportFeedbackEndToEndTest, AudioVideoReceivesTransportFeedback) {
- TransportFeedbackTester test(true, 1, 1);
+ TransportFeedbackTester test(1, 1);
RunBaseTest(&test);
}
diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc
index 58283d7..b8eccb3 100644
--- a/video/video_quality_test.cc
+++ b/video/video_quality_test.cc
@@ -827,9 +827,8 @@
if (!decode_all_receive_streams)
decode_sub_stream = params_.ss[video_idx].selected_stream;
CreateMatchingVideoReceiveConfigs(
- video_send_configs_[video_idx], recv_transport,
- params_.call.send_side_bwe, &video_decoder_factory_, decode_sub_stream,
- true, kNackRtpHistoryMs);
+ video_send_configs_[video_idx], recv_transport, &video_decoder_factory_,
+ decode_sub_stream, true, kNackRtpHistoryMs);
if (params_.screenshare[video_idx].enabled) {
// Fill out codec settings.
@@ -934,7 +933,6 @@
}
CreateMatchingFecConfig(recv_transport, *GetVideoSendConfig());
- GetFlexFecConfig()->rtp.transport_cc = params_.call.send_side_bwe;
if (params_.call.send_side_bwe) {
GetFlexFecConfig()->rtp.extensions.push_back(
RtpExtension(RtpExtension::kTransportSequenceNumberUri,
@@ -1002,8 +1000,7 @@
AddMatchingVideoReceiveConfigs(
&thumbnail_receive_configs_, thumbnail_send_config, send_transport,
- params_.call.send_side_bwe, &video_decoder_factory_, absl::nullopt,
- false, kNackRtpHistoryMs);
+ &video_decoder_factory_, absl::nullopt, false, kNackRtpHistoryMs);
}
for (size_t i = 0; i < thumbnail_send_configs_.size(); ++i) {
thumbnail_send_streams_.push_back(receiver_call_->CreateVideoSendStream(
diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc
index 9a95c58..ce96512 100644
--- a/video/video_receive_stream2.cc
+++ b/video/video_receive_stream2.cc
@@ -463,17 +463,6 @@
return rtp_video_stream_receiver_.GetRtpExtensions();
}
-bool VideoReceiveStream2::transport_cc() const {
- RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
- return config_.rtp.transport_cc;
-}
-
-void VideoReceiveStream2::SetTransportCc(bool transport_cc) {
- RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
- // TODO(tommi): Stop using the config struct for the internal state.
- const_cast<bool&>(config_.rtp.transport_cc) = transport_cc;
-}
-
void VideoReceiveStream2::SetRtcpMode(RtcpMode mode) {
RTC_DCHECK_RUN_ON(&packet_sequence_checker_);
// TODO(tommi): Stop using the config struct for the internal state.
diff --git a/video/video_receive_stream2.h b/video/video_receive_stream2.h
index 34937a2..44e2228 100644
--- a/video/video_receive_stream2.h
+++ b/video/video_receive_stream2.h
@@ -144,8 +144,6 @@
void SetRtpExtensions(std::vector<RtpExtension> extensions) override;
RtpHeaderExtensionMap GetRtpExtensionMap() const override;
- bool transport_cc() const override;
- void SetTransportCc(bool transport_cc) override;
void SetRtcpMode(RtcpMode mode) override;
void SetFlexFecProtection(RtpPacketSinkInterface* flexfec_sink) override;
void SetLossNotificationEnabled(bool enabled) override;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 923c318..d4da883 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -1615,7 +1615,6 @@
send_config->rtp.extensions.push_back(RtpExtension(
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
- (*receive_configs)[0].rtp.transport_cc = true;
}
void ModifyAudioConfigs(AudioSendStream::Config* send_config,
@@ -1627,7 +1626,6 @@
RtpExtension::kTransportSequenceNumberUri, kExtensionId));
(*receive_configs)[0].rtp.extensions.clear();
(*receive_configs)[0].rtp.extensions = send_config->rtp.extensions;
- (*receive_configs)[0].rtp.transport_cc = true;
}
Action OnSendRtp(const uint8_t* packet, size_t length) override {
