Reduces locking in RtpSenderVideo.
This CL removes some unnecessary locking, since we are already
serialized by the lock in VideoStreamEncoder. A simple RaceChecker is
used to verify this.
We also remove the usage of RegisterPayloadType() and replace it with
a parameter in SendVideo instead. This way we are prepared for removing
the payload type map and lock entirely. Some usage still exists
downstream and needs to be removed before cleaning this up.
Bug: webrtc:10809
Change-Id: Ie90163f15d11c8843f3beaf9a0df0dd2a1fd5ce6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154700
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29372}
diff --git a/call/BUILD.gn b/call/BUILD.gn
index 92dedbd..09ef540 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -168,6 +168,7 @@
"../system_wrappers:field_trial",
"//third_party/abseil-cpp/absl/algorithm:container",
"//third_party/abseil-cpp/absl/container:inlined_vector",
+ "//third_party/abseil-cpp/absl/strings:strings",
"//third_party/abseil-cpp/absl/types:optional",
"//third_party/abseil-cpp/absl/types:variant",
]
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index a88cc2c..c8196b6 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -16,6 +16,7 @@
#include <utility>
#include "absl/algorithm/container.h"
+#include "absl/strings/match.h"
#include "api/array_view.h"
#include "api/transport/field_trial_based_config.h"
#include "call/rtp_transport_controller_send_interface.h"
@@ -205,6 +206,22 @@
// rate here.
return packet_rate.RoundUpTo(Frequency::hertz(1)) * overhead_per_packet;
}
+
+absl::optional<VideoCodecType> GetVideoCodecType(const RtpConfig& config) {
+ absl::optional<VideoCodecType> video_type;
+ if (!config.raw_payload) {
+ if (absl::EqualsIgnoreCase(config.payload_name, "VP8")) {
+ video_type = kVideoCodecVP8;
+ } else if (absl::EqualsIgnoreCase(config.payload_name, "VP9")) {
+ video_type = kVideoCodecVP9;
+ } else if (absl::EqualsIgnoreCase(config.payload_name, "H264")) {
+ video_type = kVideoCodecH264;
+ } else {
+ video_type = kVideoCodecGeneric;
+ }
+ }
+ return video_type;
+}
} // namespace
RtpVideoSender::RtpVideoSender(
@@ -255,6 +272,7 @@
frame_encryptor,
crypto_options)),
rtp_config_(rtp_config),
+ codec_type_(GetVideoCodecType(rtp_config)),
transport_(transport),
transport_overhead_bytes_per_packet_(0),
overhead_bytes_per_packet_(0),
@@ -319,9 +337,6 @@
stream.rtp_rtcp->SetMaxRtpPacketSize(rtp_config_.max_packet_size);
stream.rtp_rtcp->RegisterSendPayloadFrequency(rtp_config_.payload_type,
kVideoPayloadTypeFrequency);
- stream.sender_video->RegisterPayloadType(rtp_config_.payload_type,
- rtp_config_.payload_name,
- rtp_config_.raw_payload);
}
// Currently, both ULPFEC and FlexFEC use the same FEC rate calculation logic,
// so enable that logic if either of those FEC schemes are enabled.
@@ -432,8 +447,8 @@
}
bool send_result = rtp_streams_[stream_index].sender_video->SendVideo(
- encoded_image._frameType, rtp_config_.payload_type, rtp_timestamp,
- encoded_image.capture_time_ms_, encoded_image.data(),
+ encoded_image._frameType, rtp_config_.payload_type, codec_type_,
+ rtp_timestamp, encoded_image.capture_time_ms_, encoded_image.data(),
encoded_image.size(), fragmentation, &rtp_video_header,
expected_retransmission_time_ms);
if (frame_count_observer_) {
diff --git a/call/rtp_video_sender.h b/call/rtp_video_sender.h
index b859483..b5f8a8f 100644
--- a/call/rtp_video_sender.h
+++ b/call/rtp_video_sender.h
@@ -186,6 +186,7 @@
const std::vector<webrtc_internal_rtp_video_sender::RtpStreamSender>
rtp_streams_;
const RtpConfig rtp_config_;
+ const absl::optional<VideoCodecType> codec_type_;
RtpTransportControllerSendInterface* const transport_;
// When using the generic descriptor we want all simulcast streams to share
diff --git a/modules/rtp_rtcp/source/nack_rtx_unittest.cc b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
index d9717a6..aa30005 100644
--- a/modules/rtp_rtcp/source/nack_rtx_unittest.cc
+++ b/modules/rtp_rtcp/source/nack_rtx_unittest.cc
@@ -151,8 +151,6 @@
// single rtp_rtcp module for both send and receive side.
rtp_rtcp_module_->SetRemoteSSRC(kTestSsrc);
- rtp_sender_video_->RegisterPayloadType(kPayloadType, "video",
- /*raw_payload=*/false);
rtp_rtcp_module_->SetRtxSendPayloadType(kRtxPayloadType, kPayloadType);
transport_.SetSendModule(rtp_rtcp_module_.get());
media_receiver_ = transport_.stream_receiver_controller_.CreateReceiver(
@@ -209,9 +207,9 @@
EXPECT_TRUE(rtp_rtcp_module_->OnSendingRtpFrame(timestamp, timestamp / 90,
kPayloadType, false));
EXPECT_TRUE(rtp_sender_video_->SendVideo(
- VideoFrameType::kVideoFrameDelta, kPayloadType, timestamp,
- timestamp / 90, payload_data, payload_data_length, nullptr,
- &video_header, 0));
+ VideoFrameType::kVideoFrameDelta, kPayloadType,
+ VideoCodecType::kVideoCodecGeneric, timestamp, timestamp / 90,
+ payload_data, payload_data_length, nullptr, &video_header, 0));
// Min required delay until retransmit = 5 + RTT ms (RTT = 0).
fake_clock.AdvanceTimeMilliseconds(5);
int length = BuildNackList(nack_list);
@@ -261,9 +259,9 @@
EXPECT_TRUE(rtp_rtcp_module_->OnSendingRtpFrame(timestamp, timestamp / 90,
kPayloadType, false));
EXPECT_TRUE(rtp_sender_video_->SendVideo(
- VideoFrameType::kVideoFrameDelta, kPayloadType, timestamp,
- timestamp / 90, payload_data, payload_data_length, nullptr,
- &video_header, 0));
+ VideoFrameType::kVideoFrameDelta, kPayloadType,
+ VideoCodecType::kVideoCodecGeneric, timestamp, timestamp / 90,
+ payload_data, payload_data_length, nullptr, &video_header, 0));
// Prepare next frame.
timestamp += 3000;
fake_clock.AdvanceTimeMilliseconds(33);
diff --git a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
index 3a9775e..705e53c 100644
--- a/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_rtcp_impl_unittest.cc
@@ -186,8 +186,6 @@
codec_.plType = 100;
codec_.width = 320;
codec_.height = 180;
- sender_video_->RegisterPayloadType(codec_.plType, "VP8",
- /*raw_payload=*/false);
// Receive module.
EXPECT_EQ(0, receiver_.impl_->SetSendingStatus(false));
@@ -224,8 +222,9 @@
const uint8_t payload[100] = {0};
EXPECT_TRUE(module->impl_->OnSendingRtpFrame(0, 0, codec_.plType, true));
EXPECT_TRUE(sender->SendVideo(VideoFrameType::kVideoFrameKey, codec_.plType,
- 0, 0, payload, sizeof(payload), nullptr,
- &rtp_video_header, 0));
+ VideoCodecType::kVideoCodecVP8, 0, 0, payload,
+ sizeof(payload), nullptr, &rtp_video_header,
+ 0));
}
void IncomingRtcpNack(const RtpRtcpModule* module, uint16_t sequence_number) {
diff --git a/modules/rtp_rtcp/source/rtp_sender_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
index 0d5c941..ef13d80 100644
--- a/modules/rtp_rtcp/source/rtp_sender_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_unittest.cc
@@ -591,10 +591,8 @@
false, FieldTrialBasedConfig());
const uint8_t kPayloadType = 127;
- const char payload_name[] = "GENERIC";
-
- rtp_sender_video.RegisterPayloadType(kPayloadType, payload_name,
- /*raw_payload=*/false);
+ const absl::optional<VideoCodecType> kCodecType =
+ VideoCodecType::kVideoCodecGeneric;
const uint32_t kCaptureTimeMsToRtpTimestamp = 90; // 90 kHz clock
RTPVideoHeader video_header;
@@ -607,7 +605,7 @@
int64_t capture_time_ms = fake_clock_.TimeInMilliseconds();
fake_clock_.AdvanceTimeMilliseconds(10);
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayloadType,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType,
capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms,
kPayloadData, sizeof(kPayloadData), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
@@ -619,7 +617,7 @@
.Times(1);
fake_clock_.AdvanceTimeMilliseconds(10);
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayloadType,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType,
capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms,
kPayloadData, sizeof(kPayloadData), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
@@ -632,7 +630,7 @@
.Times(1);
capture_time_ms = fake_clock_.TimeInMilliseconds();
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayloadType,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType,
capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms,
kPayloadData, sizeof(kPayloadData), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
@@ -646,7 +644,7 @@
EXPECT_CALL(send_side_delay_observer_, SendSideDelayUpdated(1, 1, 31, kSsrc))
.Times(1);
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayloadType,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType,
capture_time_ms * kCaptureTimeMsToRtpTimestamp, capture_time_ms,
kPayloadData, sizeof(kPayloadData), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
@@ -1076,21 +1074,19 @@
}
TEST_P(RtpSenderTestWithoutPacer, SendGenericVideo) {
- const char payload_name[] = "GENERIC";
- const uint8_t payload_type = 127;
+ 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());
- rtp_sender_video.RegisterPayloadType(payload_type, payload_name,
- /*raw_payload=*/false);
uint8_t payload[] = {47, 11, 32, 93, 89};
// Send keyframe
RTPVideoHeader video_header;
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
auto sent_payload = transport_.last_sent_packet().payload();
@@ -1105,8 +1101,8 @@
payload[4] = 13;
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameDelta, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameDelta, kPayloadType, kCodecType, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
sent_payload = transport_.last_sent_packet().payload();
@@ -1117,22 +1113,19 @@
}
TEST_P(RtpSenderTestWithoutPacer, SendRawVideo) {
- const char payload_name[] = "VP8";
- const uint8_t payload_type = 111;
+ const uint8_t kPayloadType = 111;
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());
- rtp_sender_video.RegisterPayloadType(payload_type, payload_name,
- /*raw_payload=*/true);
// Send a frame.
RTPVideoHeader video_header;
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameKey, kPayloadType, absl::nullopt, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
auto sent_payload = transport_.last_sent_packet().payload();
@@ -1142,6 +1135,7 @@
TEST_P(RtpSenderTest, SendFlexfecPackets) {
constexpr uint32_t kTimestamp = 1234;
constexpr int kMediaPayloadType = 127;
+ constexpr VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
constexpr int kFlexfecPayloadType = 118;
const std::vector<RtpExtension> kNoRtpExtensions;
const std::vector<RtpExtensionSize> kNoRtpExtensionSizes;
@@ -1168,8 +1162,6 @@
RTPSenderVideo rtp_sender_video(
&fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
nullptr, false, false, false, FieldTrialBasedConfig());
- rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC",
- /*raw_payload=*/false);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
@@ -1200,9 +1192,10 @@
});
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp,
- fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData),
- nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs));
+ VideoFrameType::kVideoFrameKey, kMediaPayloadType, kCodecType,
+ kTimestamp, fake_clock_.TimeInMilliseconds(), kPayloadData,
+ sizeof(kPayloadData), nullptr, &video_header,
+ kDefaultExpectedRetransmissionTimeMs));
ASSERT_TRUE(media_packet != nullptr);
ASSERT_TRUE(fec_packet != nullptr);
@@ -1227,6 +1220,7 @@
constexpr uint32_t kTimestamp = 1234;
const int64_t kCaptureTimeMs = fake_clock_.TimeInMilliseconds();
constexpr int kMediaPayloadType = 127;
+ constexpr VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
constexpr int kFlexfecPayloadType = 118;
const std::vector<RtpExtension> kNoRtpExtensions;
const std::vector<RtpExtensionSize> kNoRtpExtensionSizes;
@@ -1253,8 +1247,6 @@
RTPSenderVideo rtp_sender_video(
&fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
nullptr, false, false, false, FieldTrialBasedConfig());
- rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC",
- /*raw_payload=*/false);
// Need extension to be registered for timing frames to be sent.
ASSERT_EQ(0, rtp_sender_->RegisterRtpHeaderExtension(
@@ -1289,8 +1281,8 @@
.Times(0); // Not called because packet should not be protected.
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp,
- kCaptureTimeMs, kPayloadData, sizeof(kPayloadData), nullptr,
+ VideoFrameType::kVideoFrameKey, kMediaPayloadType, kCodecType,
+ kTimestamp, kCaptureTimeMs, kPayloadData, sizeof(kPayloadData), nullptr,
&video_header, kDefaultExpectedRetransmissionTimeMs));
EXPECT_TRUE(
@@ -1328,9 +1320,9 @@
video_header.video_timing.flags = VideoSendTiming::kInvalid;
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp + 1,
- kCaptureTimeMs + 1, kPayloadData, sizeof(kPayloadData), nullptr,
- &video_header, kDefaultExpectedRetransmissionTimeMs));
+ VideoFrameType::kVideoFrameKey, kMediaPayloadType, kCodecType,
+ kTimestamp + 1, kCaptureTimeMs + 1, kPayloadData, sizeof(kPayloadData),
+ nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs));
ASSERT_TRUE(media_packet2 != nullptr);
ASSERT_TRUE(fec_packet != nullptr);
@@ -1353,6 +1345,7 @@
TEST_P(RtpSenderTestWithoutPacer, SendFlexfecPackets) {
constexpr uint32_t kTimestamp = 1234;
constexpr int kMediaPayloadType = 127;
+ constexpr VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
constexpr int kFlexfecPayloadType = 118;
const std::vector<RtpExtension> kNoRtpExtensions;
const std::vector<RtpExtensionSize> kNoRtpExtensionSizes;
@@ -1377,8 +1370,6 @@
RTPSenderVideo rtp_sender_video(
&fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
nullptr, false, false, false, FieldTrialBasedConfig());
- rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC",
- /*raw_payload=*/false);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
@@ -1392,7 +1383,7 @@
.Times(2);
RTPVideoHeader video_header;
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp,
+ VideoFrameType::kVideoFrameKey, kMediaPayloadType, kCodecType, kTimestamp,
fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData),
nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs));
@@ -1620,6 +1611,7 @@
TEST_P(RtpSenderTest, FecOverheadRate) {
constexpr uint32_t kTimestamp = 1234;
constexpr int kMediaPayloadType = 127;
+ constexpr VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
constexpr int kFlexfecPayloadType = 118;
const std::vector<RtpExtension> kNoRtpExtensions;
const std::vector<RtpExtensionSize> kNoRtpExtensionSizes;
@@ -1645,8 +1637,6 @@
RTPSenderVideo rtp_sender_video(
&fake_clock_, rtp_sender_.get(), &flexfec_sender, &playout_delay_oracle,
nullptr, false, false, false, FieldTrialBasedConfig());
- rtp_sender_video.RegisterPayloadType(kMediaPayloadType, "GENERIC",
- /*raw_payload=*/false);
// Parameters selected to generate a single FEC packet per media packet.
FecProtectionParams params;
params.fec_rate = 15;
@@ -1663,9 +1653,10 @@
RTPVideoHeader video_header;
EXPECT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, kMediaPayloadType, kTimestamp,
- fake_clock_.TimeInMilliseconds(), kPayloadData, sizeof(kPayloadData),
- nullptr, &video_header, kDefaultExpectedRetransmissionTimeMs));
+ VideoFrameType::kVideoFrameKey, kMediaPayloadType, kCodecType,
+ kTimestamp, fake_clock_.TimeInMilliseconds(), kPayloadData,
+ sizeof(kPayloadData), nullptr, &video_header,
+ kDefaultExpectedRetransmissionTimeMs));
fake_clock_.AdvanceTimeMilliseconds(kTimeBetweenPacketsMs);
}
@@ -1718,10 +1709,8 @@
RTPSenderVideo rtp_sender_video(&fake_clock_, rtp_sender_.get(), nullptr,
&playout_delay_oracle, nullptr, false, false,
false, FieldTrialBasedConfig());
- const char payload_name[] = "GENERIC";
- const uint8_t payload_type = 127;
- rtp_sender_video.RegisterPayloadType(payload_type, payload_name,
- /*raw_payload=*/false);
+ const VideoCodecType kCodecType = VideoCodecType::kVideoCodecGeneric;
+ const uint8_t kPayloadType = 127;
// Simulate kNumPackets sent with kPacketInterval ms intervals, with the
// number of packets selected so that we fill (but don't overflow) the one
@@ -1744,8 +1733,8 @@
RTPVideoHeader video_header;
for (uint32_t i = 0; i < kNumPackets; ++i) {
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
fake_clock_.AdvanceTimeMilliseconds(kPacketInterval);
}
@@ -1801,14 +1790,12 @@
const uint8_t kRedPayloadType = 96;
const uint8_t kUlpfecPayloadType = 97;
- const char payload_name[] = "GENERIC";
- const uint8_t payload_type = 127;
+ 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());
- rtp_sender_video.RegisterPayloadType(payload_type, payload_name,
- /*raw_payload=*/false);
uint8_t payload[] = {47, 11, 32, 93, 89};
rtp_sender_->SetStorePacketsStatus(true, 1);
uint32_t ssrc = rtp_sender_->SSRC();
@@ -1818,8 +1805,8 @@
// Send a frame.
RTPVideoHeader video_header;
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameKey, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameKey, kPayloadType, kCodecType, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
StreamDataCounters expected;
expected.transmitted.payload_bytes = 6;
@@ -1861,8 +1848,8 @@
fec_params.max_fec_frames = 1;
rtp_sender_video.SetFecParameters(fec_params, fec_params);
ASSERT_TRUE(rtp_sender_video.SendVideo(
- VideoFrameType::kVideoFrameDelta, payload_type, 1234, 4321, payload,
- sizeof(payload), nullptr, &video_header,
+ VideoFrameType::kVideoFrameDelta, kPayloadType, kCodecType, 1234, 4321,
+ payload, sizeof(payload), nullptr, &video_header,
kDefaultExpectedRetransmissionTimeMs));
expected.transmitted.payload_bytes = 40;
expected.transmitted.header_bytes = 60;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index 37dcdf2..6a6ef97 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -259,12 +259,6 @@
rtc::CritScope cs(&payload_type_crit_);
payload_type_map_[payload_type] = video_type;
}
-
- // Backward compatibility for older receivers without temporal layer logic
- if (absl::EqualsIgnoreCase(payload_name, "H264")) {
- rtc::CritScope cs(&crit_);
- retransmission_settings_ = kRetransmitBaseLayer | kRetransmitHigherLayers;
- }
}
void RTPSenderVideo::AppendAsRedMaybeWithUlpfec(
@@ -462,8 +456,36 @@
const RTPFragmentationHeader* fragmentation,
const RTPVideoHeader* video_header,
absl::optional<int64_t> expected_retransmission_time_ms) {
+ absl::optional<VideoCodecType> codec_type;
+ {
+ rtc::CritScope cs(&payload_type_crit_);
+ const auto it = payload_type_map_.find(payload_type);
+ if (it == payload_type_map_.end()) {
+ RTC_LOG(LS_ERROR) << "Payload type " << static_cast<int>(payload_type)
+ << " not registered.";
+ return false;
+ }
+ codec_type = it->second;
+ }
+ return SendVideo(frame_type, payload_type, codec_type, rtp_timestamp,
+ capture_time_ms, payload_data, payload_size, fragmentation,
+ video_header, expected_retransmission_time_ms);
+}
+
+bool RTPSenderVideo::SendVideo(
+ VideoFrameType frame_type,
+ int8_t payload_type,
+ absl::optional<VideoCodecType> codec_type,
+ uint32_t rtp_timestamp,
+ int64_t capture_time_ms,
+ const uint8_t* payload_data,
+ size_t payload_size,
+ const RTPFragmentationHeader* fragmentation,
+ const RTPVideoHeader* video_header,
+ absl::optional<int64_t> expected_retransmission_time_ms) {
TRACE_EVENT_ASYNC_STEP1("webrtc", "Video", capture_time_ms, "Send", "type",
FrameTypeToString(frame_type));
+ RTC_CHECK_RUNS_SERIALIZED(&send_checker_);
if (frame_type == VideoFrameType::kEmptyFrame)
return true;
@@ -472,51 +494,55 @@
return false;
RTC_CHECK(video_header);
- size_t fec_packet_overhead;
- bool red_enabled;
- int32_t retransmission_settings;
- bool set_video_rotation;
- bool set_color_space = false;
+ int32_t retransmission_settings = retransmission_settings_;
+ if (codec_type == VideoCodecType::kVideoCodecH264) {
+ // Backward compatibility for older receivers without temporal layer logic.
+ retransmission_settings = kRetransmitBaseLayer | kRetransmitHigherLayers;
+ }
+
bool set_frame_marking =
video_header->codec == kVideoCodecH264 &&
video_header->frame_marking.temporal_id != kNoTemporalIdx;
const absl::optional<PlayoutDelay> playout_delay =
playout_delay_oracle_->PlayoutDelayToSend(video_header->playout_delay);
+
+ // According to
+ // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
+ // ts_126114v120700p.pdf Section 7.4.5:
+ // The MTSI client shall add the payload bytes as defined in this clause
+ // onto the last RTP packet in each group of packets which make up a key
+ // frame (I-frame or IDR frame in H.264 (AVC), or an IRAP picture in H.265
+ // (HEVC)). The MTSI client may also add the payload bytes onto the last RTP
+ // packet in each group of packets which make up another type of frame
+ // (e.g. a P-Frame) only if the current value is different from the previous
+ // value sent.
+ // Set rotation when key frame or when changed (to follow standard).
+ // Or when different from 0 (to follow current receiver implementation).
+ bool set_video_rotation = frame_type == VideoFrameType::kVideoFrameKey ||
+ video_header->rotation != last_rotation_ ||
+ video_header->rotation != kVideoRotation_0;
+ last_rotation_ = video_header->rotation;
+
+ // Send color space when changed or if the frame is a key frame. Keep
+ // sending color space information until the first base layer frame to
+ // guarantee that the information is retrieved by the receiver.
+ bool set_color_space;
+ if (video_header->color_space != last_color_space_) {
+ last_color_space_ = video_header->color_space;
+ set_color_space = true;
+ transmit_color_space_next_frame_ = !IsBaseLayer(*video_header);
+ } else {
+ set_color_space = frame_type == VideoFrameType::kVideoFrameKey ||
+ transmit_color_space_next_frame_;
+ transmit_color_space_next_frame_ =
+ transmit_color_space_next_frame_ ? !IsBaseLayer(*video_header) : false;
+ }
+
+ size_t fec_packet_overhead;
+ bool red_enabled;
{
rtc::CritScope cs(&crit_);
- // According to
- // http://www.etsi.org/deliver/etsi_ts/126100_126199/126114/12.07.00_60/
- // ts_126114v120700p.pdf Section 7.4.5:
- // The MTSI client shall add the payload bytes as defined in this clause
- // onto the last RTP packet in each group of packets which make up a key
- // frame (I-frame or IDR frame in H.264 (AVC), or an IRAP picture in H.265
- // (HEVC)). The MTSI client may also add the payload bytes onto the last RTP
- // packet in each group of packets which make up another type of frame
- // (e.g. a P-Frame) only if the current value is different from the previous
- // value sent.
- // Set rotation when key frame or when changed (to follow standard).
- // Or when different from 0 (to follow current receiver implementation).
- set_video_rotation = frame_type == VideoFrameType::kVideoFrameKey ||
- video_header->rotation != last_rotation_ ||
- video_header->rotation != kVideoRotation_0;
- last_rotation_ = video_header->rotation;
-
- // Send color space when changed or if the frame is a key frame. Keep
- // sending color space information until the first base layer frame to
- // guarantee that the information is retrieved by the receiver.
- if (video_header->color_space != last_color_space_) {
- last_color_space_ = video_header->color_space;
- set_color_space = true;
- transmit_color_space_next_frame_ = !IsBaseLayer(*video_header);
- } else {
- set_color_space = frame_type == VideoFrameType::kVideoFrameKey ||
- transmit_color_space_next_frame_;
- transmit_color_space_next_frame_ = transmit_color_space_next_frame_
- ? !IsBaseLayer(*video_header)
- : false;
- }
-
// FEC settings.
const FecProtectionParams& fec_params =
frame_type == VideoFrameType::kVideoFrameKey ? key_fec_params_
@@ -528,7 +554,6 @@
fec_packet_overhead = CalculateFecPacketOverhead();
red_enabled = this->red_enabled();
- retransmission_settings = retransmission_settings_;
}
// Maximum size of packet including rtp headers.
@@ -638,19 +663,8 @@
<< "one is required since require_frame_encryptor is set";
}
- absl::optional<VideoCodecType> type;
- {
- rtc::CritScope cs(&payload_type_crit_);
- const auto it = payload_type_map_.find(payload_type);
- if (it == payload_type_map_.end()) {
- RTC_LOG(LS_ERROR) << "Payload type " << static_cast<int>(payload_type)
- << " not registered.";
- return false;
- }
- type = it->second;
- }
std::unique_ptr<RtpPacketizer> packetizer = RtpPacketizer::Create(
- type, rtc::MakeArrayView(payload_data, payload_size), limits,
+ codec_type, rtc::MakeArrayView(payload_data, payload_size), limits,
*packetize_video_header, frame_type, fragmentation);
const uint8_t temporal_id = GetTemporalId(*video_header);
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index 1ee8e73..9ef9576 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -29,6 +29,7 @@
#include "modules/rtp_rtcp/source/ulpfec_generator.h"
#include "rtc_base/critical_section.h"
#include "rtc_base/one_time_event.h"
+#include "rtc_base/race_checker.h"
#include "rtc_base/rate_statistics.h"
#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/thread_annotations.h"
@@ -67,6 +68,19 @@
virtual ~RTPSenderVideo();
// expected_retransmission_time_ms.has_value() -> retransmission allowed.
+ // Calls to this method is assumed to be externally serialized.
+ bool SendVideo(VideoFrameType frame_type,
+ int8_t payload_type,
+ absl::optional<VideoCodecType> codec_type,
+ uint32_t capture_timestamp,
+ int64_t capture_time_ms,
+ const uint8_t* payload_data,
+ size_t payload_size,
+ const RTPFragmentationHeader* fragmentation,
+ const RTPVideoHeader* video_header,
+ absl::optional<int64_t> expected_retransmission_time_ms);
+
+ // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone.
bool SendVideo(VideoFrameType frame_type,
int8_t payload_type,
uint32_t capture_timestamp,
@@ -77,6 +91,7 @@
const RTPVideoHeader* video_header,
absl::optional<int64_t> expected_retransmission_time_ms);
+ // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone.
void RegisterPayloadType(int8_t payload_type,
absl::string_view payload_name,
bool raw_payload);
@@ -164,23 +179,28 @@
Clock* const clock_;
// Maps payload type to codec type, for packetization.
- // TODO(nisse): Set on construction, to avoid lock.
+ // TODO(bugs.webrtc.org/10809): Remove when downstream usage is gone.
rtc::CriticalSection payload_type_crit_;
std::map<int8_t, absl::optional<VideoCodecType>> payload_type_map_
RTC_GUARDED_BY(payload_type_crit_);
- // Should never be held when calling out of this class.
- rtc::CriticalSection crit_;
+ const int32_t retransmission_settings_;
- int32_t retransmission_settings_ RTC_GUARDED_BY(crit_);
- VideoRotation last_rotation_ RTC_GUARDED_BY(crit_);
- absl::optional<ColorSpace> last_color_space_ RTC_GUARDED_BY(crit_);
- bool transmit_color_space_next_frame_ RTC_GUARDED_BY(crit_);
+ // These members should only be accessed from within SendVideo() to avoid
+ // potential race conditions.
+ rtc::RaceChecker send_checker_;
+ VideoRotation last_rotation_ RTC_GUARDED_BY(send_checker_);
+ absl::optional<ColorSpace> last_color_space_ RTC_GUARDED_BY(send_checker_);
+ bool transmit_color_space_next_frame_ RTC_GUARDED_BY(send_checker_);
+
// Tracks the current request for playout delay limits from application
// and decides whether the current RTP frame should include the playout
// delay extension on header.
PlayoutDelayOracle* const playout_delay_oracle_;
+ // Should never be held when calling out of this class.
+ rtc::CriticalSection crit_;
+
// Maps sent packets' sequence numbers to a tuple consisting of:
// 1. The timestamp, without the randomizing offset mandated by the RFC.
// 2. Whether the packet was the first in its frame.
@@ -218,7 +238,7 @@
// If set to true will require all outgoing frames to pass through an
// initialized frame_encryptor_ before being sent out of the network.
// Otherwise these payloads will be dropped.
- bool require_frame_encryption_;
+ const bool require_frame_encryption_;
// Set to true if the generic descriptor should be authenticated.
const bool generic_descriptor_auth_experiment_;
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index 9e2313c..4bd80d3 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -47,6 +47,7 @@
};
constexpr int kPayload = 100;
+constexpr VideoCodecType kType = VideoCodecType::kVideoCodecGeneric;
constexpr uint32_t kTimestamp = 10;
constexpr uint16_t kSeqNum = 33;
constexpr uint32_t kSsrc = 725242;
@@ -153,9 +154,6 @@
rtp_sender_video_(&fake_clock_, &rtp_sender_, nullptr, field_trials_) {
rtp_sender_.SetSequenceNumber(kSeqNum);
rtp_sender_.SetTimestampOffset(0);
-
- rtp_sender_video_.RegisterPayloadType(kPayload, "generic",
- /*raw_payload=*/false);
}
void PopulateGenericFrameDescriptor(int version);
@@ -180,7 +178,7 @@
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_0;
- rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameKey, kPayload,
+ rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameKey, kPayload, kType,
kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs);
@@ -206,7 +204,7 @@
hdr.video_timing.encode_finish_delta_ms = kEncodeFinishDeltaMs;
fake_clock_.AdvanceTimeMilliseconds(kPacketizationTimeMs);
- rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameKey, kPayload,
+ rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameKey, kPayload, kType,
kTimestamp, kCaptureTimestamp, kFrame,
sizeof(kFrame), nullptr, &hdr,
kDefaultExpectedRetransmissionTimeMs);
@@ -226,13 +224,14 @@
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_90;
EXPECT_TRUE(rtp_sender_video_.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
+ VideoFrameType::kVideoFrameKey, kPayload, kType, kTimestamp, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
hdr.rotation = kVideoRotation_0;
EXPECT_TRUE(rtp_sender_video_.SendVideo(
- VideoFrameType::kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame,
- sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
+ VideoFrameType::kVideoFrameDelta, kPayload, kType, kTimestamp + 1, 0,
+ kFrame, sizeof(kFrame), nullptr, &hdr,
+ kDefaultExpectedRetransmissionTimeMs));
VideoRotation rotation;
EXPECT_TRUE(
@@ -248,12 +247,13 @@
RTPVideoHeader hdr;
hdr.rotation = kVideoRotation_90;
EXPECT_TRUE(rtp_sender_video_.SendVideo(
- VideoFrameType::kVideoFrameKey, kPayload, kTimestamp, 0, kFrame,
+ VideoFrameType::kVideoFrameKey, kPayload, kType, kTimestamp, 0, kFrame,
sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
EXPECT_TRUE(rtp_sender_video_.SendVideo(
- VideoFrameType::kVideoFrameDelta, kPayload, kTimestamp + 1, 0, kFrame,
- sizeof(kFrame), nullptr, &hdr, kDefaultExpectedRetransmissionTimeMs));
+ VideoFrameType::kVideoFrameDelta, kPayload, kType, kTimestamp + 1, 0,
+ kFrame, sizeof(kFrame), nullptr, &hdr,
+ kDefaultExpectedRetransmissionTimeMs));
VideoRotation rotation;
EXPECT_TRUE(
@@ -278,7 +278,7 @@
hdr.frame_marking.temporal_id = kNoTemporalIdx;
hdr.frame_marking.tl0_pic_idx = 99;
hdr.frame_marking.base_layer_sync = true;
- rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload,
+ rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload, kType,
kTimestamp, 0, kFrame, sizeof(kFrame), &frag,
&hdr, kDefaultExpectedRetransmissionTimeMs);
@@ -287,7 +287,7 @@
transport_.last_sent_packet().GetExtension<FrameMarkingExtension>(&fm));
hdr.frame_marking.temporal_id = 0;
- rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload,
+ rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload, kType,
kTimestamp + 1, 0, kFrame, sizeof(kFrame), &frag,
&hdr, kDefaultExpectedRetransmissionTimeMs);
@@ -529,7 +529,7 @@
generic.higher_spatial_layers.push_back(4);
generic.dependencies.push_back(kFrameId - 1);
generic.dependencies.push_back(kFrameId - 500);
- rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload,
+ rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload, kType,
kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
&hdr, kDefaultExpectedRetransmissionTimeMs);
@@ -583,10 +583,10 @@
vp8.keyIdx = 2;
RTPVideoHeader::GenericDescriptorInfo& generic = hdr.generic.emplace();
generic.frame_id = kFrameId;
- rtp_sender_video_.RegisterPayloadType(kPayload, "vp8", /*raw_payload=*/false);
rtp_sender_video_.SendVideo(VideoFrameType::kVideoFrameDelta, kPayload,
- kTimestamp, 0, kFrame, sizeof(kFrame), nullptr,
- &hdr, kDefaultExpectedRetransmissionTimeMs);
+ VideoCodecType::kVideoCodecVP8, kTimestamp, 0,
+ kFrame, sizeof(kFrame), nullptr, &hdr,
+ kDefaultExpectedRetransmissionTimeMs);
ASSERT_EQ(transport_.packets_sent(), 1);
// Expect only minimal 1-byte vp8 descriptor was generated.
