Revert "Allow sending to separate payload types for each simulcast index."
This reverts commit bcb19c00ba8ab1788ba3c08f28ee1b23e0cc77b9.
Reason for revert: speculative revert
Original change's description:
> Allow sending to separate payload types for each simulcast index.
>
> This change is for mixed-codec simulcast.
>
> By obtaining the payload type via RtpConfig::GetStreamConfig(),
> the correct payload type can be retrieved regardless of whether
> RtpConfig::stream_configs is initialized or not.
>
> Bug: webrtc:362277533
> Change-Id: I6b2a1ae66356b20a832565ce6729c3ce9e73a161
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/364760
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Commit-Queue: Florent Castelli <orphis@webrtc.org>
> Reviewed-by: Florent Castelli <orphis@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43197}
Bug: webrtc:362277533
Change-Id: I50ac1fa0d9963bf9796f8604542aef5cec653493
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/365161
Commit-Queue: Jeremy Leconte <jleconte@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/main@{#43208}
diff --git a/call/rtp_config.cc b/call/rtp_config.cc
index 1691129..02cc0f4 100644
--- a/call/rtp_config.cc
+++ b/call/rtp_config.cc
@@ -239,31 +239,4 @@
return std::nullopt;
}
-RtpStreamConfig RtpConfig::GetStreamConfig(size_t index) const {
- // GetStreamConfig function usually returns stream_configs[index], but if
- // stream_configs is not initialized (i.e., index >= stream_configs.size()),
- // it creates and returns an RtpStreamConfig using fields such as ssrcs, rids,
- // payload_name, and payload_type from RtpConfig.
- RTC_DCHECK_LT(index, ssrcs.size());
- if (index < stream_configs.size()) {
- return stream_configs[index];
- }
- RtpStreamConfig stream_config;
- stream_config.ssrc = ssrcs[index];
- if (index < rids.size()) {
- stream_config.rid = rids[index];
- }
- stream_config.payload_name = payload_name;
- stream_config.payload_type = payload_type;
- stream_config.raw_payload = raw_payload;
- if (!rtx.ssrcs.empty()) {
- RTC_DCHECK_EQ(ssrcs.size(), rtx.ssrcs.size());
- auto& stream_config_rtx = stream_config.rtx.emplace();
- stream_config_rtx.ssrc = rtx.ssrcs[index];
- stream_config_rtx.payload_type = rtx.payload_type;
- }
-
- return stream_config;
-}
-
} // namespace webrtc
diff --git a/call/rtp_config.h b/call/rtp_config.h
index d77289f..6b79e55 100644
--- a/call/rtp_config.h
+++ b/call/rtp_config.h
@@ -193,9 +193,6 @@
uint32_t GetMediaSsrcAssociatedWithRtxSsrc(uint32_t rtx_ssrc) const;
uint32_t GetMediaSsrcAssociatedWithFlexfecSsrc(uint32_t flexfec_ssrc) const;
std::optional<std::string> GetRidForSsrc(uint32_t ssrc) const;
-
- // Returns send config for RTP stream by provided simulcast `index`.
- RtpStreamConfig GetStreamConfig(size_t index) const;
};
} // namespace webrtc
#endif // CALL_RTP_CONFIG_H_
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 3e931de..be56eac 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -470,14 +470,12 @@
}
bool fec_enabled = false;
- for (size_t i = 0; i < rtp_streams_.size(); i++) {
- const RtpStreamSender& stream = rtp_streams_[i];
+ for (const RtpStreamSender& stream : rtp_streams_) {
// Simulcast has one module for each layer. Set the CNAME on all modules.
stream.rtp_rtcp->SetCNAME(rtp_config_.c_name.c_str());
stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
- stream.rtp_rtcp->RegisterSendPayloadFrequency(
- rtp_config_.GetStreamConfig(i).payload_type,
- kVideoPayloadTypeFrequency);
+ stream.rtp_rtcp->RegisterSendPayloadFrequency(rtp_config_.payload_type,
+ kVideoPayloadTypeFrequency);
if (stream.fec_generator != nullptr) {
fec_enabled = true;
}
@@ -578,7 +576,7 @@
// knowledge of the offset to a single place.
if (!rtp_streams_[simulcast_index].rtp_rtcp->OnSendingRtpFrame(
encoded_image.RtpTimestamp(), encoded_image.capture_time_ms_,
- rtp_config_.GetStreamConfig(simulcast_index).payload_type,
+ rtp_config_.payload_type,
encoded_image._frameType == VideoFrameType::kVideoFrameKey)) {
// The payload router could be active but this module isn't sending.
return Result(Result::ERROR_SEND_FAILED);
@@ -618,8 +616,7 @@
bool send_result =
rtp_streams_[simulcast_index].sender_video->SendEncodedImage(
- rtp_config_.GetStreamConfig(simulcast_index).payload_type,
- codec_type_, rtp_timestamp, encoded_image,
+ rtp_config_.payload_type, codec_type_, rtp_timestamp, encoded_image,
params_[simulcast_index].GetRtpVideoHeader(
encoded_image, codec_specific_info, frame_id),
expected_retransmission_time);
@@ -757,12 +754,9 @@
// Configure RTX payload types.
RTC_DCHECK_GE(rtp_config_.rtx.payload_type, 0);
- for (size_t i = 0; i < rtp_streams_.size(); ++i) {
- const RtpStreamSender& stream = rtp_streams_[i];
- RtpStreamConfig stream_config = rtp_config_.GetStreamConfig(i);
- RTC_DCHECK(stream_config.rtx);
- stream.rtp_rtcp->SetRtxSendPayloadType(stream_config.rtx->payload_type,
- stream_config.payload_type);
+ for (const RtpStreamSender& stream : rtp_streams_) {
+ stream.rtp_rtcp->SetRtxSendPayloadType(rtp_config_.rtx.payload_type,
+ rtp_config_.payload_type);
stream.rtp_rtcp->SetRtxSendStatus(kRtxRetransmitted |
kRtxRedundantPayloads);
}
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 5e8ab85..32fa2cd 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -82,7 +82,6 @@
using ::testing::SizeIs;
const int8_t kPayloadType = 96;
-const int8_t kPayloadType2 = 98;
const uint32_t kSsrc1 = 12345;
const uint32_t kSsrc2 = 23456;
const uint32_t kRtxSsrc1 = 34567;
@@ -132,8 +131,7 @@
Transport* transport,
const std::vector<uint32_t>& ssrcs,
const std::vector<uint32_t>& rtx_ssrcs,
- int payload_type,
- rtc::ArrayView<const int> payload_types) {
+ int payload_type) {
VideoSendStream::Config config(transport);
config.rtp.ssrcs = ssrcs;
config.rtp.rtx.ssrcs = rtx_ssrcs;
@@ -145,20 +143,6 @@
config.rtp.extensions.emplace_back(RtpDependencyDescriptorExtension::Uri(),
kDependencyDescriptorExtensionId);
config.rtp.extmap_allow_mixed = true;
-
- if (!payload_types.empty()) {
- RTC_CHECK_EQ(payload_types.size(), ssrcs.size());
- for (size_t i = 0; i < ssrcs.size(); ++i) {
- auto& stream_config = config.rtp.stream_configs.emplace_back();
- stream_config.ssrc = ssrcs[i];
- stream_config.payload_type = payload_types[i];
- if (i < rtx_ssrcs.size()) {
- auto& rtx = stream_config.rtx.emplace();
- rtx.ssrc = rtx_ssrcs[i];
- rtx.payload_type = payload_types[i] + 1;
- }
- }
- }
return config;
}
@@ -171,7 +155,6 @@
const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
FrameCountObserver* frame_count_observer,
rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
- const std::vector<int>& payload_types,
const FieldTrialsView* field_trials = nullptr)
: time_controller_(Timestamp::Millis(1000000)),
env_(CreateEnvironment(&field_trials_,
@@ -181,8 +164,7 @@
config_(CreateVideoSendStreamConfig(&transport_,
ssrcs,
rtx_ssrcs,
- payload_type,
- payload_types)),
+ payload_type)),
bitrate_config_(GetBitrateConfig()),
transport_controller_(
RtpTransportConfig{.env = env_, .bitrate_config = bitrate_config_}),
@@ -204,22 +186,6 @@
std::make_unique<FecControllerDefault>(env_), nullptr, CryptoOptions{},
frame_transformer);
}
- RtpVideoSenderTestFixture(
- const std::vector<uint32_t>& ssrcs,
- const std::vector<uint32_t>& rtx_ssrcs,
- int payload_type,
- const std::map<uint32_t, RtpPayloadState>& suspended_payload_states,
- FrameCountObserver* frame_count_observer,
- rtc::scoped_refptr<FrameTransformerInterface> frame_transformer,
- const FieldTrialsView* field_trials = nullptr)
- : RtpVideoSenderTestFixture(ssrcs,
- rtx_ssrcs,
- payload_type,
- suspended_payload_states,
- frame_count_observer,
- frame_transformer,
- /*payload_types=*/{},
- field_trials) {}
RtpVideoSenderTestFixture(
const std::vector<uint32_t>& ssrcs,
@@ -234,7 +200,6 @@
suspended_payload_states,
frame_count_observer,
/*frame_transformer=*/nullptr,
- /*payload_types=*/{},
field_trials) {}
RtpVideoSenderTestFixture(
@@ -249,7 +214,6 @@
suspended_payload_states,
/*frame_count_observer=*/nullptr,
/*frame_transformer=*/nullptr,
- /*payload_types=*/{},
field_trials) {}
~RtpVideoSenderTestFixture() { SetSending(false); }
@@ -987,79 +951,6 @@
EXPECT_EQ(dd_s1.frame_number(), 1002);
}
-TEST(RtpVideoSenderTest, MixedCodecSimulcastPayloadType) {
- // When multiple payload types are set, verify that the payload type switches
- // corresponding to the simulcast index.
- RtpVideoSenderTestFixture test({kSsrc1, kSsrc2}, {kRtxSsrc1, kRtxSsrc2},
- kPayloadType, {}, nullptr, nullptr,
- {kPayloadType, kPayloadType2});
- test.SetSending(true);
-
- std::vector<uint16_t> rtp_sequence_numbers;
- std::vector<RtpPacket> sent_packets;
- EXPECT_CALL(test.transport(), SendRtp)
- .Times(3)
- .WillRepeatedly([&](rtc::ArrayView<const uint8_t> packet,
- const PacketOptions& options) -> bool {
- RtpPacket& rtp_packet = sent_packets.emplace_back();
- EXPECT_TRUE(rtp_packet.Parse(packet));
- rtp_sequence_numbers.push_back(rtp_packet.SequenceNumber());
- return true;
- });
-
- const uint8_t kPayload[1] = {'a'};
- EncodedImage encoded_image;
- encoded_image.SetEncodedData(
- EncodedImageBuffer::Create(kPayload, sizeof(kPayload)));
-
- CodecSpecificInfo codec_specific;
- codec_specific.codecType = VideoCodecType::kVideoCodecVP8;
-
- encoded_image.SetSimulcastIndex(0);
- ASSERT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error,
- EncodedImageCallback::Result::OK);
- ASSERT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error,
- EncodedImageCallback::Result::OK);
- encoded_image.SetSimulcastIndex(1);
- ASSERT_EQ(test.router()->OnEncodedImage(encoded_image, &codec_specific).error,
- EncodedImageCallback::Result::OK);
-
- test.AdvanceTime(TimeDelta::Millis(33));
- ASSERT_THAT(sent_packets, SizeIs(3));
- EXPECT_EQ(sent_packets[0].PayloadType(), kPayloadType);
- EXPECT_EQ(sent_packets[1].PayloadType(), kPayloadType);
- EXPECT_EQ(sent_packets[2].PayloadType(), kPayloadType2);
-
- // Verify that NACK is sent to the RTX payload type corresponding to the
- // payload type.
- rtcp::Nack nack1, nack2;
- nack1.SetMediaSsrc(kSsrc1);
- nack2.SetMediaSsrc(kSsrc2);
- nack1.SetPacketIds({rtp_sequence_numbers[0], rtp_sequence_numbers[1]});
- nack2.SetPacketIds({rtp_sequence_numbers[2]});
- rtc::Buffer nack_buffer1 = nack1.Build();
- rtc::Buffer nack_buffer2 = nack2.Build();
-
- std::vector<RtpPacket> sent_rtx_packets;
- EXPECT_CALL(test.transport(), SendRtp)
- .Times(3)
- .WillRepeatedly([&](rtc::ArrayView<const uint8_t> packet,
- const PacketOptions& options) {
- RtpPacket& rtp_packet = sent_rtx_packets.emplace_back();
- EXPECT_TRUE(rtp_packet.Parse(packet));
- return true;
- });
- test.router()->DeliverRtcp(nack_buffer1.data(), nack_buffer1.size());
- test.router()->DeliverRtcp(nack_buffer2.data(), nack_buffer2.size());
-
- test.AdvanceTime(TimeDelta::Millis(33));
-
- ASSERT_THAT(sent_rtx_packets, SizeIs(3));
- EXPECT_EQ(sent_rtx_packets[0].PayloadType(), kPayloadType + 1);
- EXPECT_EQ(sent_rtx_packets[1].PayloadType(), kPayloadType + 1);
- EXPECT_EQ(sent_rtx_packets[2].PayloadType(), kPayloadType2 + 1);
-}
-
TEST(RtpVideoSenderTest,
SupportsDependencyDescriptorForVp8NotProvidedByEncoder) {
constexpr uint8_t kPayload[1] = {'a'};
