Request a new key frame if packet buffer is cleared
Bug: webrtc:10843 Change-Id: I1eab0891f3e68b7d504dc637790604a25c243856 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/147721 Commit-Queue: Johannes Kron <kron@webrtc.org> Reviewed-by: Erik Språng <sprang@webrtc.org> Cr-Commit-Position: refs/heads/master@{#28735}
This commit is contained in:

committed by
Commit Bot

parent
77d3efc509
commit
bd3f30535c
@ -82,11 +82,11 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) {
|
||||
first_packet_received_ = true;
|
||||
} else if (AheadOf(first_seq_num_, seq_num)) {
|
||||
// If we have explicitly cleared past this packet then it's old,
|
||||
// don't insert it.
|
||||
// don't insert it, just silently ignore it.
|
||||
if (is_cleared_to_first_seq_num_) {
|
||||
delete[] packet->dataPtr;
|
||||
packet->dataPtr = nullptr;
|
||||
return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
first_seq_num_ = seq_num;
|
||||
@ -105,8 +105,12 @@ bool PacketBuffer::InsertPacket(VCMPacket* packet) {
|
||||
}
|
||||
index = seq_num % size_;
|
||||
|
||||
// Packet buffer is still full.
|
||||
// Packet buffer is still full since we were unable to expand the buffer.
|
||||
if (sequence_buffer_[index].used) {
|
||||
// Clear the buffer, delete payload, and return false to signal that a
|
||||
// new keyframe is needed.
|
||||
RTC_LOG(LS_WARNING) << "Clear PacketBuffer and request key frame.";
|
||||
Clear();
|
||||
delete[] packet->dataPtr;
|
||||
packet->dataPtr = nullptr;
|
||||
return false;
|
||||
@ -224,8 +228,7 @@ int PacketBuffer::GetUniqueFramesSeen() const {
|
||||
bool PacketBuffer::ExpandBufferSize() {
|
||||
if (size_ == max_size_) {
|
||||
RTC_LOG(LS_WARNING) << "PacketBuffer is already at max size (" << max_size_
|
||||
<< "), failed to increase size. Clearing PacketBuffer.";
|
||||
Clear();
|
||||
<< "), failed to increase size.";
|
||||
return false;
|
||||
}
|
||||
|
||||
|
@ -49,9 +49,10 @@ class PacketBuffer {
|
||||
|
||||
virtual ~PacketBuffer();
|
||||
|
||||
// Returns true if |packet| is inserted into the packet buffer, false
|
||||
// otherwise. The PacketBuffer will always take ownership of the
|
||||
// |packet.dataPtr| when this function is called. Made virtual for testing.
|
||||
// Returns true unless the packet buffer is cleared, which means that a key
|
||||
// frame request should be sent. The PacketBuffer will always take ownership
|
||||
// of the |packet.dataPtr| when this function is called. Made virtual for
|
||||
// testing.
|
||||
virtual bool InsertPacket(VCMPacket* packet);
|
||||
void ClearTo(uint16_t seq_num);
|
||||
void Clear();
|
||||
|
@ -153,7 +153,7 @@ TEST_F(TestPacketBuffer, InsertOldPackets) {
|
||||
EXPECT_TRUE(Insert(seq_num + 2, kDeltaFrame, kFirst, kLast));
|
||||
|
||||
packet_buffer_->ClearTo(seq_num + 2);
|
||||
EXPECT_FALSE(Insert(seq_num + 2, kDeltaFrame, kFirst, kLast));
|
||||
EXPECT_TRUE(Insert(seq_num + 2, kDeltaFrame, kFirst, kLast));
|
||||
EXPECT_TRUE(Insert(seq_num + 3, kDeltaFrame, kFirst, kLast));
|
||||
ASSERT_EQ(2UL, frames_from_callback_.size());
|
||||
}
|
||||
@ -246,21 +246,20 @@ TEST_F(TestPacketBuffer, HasHistoryOfUniqueFrames) {
|
||||
const uint32_t timestamp = 0xFFFFFFF0; // Large enough to cause wrap-around.
|
||||
|
||||
for (int i = 0; i < kNumFrames; ++i) {
|
||||
EXPECT_TRUE(Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
|
||||
timestamp + 10 * i));
|
||||
Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
|
||||
timestamp + 10 * i);
|
||||
}
|
||||
ASSERT_EQ(kNumFrames, packet_buffer_->GetUniqueFramesSeen());
|
||||
|
||||
// Old packets within history should not affect number of seen unique frames.
|
||||
for (int i = kNumFrames - kRequiredHistoryLength; i < kNumFrames; ++i) {
|
||||
EXPECT_TRUE(Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
|
||||
timestamp + 10 * i));
|
||||
Insert(seq_num + i, kKeyFrame, kFirst, kNotLast, 0, nullptr,
|
||||
timestamp + 10 * i);
|
||||
}
|
||||
ASSERT_EQ(kNumFrames, packet_buffer_->GetUniqueFramesSeen());
|
||||
|
||||
// Very old packets should be treated as unique.
|
||||
EXPECT_TRUE(
|
||||
Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp));
|
||||
Insert(seq_num, kKeyFrame, kFirst, kNotLast, 0, nullptr, timestamp);
|
||||
ASSERT_EQ(kNumFrames + 1, packet_buffer_->GetUniqueFramesSeen());
|
||||
}
|
||||
|
||||
@ -289,7 +288,7 @@ TEST_F(TestPacketBuffer, ExpandBufferOverflow) {
|
||||
|
||||
for (int i = 0; i < kMaxSize; ++i)
|
||||
EXPECT_TRUE(Insert(seq_num + i, kKeyFrame, kFirst, kLast));
|
||||
EXPECT_TRUE(Insert(seq_num + kMaxSize + 1, kKeyFrame, kFirst, kLast));
|
||||
EXPECT_FALSE(Insert(seq_num + kMaxSize + 1, kKeyFrame, kFirst, kLast));
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, OnePacketOneFrame) {
|
||||
@ -728,10 +727,10 @@ TEST_F(TestPacketBuffer, DontLeakPayloadData) {
|
||||
|
||||
// Expect to free data3 upon insertion (old packet).
|
||||
packet_buffer_->ClearTo(1);
|
||||
EXPECT_FALSE(Insert(1, kKeyFrame, kFirst, kNotLast, 5, data3));
|
||||
EXPECT_TRUE(Insert(1, kKeyFrame, kFirst, kNotLast, 5, data3));
|
||||
|
||||
// Expect to free data4 upon insertion (packet buffer is full).
|
||||
EXPECT_TRUE(Insert(2 + kMaxSize, kKeyFrame, kFirst, kNotLast, 5, data4));
|
||||
EXPECT_FALSE(Insert(2 + kMaxSize, kKeyFrame, kFirst, kNotLast, 5, data4));
|
||||
}
|
||||
|
||||
TEST_F(TestPacketBuffer, ContinuousSeqNumDoubleMarkerBit) {
|
||||
|
@ -382,7 +382,9 @@ int32_t RtpVideoStreamReceiver::OnReceivedPayloadData(
|
||||
}
|
||||
|
||||
rtcp_feedback_buffer_.SendBufferedRtcpFeedback();
|
||||
packet_buffer_->InsertPacket(&packet);
|
||||
if (!packet_buffer_->InsertPacket(&packet)) {
|
||||
RequestKeyFrame();
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
|
@ -560,6 +560,32 @@ TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeIfFirstFrameIsDelta) {
|
||||
data.data(), data.size(), rtp_header, video_header, absl::nullopt, false);
|
||||
}
|
||||
|
||||
TEST_F(RtpVideoStreamReceiverTest, RequestKeyframeWhenPacketBufferGetsFull) {
|
||||
constexpr int kPacketBufferMaxSize = 2048;
|
||||
|
||||
RTPHeader rtp_header;
|
||||
RTPVideoHeader video_header;
|
||||
const std::vector<uint8_t> data({1, 2, 3, 4});
|
||||
video_header.is_first_packet_in_frame = true;
|
||||
// Incomplete frames so that the packet buffer is filling up.
|
||||
video_header.is_last_packet_in_frame = false;
|
||||
video_header.codec = kVideoCodecGeneric;
|
||||
video_header.frame_type = VideoFrameType::kVideoFrameDelta;
|
||||
uint16_t start_sequence_number = 1234;
|
||||
rtp_header.sequenceNumber = start_sequence_number;
|
||||
while (rtp_header.sequenceNumber - start_sequence_number <
|
||||
kPacketBufferMaxSize) {
|
||||
rtp_video_stream_receiver_->OnReceivedPayloadData(data.data(), data.size(),
|
||||
rtp_header, video_header,
|
||||
absl::nullopt, false);
|
||||
rtp_header.sequenceNumber += 2;
|
||||
}
|
||||
|
||||
EXPECT_CALL(mock_key_frame_request_sender_, RequestKeyFrame());
|
||||
rtp_video_stream_receiver_->OnReceivedPayloadData(
|
||||
data.data(), data.size(), rtp_header, video_header, absl::nullopt, false);
|
||||
}
|
||||
|
||||
TEST_F(RtpVideoStreamReceiverTest, SecondarySinksGetRtpNotifications) {
|
||||
rtp_video_stream_receiver_->StartReceive();
|
||||
|
||||
|
Reference in New Issue
Block a user