Allow video frame gaps in packet buffer for H.264

With LTR and SVC etc., H.264 should be able to skip lost frames, and continue to play from the new frames. With DependencyDescriptor, it is allowed to reference the previous frames, even there is a gap in the middle. However, we found there is a special logic for H.264 in packet_buffer.cc, which requires no gap for H.264.

We should allow gaps if the packet has GenericDescriptorInfo (either GenericDescriptor or DependencyDescriptor header extension).

Bug: webrtc:14887
Change-Id: Id66726bab33229bd883f257136ff2e8523fb44c0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/294062
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39370}
This commit is contained in:
Zen Xu 2023-02-21 13:55:08 -08:00 committed by WebRTC LUCI CQ
parent b57053ec21
commit b3c5bdb85a
2 changed files with 37 additions and 8 deletions

View File

@ -256,7 +256,9 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
int64_t frame_timestamp = buffer_[start_index]->timestamp;
// Identify H.264 keyframes by means of SPS, PPS, and IDR.
bool is_h264 = buffer_[start_index]->codec() == kVideoCodecH264;
bool is_generic = buffer_[start_index]->video_header.generic.has_value();
bool is_h264_descriptor =
(buffer_[start_index]->codec() == kVideoCodecH264) && !is_generic;
bool has_h264_sps = false;
bool has_h264_pps = false;
bool has_h264_idr = false;
@ -267,7 +269,7 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
while (true) {
++tested_packets;
if (!is_h264) {
if (!is_h264_descriptor) {
if (buffer_[start_index] == nullptr ||
buffer_[start_index]->is_first_packet_in_frame()) {
full_frame_found = buffer_[start_index] != nullptr;
@ -275,7 +277,7 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
}
}
if (is_h264) {
if (is_h264_descriptor) {
const auto* h264_header = absl::get_if<RTPVideoHeaderH264>(
&buffer_[start_index]->video_header.video_type_header);
if (!h264_header || h264_header->nalus_length >= kMaxNalusPerPacket)
@ -317,7 +319,8 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
// the timestamp of that packet is the same as this one. This may cause
// the PacketBuffer to hand out incomplete frames.
// See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7106
if (is_h264 && (buffer_[start_index] == nullptr ||
if (is_h264_descriptor &&
(buffer_[start_index] == nullptr ||
buffer_[start_index]->timestamp != frame_timestamp)) {
break;
}
@ -325,7 +328,7 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
--start_seq_num;
}
if (is_h264) {
if (is_h264_descriptor) {
// Warn if this is an unsafe frame.
if (has_h264_idr && (!has_h264_sps || !has_h264_pps)) {
RTC_LOG(LS_WARNING)
@ -363,7 +366,7 @@ std::vector<std::unique_ptr<PacketBuffer::Packet>> PacketBuffer::FindFrames(
}
}
if (is_h264 || full_frame_found) {
if (is_h264_descriptor || full_frame_found) {
const uint16_t end_seq_num = seq_num + 1;
// Use uint16_t type to handle sequence number wrap around case.
uint16_t num_packets = end_seq_num - start_seq_num;

View File

@ -405,8 +405,9 @@ class PacketBufferH264Test : public PacketBufferTest {
IsLast last, // is last packet of frame
uint32_t timestamp, // rtp timestamp
rtc::ArrayView<const uint8_t> data = {},
uint32_t width = 0, // width of frame (SPS/IDR)
uint32_t height = 0) { // height of frame (SPS/IDR)
uint32_t width = 0, // width of frame (SPS/IDR)
uint32_t height = 0, // height of frame (SPS/IDR)
bool generic = false) { // has generic descriptor
auto packet = std::make_unique<PacketBuffer::Packet>();
packet->video_header.codec = kVideoCodecH264;
auto& h264_header =
@ -428,6 +429,9 @@ class PacketBufferH264Test : public PacketBufferTest {
packet->video_header.height = height;
packet->video_header.is_first_packet_in_frame = first == kFirst;
packet->video_header.is_last_packet_in_frame = last == kLast;
if (generic) {
packet->video_header.generic.emplace();
}
packet->video_payload.SetData(data.data(), data.size());
return PacketBufferInsertResult(
@ -823,6 +827,28 @@ TEST_F(PacketBufferH264SpsPpsIdrIsKeyframeTest, SpsPpsIdrIsKeyframe) {
ElementsAre(KeyFrame()));
}
class PacketBufferH264FrameGap : public PacketBufferH264Test {
protected:
PacketBufferH264FrameGap() : PacketBufferH264Test(true) {}
};
TEST_F(PacketBufferH264FrameGap, AllowFrameGapForH264WithGeneric) {
auto generic = true;
InsertH264(1, kKeyFrame, kFirst, kLast, 1001, {}, 0, 0, generic);
EXPECT_THAT(InsertH264(3, kDeltaFrame, kFirst, kLast, 1003, {}, 0, 0, generic)
.packets,
SizeIs(1));
}
TEST_F(PacketBufferH264FrameGap, DisallowFrameGapForH264NoGeneric) {
auto generic = false;
InsertH264(1, kKeyFrame, kFirst, kLast, 1001, {}, 0, 0, generic);
EXPECT_THAT(InsertH264(3, kDeltaFrame, kFirst, kLast, 1003, {}, 0, 0, generic)
.packets,
IsEmpty());
}
} // namespace
} // namespace video_coding
} // namespace webrtc