Adds RTPSenderVideo::Config struct with red/ulpfec config
This CL moves the various parameters in the the RTPSenderVideo ctor into
a struct, and adds the red/ulpfec payload types to it.
Once the downstream usage of SetUlpfecConfig() is gone, we can make
those members const and avoid locking in SendVideo().
Bug: webrtc:10809
Change-Id: I9a96ab5b2a4eb2997ebf4a3a3e3cd2eb5715fd79
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/155365
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29384}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index c8196b6..ca6132f 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -55,6 +55,68 @@
using webrtc_internal_rtp_video_sender::RtpStreamSender;
+bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
+ const VideoCodecType codecType = PayloadStringToCodecType(payload_name);
+ if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) {
+ return true;
+ }
+ if (codecType == kVideoCodecGeneric &&
+ field_trial::IsEnabled("WebRTC-GenericPictureId")) {
+ return true;
+ }
+ return false;
+}
+
+bool ShouldDisableRedAndUlpfec(bool flexfec_enabled,
+ const RtpConfig& rtp_config) {
+ // Consistency of NACK and RED+ULPFEC parameters is checked in this function.
+ const bool nack_enabled = rtp_config.nack.rtp_history_ms > 0;
+
+ // Shorthands.
+ auto IsRedEnabled = [&]() { return rtp_config.ulpfec.red_payload_type >= 0; };
+ auto IsUlpfecEnabled = [&]() {
+ return rtp_config.ulpfec.ulpfec_payload_type >= 0;
+ };
+
+ bool should_disable_red_and_ulpfec = false;
+
+ if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) {
+ RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled.";
+ should_disable_red_and_ulpfec = true;
+ }
+
+ // If enabled, FlexFEC takes priority over RED+ULPFEC.
+ if (flexfec_enabled) {
+ if (IsUlpfecEnabled()) {
+ RTC_LOG(LS_INFO)
+ << "Both FlexFEC and ULPFEC are configured. Disabling ULPFEC.";
+ }
+ should_disable_red_and_ulpfec = true;
+ }
+
+ // Payload types without picture ID cannot determine that a stream is complete
+ // without retransmitting FEC, so using ULPFEC + NACK for H.264 (for instance)
+ // is a waste of bandwidth since FEC packets still have to be transmitted.
+ // Note that this is not the case with FlexFEC.
+ if (nack_enabled && IsUlpfecEnabled() &&
+ !PayloadTypeSupportsSkippingFecPackets(rtp_config.payload_name)) {
+ RTC_LOG(LS_WARNING)
+ << "Transmitting payload type without picture ID using "
+ "NACK+ULPFEC is a waste of bandwidth since ULPFEC packets "
+ "also have to be retransmitted. Disabling ULPFEC.";
+ should_disable_red_and_ulpfec = true;
+ }
+
+ // Verify payload types.
+ if (IsUlpfecEnabled() ^ IsRedEnabled()) {
+ RTC_LOG(LS_WARNING)
+ << "Only RED or only ULPFEC enabled, but not both. Disabling both.";
+ should_disable_red_and_ulpfec = true;
+ }
+
+ return should_disable_red_and_ulpfec;
+}
+
std::vector<RtpStreamSender> CreateRtpStreamSenders(
Clock* clock,
const RtpConfig& rtp_config,
@@ -129,31 +191,38 @@
rtp_rtcp->SetSendingStatus(false);
rtp_rtcp->SetSendingMediaStatus(false);
rtp_rtcp->SetRTCPStatus(RtcpMode::kCompound);
+ // Set NACK.
+ rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
- auto sender_video = std::make_unique<RTPSenderVideo>(
- configuration.clock, rtp_rtcp->RtpSender(),
- configuration.flexfec_sender, playout_delay_oracle.get(),
- frame_encryptor, crypto_options.sframe.require_frame_encryption,
- rtp_config.lntf.enabled, /*enable_retransmit_all_layers*/ false,
- FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trial_config;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = configuration.clock;
+ video_config.rtp_sender = rtp_rtcp->RtpSender();
+ video_config.flexfec_sender = configuration.flexfec_sender;
+ video_config.playout_delay_oracle = playout_delay_oracle.get();
+ video_config.frame_encryptor = frame_encryptor;
+ video_config.require_frame_encryption =
+ crypto_options.sframe.require_frame_encryption;
+ video_config.need_rtp_packet_infos = rtp_config.lntf.enabled;
+ video_config.enable_retransmit_all_layers = false;
+ video_config.field_trials = &field_trial_config;
+ const bool should_disable_red_and_ulpfec =
+ ShouldDisableRedAndUlpfec(enable_flexfec, rtp_config);
+ if (rtp_config.ulpfec.red_payload_type != -1 &&
+ !should_disable_red_and_ulpfec) {
+ video_config.red_payload_type = rtp_config.ulpfec.red_payload_type;
+ }
+ if (rtp_config.ulpfec.ulpfec_payload_type != -1 &&
+ !should_disable_red_and_ulpfec) {
+ video_config.ulpfec_payload_type = rtp_config.ulpfec.ulpfec_payload_type;
+ }
+ auto sender_video = std::make_unique<RTPSenderVideo>(video_config);
rtp_streams.emplace_back(std::move(playout_delay_oracle),
std::move(rtp_rtcp), std::move(sender_video));
}
return rtp_streams;
}
-bool PayloadTypeSupportsSkippingFecPackets(const std::string& payload_name) {
- const VideoCodecType codecType = PayloadStringToCodecType(payload_name);
- if (codecType == kVideoCodecVP8 || codecType == kVideoCodecVP9) {
- return true;
- }
- if (codecType == kVideoCodecGeneric &&
- field_trial::IsEnabled("WebRTC-GenericPictureId")) {
- return true;
- }
- return false;
-}
-
// TODO(brandtr): Update this function when we support multistream protection.
std::unique_ptr<FlexfecSender> MaybeCreateFlexfecSender(
Clock* clock,
@@ -316,7 +385,6 @@
}
}
- ConfigureProtection();
ConfigureSsrcs();
ConfigureRids();
@@ -497,65 +565,6 @@
}
}
-void RtpVideoSender::ConfigureProtection() {
- // Consistency of FlexFEC parameters is checked in MaybeCreateFlexfecSender.
- const bool flexfec_enabled = (flexfec_sender_ != nullptr);
-
- // Consistency of NACK and RED+ULPFEC parameters is checked in this function.
- const bool nack_enabled = rtp_config_.nack.rtp_history_ms > 0;
- int red_payload_type = rtp_config_.ulpfec.red_payload_type;
- int ulpfec_payload_type = rtp_config_.ulpfec.ulpfec_payload_type;
-
- // Shorthands.
- auto IsRedEnabled = [&]() { return red_payload_type >= 0; };
- auto IsUlpfecEnabled = [&]() { return ulpfec_payload_type >= 0; };
- auto DisableRedAndUlpfec = [&]() {
- red_payload_type = -1;
- ulpfec_payload_type = -1;
- };
-
- if (webrtc::field_trial::IsEnabled("WebRTC-DisableUlpFecExperiment")) {
- RTC_LOG(LS_INFO) << "Experiment to disable sending ULPFEC is enabled.";
- DisableRedAndUlpfec();
- }
-
- // If enabled, FlexFEC takes priority over RED+ULPFEC.
- if (flexfec_enabled) {
- if (IsUlpfecEnabled()) {
- RTC_LOG(LS_INFO)
- << "Both FlexFEC and ULPFEC are configured. Disabling ULPFEC.";
- }
- DisableRedAndUlpfec();
- }
-
- // Payload types without picture ID cannot determine that a stream is complete
- // without retransmitting FEC, so using ULPFEC + NACK for H.264 (for instance)
- // is a waste of bandwidth since FEC packets still have to be transmitted.
- // Note that this is not the case with FlexFEC.
- if (nack_enabled && IsUlpfecEnabled() &&
- !PayloadTypeSupportsSkippingFecPackets(rtp_config_.payload_name)) {
- RTC_LOG(LS_WARNING)
- << "Transmitting payload type without picture ID using "
- "NACK+ULPFEC is a waste of bandwidth since ULPFEC packets "
- "also have to be retransmitted. Disabling ULPFEC.";
- DisableRedAndUlpfec();
- }
-
- // Verify payload types.
- if (IsUlpfecEnabled() ^ IsRedEnabled()) {
- RTC_LOG(LS_WARNING)
- << "Only RED or only ULPFEC enabled, but not both. Disabling both.";
- DisableRedAndUlpfec();
- }
-
- for (const RtpStreamSender& stream : rtp_streams_) {
- // Set NACK.
- stream.rtp_rtcp->SetStorePacketsStatus(true, kMinSendSidePacketHistorySize);
- // Set RED/ULPFEC information.
- stream.sender_video->SetUlpfecConfig(red_payload_type, ulpfec_payload_type);
- }
-}
-
bool RtpVideoSender::FecEnabled() const {
const bool flexfec_enabled = (flexfec_sender_ != nullptr);
const bool ulpfec_enabled =
diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
index aa30005..62d5f98 100644
--- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc
+++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
@@ -136,10 +136,13 @@
configuration.retransmission_rate_limiter = &retransmission_rate_limiter_;
configuration.local_media_ssrc = kTestSsrc;
rtp_rtcp_module_ = RtpRtcp::Create(configuration);
- rtp_sender_video_ = std::make_unique<RTPSenderVideo>(
- &fake_clock, rtp_rtcp_module_->RtpSender(), nullptr,
- &playout_delay_oracle_, nullptr, false, false, false,
- FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock;
+ video_config.rtp_sender = rtp_rtcp_module_->RtpSender();
+ video_config.playout_delay_oracle = &playout_delay_oracle_;
+ video_config.field_trials = &field_trials;
+ rtp_sender_video_ = std::make_unique<RTPSenderVideo>(video_config);
rtp_rtcp_module_->SetRTCPStatus(RtcpMode::kCompound);
rtp_rtcp_module_->SetStorePacketsStatus(true, 600);
EXPECT_EQ(0, rtp_rtcp_module_->SetSendingStatus(true));
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 705e53c..34944bc 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -178,9 +178,13 @@
sender_.impl_->SetSequenceNumber(kSequenceNumber);
sender_.impl_->SetStorePacketsStatus(true, 100);
- sender_video_ = std::make_unique<RTPSenderVideo>(
- &clock_, sender_.impl_->RtpSender(), nullptr, &playout_delay_oracle_,
- nullptr, false, false, false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &clock_;
+ video_config.rtp_sender = sender_.impl_->RtpSender();
+ video_config.playout_delay_oracle = &playout_delay_oracle_;
+ video_config.field_trials = &field_trials;
+ sender_video_ = std::make_unique<RTPSenderVideo>(video_config);
memset(&codec_, 0, sizeof(VideoCodec));
codec_.plType = 100;
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 1138591..90b92b3 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -588,9 +588,13 @@
rtp_sender_ = std::make_unique<RTPSender>(config);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
- &playout_delay_oracle, nullptr, false, false,
- false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
const uint8_t kPayloadType = 127;
const absl::optional<VideoCodecType> kCodecType =
@@ -1078,9 +1082,13 @@
const uint8_t kPayloadType = 127;
const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
- &playout_delay_oracle, nullptr, false, false,
- false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
uint8_t payload[] = {47, 11, 32, 93, 89};
// Send keyframe
@@ -1118,9 +1126,13 @@
const uint8_t payload[] = {11, 22, 33, 44, 55};
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
- &playout_delay_oracle, nullptr, false, false,
- false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
// Send a frame.
RTPVideoHeader video_header;
@@ -1160,9 +1172,14 @@
rtp_sender_->SetStorePacketsStatus(true, 10);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(
- &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
- nullptr, false, false, false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.flexfec_sender = &flexfec_sender;
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
@@ -1248,9 +1265,14 @@
rtp_sender_->SetStorePacketsStatus(true, 10);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(
- &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
- nullptr, false, false, false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.flexfec_sender = &flexfec_sender;
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
// Need extension to be registered for timing frames to be sent.
ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
@@ -1376,9 +1398,14 @@
rtp_sender_->SetSequenceNumber(kSeqNum);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(
- &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
- nullptr, false, false, false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.flexfec_sender = &flexfec_sender;
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
@@ -1643,9 +1670,14 @@
rtp_sender_->SetSequenceNumber(kSeqNum);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(
- &fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
- nullptr, false, false, false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.flexfec_sender = &flexfec_sender;
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
params.fec_rate = 15;
@@ -1715,9 +1747,13 @@
rtp_sender_ = std::make_unique<RTPSender>(config);
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
- &playout_delay_oracle, nullptr, false, false,
- false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
const uint8_t kPayloadType = 127;
@@ -1766,45 +1802,50 @@
rtp_sender_.reset();
}
+class StreamDataTestCallback : public StreamDataCountersCallback {
+ public:
+ StreamDataTestCallback()
+ : StreamDataCountersCallback(), ssrc_(0), counters_() {}
+ ~StreamDataTestCallback() override = default;
+
+ void DataCountersUpdated(const StreamDataCounters& counters,
+ uint32_t ssrc) override {
+ ssrc_ = ssrc;
+ counters_ = counters;
+ }
+
+ uint32_t ssrc_;
+ StreamDataCounters counters_;
+
+ void MatchPacketCounter(const RtpPacketCounter& expected,
+ const RtpPacketCounter& actual) {
+ EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
+ EXPECT_EQ(expected.header_bytes, actual.header_bytes);
+ EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
+ EXPECT_EQ(expected.packets, actual.packets);
+ }
+
+ void Matches(uint32_t ssrc, const StreamDataCounters& counters) {
+ EXPECT_EQ(ssrc, ssrc_);
+ MatchPacketCounter(counters.transmitted, counters_.transmitted);
+ MatchPacketCounter(counters.retransmitted, counters_.retransmitted);
+ EXPECT_EQ(counters.fec.packets, counters_.fec.packets);
+ }
+};
+
TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacks) {
- class TestCallback : public StreamDataCountersCallback {
- public:
- TestCallback() : StreamDataCountersCallback(), ssrc_(0), counters_() {}
- ~TestCallback() override = default;
+ StreamDataTestCallback callback;
- void DataCountersUpdated(const StreamDataCounters& counters,
- uint32_t ssrc) override {
- ssrc_ = ssrc;
- counters_ = counters;
- }
-
- uint32_t ssrc_;
- StreamDataCounters counters_;
-
- void MatchPacketCounter(const RtpPacketCounter& expected,
- const RtpPacketCounter& actual) {
- EXPECT_EQ(expected.payload_bytes, actual.payload_bytes);
- EXPECT_EQ(expected.header_bytes, actual.header_bytes);
- EXPECT_EQ(expected.padding_bytes, actual.padding_bytes);
- EXPECT_EQ(expected.packets, actual.packets);
- }
-
- void Matches(uint32_t ssrc, const StreamDataCounters& counters) {
- EXPECT_EQ(ssrc, ssrc_);
- MatchPacketCounter(counters.transmitted, counters_.transmitted);
- MatchPacketCounter(counters.retransmitted, counters_.retransmitted);
- EXPECT_EQ(counters.fec.packets, counters_.fec.packets);
- }
- } callback;
-
- const uint8_t kRedPayloadType = 96;
- const uint8_t kUlpfecPayloadType = 97;
const uint8_t kPayloadType = 127;
const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
PlayoutDelayOracle playout_delay_oracle;
- RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
- &playout_delay_oracle, nullptr, false, false,
- false, FieldTrialBasedConfig());
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ RTPSenderVideo rtp_sender_video(video_config);
uint8_t payload[] = {47, 11, 32, 93, 89};
rtp_sender_->SetStorePacketsStatus(true, 1);
uint32_t ssrc = rtp_sender_->SSRC();
@@ -1849,8 +1890,36 @@
expected.transmitted.packets = 3;
callback.Matches(ssrc, expected);
+ rtp_sender_->RegisterRtpStatisticsCallback(nullptr);
+}
+
+TEST_P(RtpSenderTestWithoutPacer, StreamDataCountersCallbacksUlpfec) {
+ StreamDataTestCallback callback;
+
+ const uint8_t kRedPayloadType = 96;
+ const uint8_t kUlpfecPayloadType = 97;
+ const uint8_t kPayloadType = 127;
+ const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
+ PlayoutDelayOracle playout_delay_oracle;
+ FieldTrialBasedConfig field_trials;
+ RTPSenderVideo::Config video_config;
+ video_config.clock = &fake_clock_;
+ video_config.rtp_sender = rtp_sender_.get();
+ video_config.playout_delay_oracle = &playout_delay_oracle;
+ video_config.field_trials = &field_trials;
+ video_config.red_payload_type = kRedPayloadType;
+ video_config.ulpfec_payload_type = kUlpfecPayloadType;
+ RTPSenderVideo rtp_sender_video(video_config);
+ uint8_t payload[] = {47, 11, 32, 93, 89};
+ rtp_sender_->SetStorePacketsStatus(true, 1);
+ uint32_t ssrc = rtp_sender_->SSRC();
+
+ rtp_sender_->RegisterRtpStatisticsCallback(&callback);
+
+ RTPVideoHeader video_header;
+ StreamDataCounters expected;
+
// Send ULPFEC.
- rtp_sender_video.SetUlpfecConfig(kRedPayloadType, kUlpfecPayloadType);
FecProtectionParams fec_params;
fec_params.fec_mask_type = kFecMaskRandom;
fec_params.fec_rate = 1;
@@ -1860,9 +1929,9 @@
VideoFrameType::kVideoFrameDelta, kPayloadType, kCodecType, 1234, 4321,
payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
- expected.transmitted.payload_bytes = 40;
- expected.transmitted.header_bytes = 60;
- expected.transmitted.packets = 5;
+ expected.transmitted.payload_bytes = 28;
+ expected.transmitted.header_bytes = 24;
+ expected.transmitted.packets = 2;
expected.fec.packets = 1;
callback.Matches(ssrc, expected);
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 6a6ef97..fc40a97 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -205,34 +205,50 @@
bool need_rtp_packet_infos,
bool enable_retransmit_all_layers,
const WebRtcKeyValueConfig& field_trials)
- : rtp_sender_(rtp_sender),
- clock_(clock),
+ : RTPSenderVideo([&] {
+ Config config;
+ config.clock = clock;
+ config.rtp_sender = rtp_sender;
+ config.flexfec_sender = flexfec_sender;
+ config.playout_delay_oracle = playout_delay_oracle;
+ config.frame_encryptor = frame_encryptor;
+ config.require_frame_encryption = require_frame_encryption;
+ config.need_rtp_packet_infos = need_rtp_packet_infos;
+ config.enable_retransmit_all_layers = enable_retransmit_all_layers;
+ config.field_trials = &field_trials;
+ return config;
+ }()) {}
+
+RTPSenderVideo::RTPSenderVideo(const Config& config)
+ : rtp_sender_(config.rtp_sender),
+ clock_(config.clock),
retransmission_settings_(
- enable_retransmit_all_layers
+ config.enable_retransmit_all_layers
? kRetransmitAllLayers
: (kRetransmitBaseLayer | kConditionallyRetransmitHigherLayers)),
last_rotation_(kVideoRotation_0),
transmit_color_space_next_frame_(false),
- playout_delay_oracle_(playout_delay_oracle),
- rtp_sequence_number_map_(need_rtp_packet_infos
+ playout_delay_oracle_(config.playout_delay_oracle),
+ rtp_sequence_number_map_(config.need_rtp_packet_infos
? std::make_unique<RtpSequenceNumberMap>(
kRtpSequenceNumberMapMaxEntries)
: nullptr),
- red_payload_type_(-1),
- ulpfec_payload_type_(-1),
- flexfec_sender_(flexfec_sender),
+ red_payload_type_(config.red_payload_type),
+ ulpfec_payload_type_(config.ulpfec_payload_type),
+ flexfec_sender_(config.flexfec_sender),
delta_fec_params_{0, 1, kFecMaskRandom},
key_fec_params_{0, 1, kFecMaskRandom},
fec_bitrate_(1000, RateStatistics::kBpsScale),
video_bitrate_(1000, RateStatistics::kBpsScale),
packetization_overhead_bitrate_(1000, RateStatistics::kBpsScale),
- frame_encryptor_(frame_encryptor),
- require_frame_encryption_(require_frame_encryption),
+ frame_encryptor_(config.frame_encryptor),
+ require_frame_encryption_(config.require_frame_encryption),
generic_descriptor_auth_experiment_(
- field_trials.Lookup("WebRTC-GenericDescriptorAuth").find("Enabled") ==
- 0),
+ config.field_trials->Lookup("WebRTC-GenericDescriptorAuth")
+ .find("Enabled") == 0),
exclude_transport_sequence_number_from_fec_experiment_(
- field_trials.Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial)
+ config.field_trials
+ ->Lookup(kExcludeTransportSequenceNumberFromFecFieldTrial)
.find("Enabled") == 0) {
RTC_DCHECK(playout_delay_oracle_);
}
@@ -273,7 +289,7 @@
{
// Only protect while creating RED and FEC packets, not when sending.
rtc::CritScope cs(&crit_);
- red_packet->SetPayloadType(red_payload_type_);
+ red_packet->SetPayloadType(*red_payload_type_);
if (ulpfec_enabled()) {
if (protect_media_packet) {
if (exclude_transport_sequence_number_from_fec_experiment_) {
@@ -302,7 +318,8 @@
uint16_t first_fec_sequence_number =
rtp_sender_->AllocateSequenceNumber(num_fec_packets);
fec_packets = ulpfec_generator_.GetUlpfecPacketsAsRed(
- red_payload_type_, ulpfec_payload_type_, first_fec_sequence_number);
+ *red_payload_type_, *ulpfec_payload_type_,
+ first_fec_sequence_number);
RTC_DCHECK_EQ(num_fec_packets, fec_packets.size());
}
}
@@ -399,8 +416,12 @@
RTC_DCHECK_LE(ulpfec_payload_type, 127);
rtc::CritScope cs(&crit_);
- red_payload_type_ = red_payload_type;
- ulpfec_payload_type_ = ulpfec_payload_type;
+ if (red_payload_type != -1) {
+ red_payload_type_ = red_payload_type;
+ }
+ if (ulpfec_payload_type != -1) {
+ ulpfec_payload_type_ = ulpfec_payload_type;
+ }
// Must not enable ULPFEC without RED.
RTC_DCHECK(!(red_enabled() ^ ulpfec_enabled()));
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index 9ef9576..1b956fd 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -56,6 +56,29 @@
public:
static constexpr int64_t kTLRateWindowSizeMs = 2500;
+ struct Config {
+ Config() = default;
+ Config(const Config&) = delete;
+ Config(Config&&) = default;
+
+ // All members of this struct, with the exception of |field_trials|, are
+ // expected to outlive the RTPSenderVideo object they are passed to.
+ Clock* clock = nullptr;
+ RTPSender* rtp_sender = nullptr;
+ FlexfecSender* flexfec_sender = nullptr;
+ PlayoutDelayOracle* playout_delay_oracle = nullptr;
+ FrameEncryptorInterface* frame_encryptor = nullptr;
+ bool require_frame_encryption = false;
+ bool need_rtp_packet_infos = false;
+ bool enable_retransmit_all_layers = false;
+ absl::optional<int> red_payload_type;
+ absl::optional<int> ulpfec_payload_type;
+ const WebRtcKeyValueConfig* field_trials = nullptr;
+ };
+
+ explicit RTPSenderVideo(const Config& config);
+
+ // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone.
RTPSenderVideo(Clock* clock,
RTPSender* rtpSender,
FlexfecSender* flexfec_sender,
@@ -100,6 +123,7 @@
// corresponding feature is turned off. Note that we DO NOT support enabling
// ULPFEC without enabling RED, and RED is only ever used when ULPFEC is
// enabled.
+ // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone.
void SetUlpfecConfig(int red_payload_type, int ulpfec_payload_type);
// FlexFEC/ULPFEC.
@@ -162,11 +186,11 @@
size_t unpacketized_payload_size);
bool red_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) {
- return red_payload_type_ >= 0;
+ return red_payload_type_.has_value();
}
bool ulpfec_enabled() const RTC_EXCLUSIVE_LOCKS_REQUIRED(crit_) {
- return ulpfec_payload_type_ >= 0;
+ return ulpfec_payload_type_.has_value();
}
bool flexfec_enabled() const { return flexfec_sender_ != nullptr; }
@@ -209,8 +233,8 @@
RTC_PT_GUARDED_BY(crit_);
// RED/ULPFEC.
- int red_payload_type_ RTC_GUARDED_BY(crit_);
- int ulpfec_payload_type_ RTC_GUARDED_BY(crit_);
+ absl::optional<int> red_payload_type_ RTC_GUARDED_BY(crit_);
+ absl::optional<int> ulpfec_payload_type_ RTC_GUARDED_BY(crit_);
UlpfecGenerator ulpfec_generator_ RTC_GUARDED_BY(crit_);
// FlexFEC.
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index 4bd80d3..856d239 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -99,15 +99,15 @@
RTPSender* rtp_sender,
FlexfecSender* flexfec_sender,
const WebRtcKeyValueConfig& field_trials)
- : RTPSenderVideo(clock,
- rtp_sender,
- flexfec_sender,
- &playout_delay_oracle_,
- nullptr,
- false,
- false,
- false,
- field_trials) {}
+ : RTPSenderVideo([&] {
+ Config config;
+ config.clock = clock;
+ config.rtp_sender = rtp_sender;
+ config.flexfec_sender = flexfec_sender;
+ config.playout_delay_oracle = &playout_delay_oracle_;
+ config.field_trials = &field_trials;
+ return config;
+ }()) {}
~TestRtpSenderVideo() override {}
bool AllowRetransmission(const RTPVideoHeader& header,
