Check (correctly) if packet is a padding packet based on payload size rather than the (incorrect) parsed payload size.
Bug: webrtc:12579
Change-Id: I5f2aff3b0bac8eeb31ac8066aef62b825815a601
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/235207
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35214}
diff --git a/api/video/rtp_video_frame_assembler.cc b/api/video/rtp_video_frame_assembler.cc
index 8f3d04c..2629040 100644
--- a/api/video/rtp_video_frame_assembler.cc
+++ b/api/video/rtp_video_frame_assembler.cc
@@ -92,6 +92,11 @@
RtpVideoFrameAssembler::FrameVector RtpVideoFrameAssembler::Impl::InsertPacket(
const RtpPacketReceived& rtp_packet) {
+ if (rtp_packet.payload_size() == 0) {
+ ClearOldData(rtp_packet.SequenceNumber());
+ return UpdateWithPadding(rtp_packet.SequenceNumber());
+ }
+
absl::optional<VideoRtpDepacketizer::ParsedRtpPayload> parsed_payload =
depacketizer_->Parse(rtp_packet.PayloadBuffer());
@@ -99,11 +104,6 @@
return {};
}
- if (parsed_payload->video_payload.size() == 0) {
- ClearOldData(rtp_packet.SequenceNumber());
- return UpdateWithPadding(rtp_packet.SequenceNumber());
- }
-
if (rtp_packet.HasExtension<RtpDependencyDescriptorExtension>()) {
if (!ParseDependenciesDescriptorExtension(rtp_packet,
parsed_payload->video_header)) {
diff --git a/api/video/rtp_video_frame_assembler_unittests.cc b/api/video/rtp_video_frame_assembler_unittests.cc
index 916a83c..fd70f72 100644
--- a/api/video/rtp_video_frame_assembler_unittests.cc
+++ b/api/video/rtp_video_frame_assembler_unittests.cc
@@ -105,6 +105,13 @@
RtpPacketToSend packet_to_send_;
};
+RtpPacketReceived PaddingPacket(uint16_t seq_num) {
+ RtpPacketReceived padding_packet;
+ padding_packet.SetSequenceNumber(seq_num);
+ padding_packet.SetPadding(224);
+ return padding_packet;
+}
+
void AppendFrames(RtpVideoFrameAssembler::FrameVector from,
RtpVideoFrameAssembler::FrameVector& to) {
to.insert(to.end(), std::make_move_iterator(from.begin()),
@@ -389,26 +396,13 @@
frames);
ASSERT_THAT(frames, SizeIs(1));
-
EXPECT_THAT(frames[0]->Id(), Eq(123));
EXPECT_THAT(Payload(frames[0]), ElementsAreArray(kPayload));
EXPECT_THAT(References(frames[0]), IsEmpty());
- // Padding packets have no bitstream data. An easy way to generate one is to
- // build a normal packet and then simply remove the bitstream portion of the
- // payload.
- RtpPacketReceived padding_packet = PacketBuilder(PayloadFormat::kGeneric)
- .WithPayload(kPayload)
- .WithVideoHeader(video_header)
- .WithSeqNum(124)
- .Build();
- // The payload descriptor is one byte, keep it.
- padding_packet.SetPayloadSize(1);
-
- AppendFrames(assembler.InsertPacket(padding_packet), frames);
+ AppendFrames(assembler.InsertPacket(PaddingPacket(/*seq_num=*/124)), frames);
ASSERT_THAT(frames, SizeIs(2));
-
EXPECT_THAT(frames[1]->Id(), Eq(125));
EXPECT_THAT(Payload(frames[1]), ElementsAreArray(kPayload));
EXPECT_THAT(References(frames[1]), UnorderedElementsAre(123));
@@ -464,17 +458,8 @@
.Build()),
SizeIs(1));
- // Padding packets have no bitstream data. An easy way to generate one is to
- // build a normal packet and then simply remove the bitstream portion of the
- // payload.
- RtpPacketReceived padding_packet = PacketBuilder(PayloadFormat::kGeneric)
- .WithPayload(kPayload)
- .WithVideoHeader(video_header)
- .WithSeqNum(2000)
- .Build();
- // The payload descriptor is one byte, keep it.
- padding_packet.SetPayloadSize(1);
- EXPECT_THAT(assembler.InsertPacket(padding_packet), SizeIs(0));
+ EXPECT_THAT(assembler.InsertPacket(PaddingPacket(/*seq_num=*/2000)),
+ SizeIs(0));
EXPECT_THAT(assembler.InsertPacket(PacketBuilder(PayloadFormat::kGeneric)
.WithPayload(kPayload)
