Revert "Reland "Allow sending to separate payload types for each simulcast index.""
This reverts commit 49ac6b758cc3c28be2fc13028a829f016b453d39.
Reason for revert: Break codec switch in singlecast.
Original change's description:
> Reland "Allow sending to separate payload types for each simulcast index."
>
> This is a reland of commit bcb19c00ba8ab1788ba3c08f28ee1b23e0cc77b9
>
> 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: Ia82c3390cceb9f68315c2fd9ba5114693669af32
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374780
> Commit-Queue: Henrik Boström <hbos@webrtc.org>
> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
> Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#43787}
Bug: webrtc:362277533
Change-Id: Ife7d43471c85fdea9bd26cc982bce410c0d75527
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376040
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43830}
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 8964e4b..37cab22 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -330,13 +330,11 @@
return rtp_streams;
}
-std::optional<VideoCodecType> GetVideoCodecType(const RtpConfig& config,
- size_t simulcast_index) {
- auto stream_config = config.GetStreamConfig(simulcast_index);
- if (stream_config.raw_payload) {
+std::optional<VideoCodecType> GetVideoCodecType(const RtpConfig& config) {
+ if (config.raw_payload) {
return std::nullopt;
}
- return PayloadStringToCodecType(stream_config.payload_name);
+ return PayloadStringToCodecType(config.payload_name);
}
bool TransportSeqNumExtensionConfigured(const RtpConfig& config) {
return absl::c_any_of(config.extensions, [](const RtpExtension& ext) {
@@ -423,6 +421,7 @@
crypto_options,
std::move(frame_transformer))),
rtp_config_(rtp_config),
+ codec_type_(GetVideoCodecType(rtp_config)),
transport_(transport),
independent_frame_ids_(
!env.field_trials().IsDisabled(
@@ -471,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;
}
@@ -579,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);
@@ -619,9 +616,7 @@
bool send_result =
rtp_streams_[simulcast_index].sender_video->SendEncodedImage(
- rtp_config_.GetStreamConfig(simulcast_index).payload_type,
- GetVideoCodecType(rtp_config_, simulcast_index), 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);
@@ -759,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.h b/call/rtp_video_sender.h
index 8965cfb..65da26d 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -201,6 +201,7 @@
const std::vector<webrtc_internal_rtp_video_sender::RtpStreamSender>
rtp_streams_;
const RtpConfig rtp_config_;
+ const std::optional<VideoCodecType> codec_type_;
RtpTransportControllerSendInterface* const transport_;
// When using the generic descriptor we want all simulcast streams to share
diff --git a/call/rtp_video_sender_unittest.cc b/call/rtp_video_sender_unittest.cc
index 8fc744f..936f293 100644
--- a/call/rtp_video_sender_unittest.cc
+++ b/call/rtp_video_sender_unittest.cc
@@ -84,7 +84,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;
@@ -134,8 +133,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;
@@ -147,20 +145,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;
}
@@ -173,7 +157,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_,
@@ -183,8 +166,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_}),
@@ -206,22 +188,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,
@@ -236,7 +202,6 @@
suspended_payload_states,
frame_count_observer,
/*frame_transformer=*/nullptr,
- /*payload_types=*/{},
field_trials) {}
RtpVideoSenderTestFixture(
@@ -251,7 +216,6 @@
suspended_payload_states,
/*frame_count_observer=*/nullptr,
/*frame_transformer=*/nullptr,
- /*payload_types=*/{},
field_trials) {}
~RtpVideoSenderTestFixture() { SetSending(false); }
@@ -989,79 +953,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'};
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index c463585..ac0042e 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1428,51 +1428,31 @@
break;
}
- if (send_codec_ &&
- std::any_of(parameters.encodings.begin(), parameters.encodings.end(),
- [](const auto& e) { return e.codec; })) {
- std::vector<VideoCodecSettings> send_codecs;
-
- for (size_t i = 0; i < parameters.encodings.size(); i++) {
- const auto& codec = parameters.encodings[i].codec;
- std::optional<VideoCodecSettings> found_codec;
- if (!codec) {
- found_codec = *send_codec_;
- } else if (i < send_codecs_.size()) {
- const auto& send_codec = send_codecs_[i];
- if (IsSameRtpCodecIgnoringLevel(send_codec.codec, *codec)) {
- found_codec = send_codec;
- }
- }
- if (!found_codec) {
- RTC_DCHECK(codec);
- auto matched_codec =
- absl::c_find_if(negotiated_codecs_, [&](auto negotiated_codec) {
- return IsSameRtpCodecIgnoringLevel(negotiated_codec.codec,
- *codec);
- });
- if (matched_codec == negotiated_codecs_.end()) {
- return webrtc::InvokeSetParametersCallback(
- callback,
- webrtc::RTCError(
- webrtc::RTCErrorType::INVALID_MODIFICATION,
- "Attempted to use an unsupported codec for layer " +
- std::to_string(i)));
- }
- found_codec = *matched_codec;
- }
- RTC_DCHECK(found_codec);
- send_codecs.push_back(*found_codec);
+ // Since we validate that all layers have the same value, we can just check
+ // the first layer.
+ // TODO: https://issues.webrtc.org/362277533 - Support mixed-codec simulcast
+ if (parameters.encodings[0].codec && send_codec_ &&
+ !IsSameRtpCodecIgnoringLevel(send_codec_->codec,
+ *parameters.encodings[0].codec)) {
+ RTC_LOG(LS_VERBOSE) << "Trying to change codec to "
+ << parameters.encodings[0].codec->name;
+ // Ignore level when matching negotiated codecs against the requested
+ // codec.
+ auto matched_codec =
+ absl::c_find_if(negotiated_codecs_, [&](auto negotiated_codec) {
+ return IsSameRtpCodecIgnoringLevel(negotiated_codec.codec,
+ *parameters.encodings[0].codec);
+ });
+ if (matched_codec == negotiated_codecs_.end()) {
+ return webrtc::InvokeSetParametersCallback(
+ callback, webrtc::RTCError(
+ webrtc::RTCErrorType::INVALID_MODIFICATION,
+ "Attempted to use an unsupported codec for layer 0"));
}
- if (send_codecs_ != send_codecs) {
- ChangedSenderParameters params;
- if (!send_codecs.empty()) {
- params.send_codec = send_codecs[0];
- }
- params.send_codecs = send_codecs;
- ApplyChangedParams(params);
- }
+ ChangedSenderParameters params;
+ params.send_codec = *matched_codec;
+ ApplyChangedParams(params);
}
SetPreferredDscp(new_dscp);
