Reland "Store a raw_packetization bool in RtpSenderVideo rather than inferring"
This reverts commit 37ba143ad7d89961d109c91448dd284d16397cbd.
Reason for revert: Reland without the CHECK to allow changing downstream to set RTPSenderVideo::Config::raw_packetization rather than calling SendVideo with an absent codec_type.
Original change's description:
> Revert "Store a raw_packetization bool in RtpSenderVideo rather than inferring"
>
> This reverts commit fc1cbcf05221bf26b6fa94ecb018a5225060ddb4.
>
> Reason for revert: Reverting while we figure out why the check is triggering downstream
>
> Original change's description:
> > Store a raw_packetization bool in RtpSenderVideo rather than inferring
> >
> > Ensure all calls to SendVideo() use a raw packetizer when they should
> > rather than inferring it from the absence of |codec_type| - an issue
> > when the caller doesn't know the correct packetization (eg
> > RTPSenderVideoFrameTransformerDelegate).
> >
> > Bug: b/446768451
> > Change-Id: Ib628d69ac1697de63cc293c5f4c681d6450f72d9
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/411560
> > Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
> > Auto-Submit: Tony Herre <herre@google.com>
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Commit-Queue: Henrik Boström <hbos@webrtc.org>
> > Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
> > Cr-Commit-Position: refs/heads/main@{#45724}
>
> Bug: b/446768451
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Change-Id: I456daecded27f9e52e126b8e5760238704f67300
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/411940
> Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#45731}
Bug: b/446768451
Change-Id: I828c13259c99a6b973c510d20b8646258454d25e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/412000
Commit-Queue: Tony Herre <herre@google.com>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Auto-Submit: Tony Herre <herre@google.com>
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45736}
diff --git a/call/rtp_video_sender.cc b/call/rtp_video_sender.cc
index 25cc813..7814412 100644
--- a/call/rtp_video_sender.cc
+++ b/call/rtp_video_sender.cc
@@ -324,6 +324,8 @@
}
video_config.frame_transformer = frame_transformer;
video_config.task_queue_factory = &env.task_queue_factory();
+ video_config.raw_packetization = rtp_config.raw_payload;
+
auto sender_video = std::make_unique<RTPSenderVideo>(video_config);
rtp_streams.emplace_back(std::move(rtp_rtcp), std::move(sender_video),
std::move(fec_generator));
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.cc b/modules/rtp_rtcp/source/rtp_sender_video.cc
index ee7414b..f5ea03a 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video.cc
@@ -177,6 +177,7 @@
require_frame_encryption_(config.require_frame_encryption),
generic_descriptor_auth_experiment_(
!config.field_trials->IsDisabled("WebRTC-GenericDescriptorAuth")),
+ raw_packetization_(config.raw_packetization),
absolute_capture_time_sender_(config.clock),
frame_transformer_delegate_(
config.frame_transformer
@@ -498,6 +499,9 @@
std::vector<uint32_t> csrcs) {
RTC_CHECK_RUNS_SERIALIZED(&send_checker_);
+ // TODO(b/446768451): Add a check that Codec type can only be absent when
+ // using raw packetization once downstream projects have been updated.
+
if (video_header.frame_type == VideoFrameType::kEmptyFrame)
return true;
@@ -674,7 +678,8 @@
}
std::unique_ptr<RtpPacketizer> packetizer =
- RtpPacketizer::Create(codec_type, payload, limits, video_header);
+ RtpPacketizer::Create(raw_packetization_ ? std::nullopt : codec_type,
+ payload, limits, video_header);
const size_t num_packets = packetizer->NumPackets();
diff --git a/modules/rtp_rtcp/source/rtp_sender_video.h b/modules/rtp_rtcp/source/rtp_sender_video.h
index 8e30021..681675e 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video.h
+++ b/modules/rtp_rtcp/source/rtp_sender_video.h
@@ -90,6 +90,7 @@
const FieldTrialsView* field_trials = nullptr;
scoped_refptr<FrameTransformerInterface> frame_transformer;
TaskQueueFactory* task_queue_factory = nullptr;
+ bool raw_packetization = false;
};
explicit RTPSenderVideo(const Config& config);
@@ -249,6 +250,8 @@
// Set to true if the generic descriptor should be authenticated.
const bool generic_descriptor_auth_experiment_;
+ const bool raw_packetization_;
+
AbsoluteCaptureTimeSender absolute_capture_time_sender_
RTC_GUARDED_BY(send_checker_);
// Tracks updates to the active decode targets and decides when active decode
diff --git a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
index 9479714..b63bafe 100644
--- a/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
+++ b/modules/rtp_rtcp/source/rtp_sender_video_unittest.cc
@@ -166,12 +166,14 @@
public:
TestRtpSenderVideo(Clock* clock,
RTPSender* rtp_sender,
- const FieldTrialsView& field_trials)
+ const FieldTrialsView& field_trials,
+ bool raw_packetization)
: RTPSenderVideo([&] {
Config config;
config.clock = clock;
config.rtp_sender = rtp_sender;
config.field_trials = &field_trials;
+ config.raw_packetization = raw_packetization;
return config;
}()) {}
~TestRtpSenderVideo() override {}
@@ -187,7 +189,7 @@
class RtpSenderVideoTest : public ::testing::Test {
public:
- RtpSenderVideoTest()
+ explicit RtpSenderVideoTest(bool raw_packetization = false)
: fake_clock_(kStartTime),
env_(CreateEnvironment(&fake_clock_)),
retransmission_rate_limiter_(&fake_clock_, 1000),
@@ -201,7 +203,8 @@
rtp_sender_video_(
std::make_unique<TestRtpSenderVideo>(&fake_clock_,
rtp_module_.RtpSender(),
- env_.field_trials())) {
+ env_.field_trials(),
+ raw_packetization)) {
rtp_module_.SetSequenceNumber(kSeqNum);
rtp_module_.SetStartTimestamp(0);
}
@@ -1356,7 +1359,8 @@
TEST_F(RtpSenderVideoTest, AbsoluteCaptureTime) {
rtp_sender_video_ = std::make_unique<TestRtpSenderVideo>(
- &fake_clock_, rtp_module_.RtpSender(), env_.field_trials());
+ &fake_clock_, rtp_module_.RtpSender(), env_.field_trials(),
+ /*raw_packetization=*/false);
constexpr Timestamp kAbsoluteCaptureTimestamp = Timestamp::Millis(12345678);
uint8_t kFrame[kMaxPacketLength];
@@ -1522,7 +1526,12 @@
EXPECT_THAT(sent_payload.subview(1), ElementsAreArray(kDeltaPayload));
}
-TEST_F(RtpSenderVideoTest, SendRawVideo) {
+class RtpSenderVideoRawPacketizationTest : public RtpSenderVideoTest {
+ public:
+ RtpSenderVideoRawPacketizationTest() : RtpSenderVideoTest(true) {}
+};
+
+TEST_F(RtpSenderVideoRawPacketizationTest, SendRawVideo) {
const uint8_t kPayloadTypeRaw = 111;
const uint8_t kPayload[] = {11, 22, 33, 44, 55};
@@ -1538,6 +1547,23 @@
EXPECT_THAT(sent_payload, ElementsAreArray(kPayload));
}
+TEST_F(RtpSenderVideoRawPacketizationTest, SendVideoWithSetCodecTypeStillRaw) {
+ const uint8_t kPayloadTypeRaw = 111;
+ const uint8_t kPayload[] = {11, 22, 33, 44, 55};
+
+ // Send a frame with codectype "generic"
+ RTPVideoHeader video_header;
+ video_header.frame_type = VideoFrameType::kVideoFrameKey;
+ ASSERT_TRUE(rtp_sender_video_->SendVideo(
+ kPayloadTypeRaw, VideoCodecType::kVideoCodecGeneric, 1234,
+ fake_clock_.CurrentTime(), kPayload, sizeof(kPayload), video_header,
+ TimeDelta::PlusInfinity(), {}));
+
+ // Should still be packetized as raw.
+ EXPECT_THAT(transport_.last_sent_packet().payload(),
+ ElementsAreArray(kPayload));
+}
+
class RtpSenderVideoWithFrameTransformerTest : public ::testing::Test {
public:
RtpSenderVideoWithFrameTransformerTest()
