Track last packet receive times in RtpVideoStreamReceiver instead of the PacketBuffer.
Bug: webrtc:12579
Change-Id: I4adb8c6ada913127b9e65d97ddce0dc71ec6ccee
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/214784
Reviewed-by: Sam Zackrisson <saza@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Philip Eliasson <philipel@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33713}
diff --git a/modules/video_coding/packet_buffer.cc b/modules/video_coding/packet_buffer.cc
index cc4b8c1..0d4c085 100644
--- a/modules/video_coding/packet_buffer.cc
+++ b/modules/video_coding/packet_buffer.cc
@@ -30,7 +30,6 @@
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/numerics/mod_ops.h"
-#include "system_wrappers/include/clock.h"
namespace webrtc {
namespace video_coding {
@@ -51,11 +50,8 @@
rtp_packet.GetExtension<AbsoluteCaptureTimeExtension>(),
receive_time_ms) {}
-PacketBuffer::PacketBuffer(Clock* clock,
- size_t start_buffer_size,
- size_t max_buffer_size)
- : clock_(clock),
- max_size_(max_buffer_size),
+PacketBuffer::PacketBuffer(size_t start_buffer_size, size_t max_buffer_size)
+ : max_size_(max_buffer_size),
first_seq_num_(0),
first_packet_received_(false),
is_cleared_to_first_seq_num_(false),
@@ -114,14 +110,6 @@
}
}
- int64_t now_ms = clock_->TimeInMilliseconds();
- last_received_packet_ms_ = now_ms;
- if (packet->video_header.frame_type == VideoFrameType::kVideoFrameKey ||
- last_received_keyframe_rtp_timestamp_ == packet->timestamp) {
- last_received_keyframe_packet_ms_ = now_ms;
- last_received_keyframe_rtp_timestamp_ = packet->timestamp;
- }
-
packet->continuous = false;
buffer_[index] = std::move(packet);
@@ -181,18 +169,10 @@
return result;
}
-absl::optional<int64_t> PacketBuffer::LastReceivedPacketMs() const {
- MutexLock lock(&mutex_);
- return last_received_packet_ms_;
-}
-
-absl::optional<int64_t> PacketBuffer::LastReceivedKeyframePacketMs() const {
- MutexLock lock(&mutex_);
- return last_received_keyframe_packet_ms_;
-}
void PacketBuffer::ForceSpsPpsIdrIsH264Keyframe() {
sps_pps_idr_is_h264_keyframe_ = true;
}
+
void PacketBuffer::ClearInternal() {
for (auto& entry : buffer_) {
entry = nullptr;
@@ -200,8 +180,6 @@
first_packet_received_ = false;
is_cleared_to_first_seq_num_ = false;
- last_received_packet_ms_.reset();
- last_received_keyframe_packet_ms_.reset();
newest_inserted_seq_num_.reset();
missing_packets_.clear();
}
diff --git a/modules/video_coding/packet_buffer.h b/modules/video_coding/packet_buffer.h
index 35dcf82..eb8d836 100644
--- a/modules/video_coding/packet_buffer.h
+++ b/modules/video_coding/packet_buffer.h
@@ -25,7 +25,6 @@
#include "rtc_base/numerics/sequence_number_util.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/thread_annotations.h"
-#include "system_wrappers/include/clock.h"
namespace webrtc {
namespace video_coding {
@@ -76,7 +75,7 @@
};
// Both |start_buffer_size| and |max_buffer_size| must be a power of 2.
- PacketBuffer(Clock* clock, size_t start_buffer_size, size_t max_buffer_size);
+ PacketBuffer(size_t start_buffer_size, size_t max_buffer_size);
~PacketBuffer();
ABSL_MUST_USE_RESULT InsertResult InsertPacket(std::unique_ptr<Packet> packet)
@@ -86,16 +85,9 @@
void ClearTo(uint16_t seq_num) RTC_LOCKS_EXCLUDED(mutex_);
void Clear() RTC_LOCKS_EXCLUDED(mutex_);
- // Timestamp (not RTP timestamp) of the last received packet/keyframe packet.
- absl::optional<int64_t> LastReceivedPacketMs() const
- RTC_LOCKS_EXCLUDED(mutex_);
- absl::optional<int64_t> LastReceivedKeyframePacketMs() const
- RTC_LOCKS_EXCLUDED(mutex_);
void ForceSpsPpsIdrIsH264Keyframe();
private:
- Clock* const clock_;
-
// Clears with |mutex_| taken.
void ClearInternal() RTC_EXCLUSIVE_LOCKS_REQUIRED(mutex_);
@@ -132,13 +124,6 @@
// determine continuity between them.
std::vector<std::unique_ptr<Packet>> buffer_ RTC_GUARDED_BY(mutex_);
- // Timestamp of the last received packet/keyframe packet.
- absl::optional<int64_t> last_received_packet_ms_ RTC_GUARDED_BY(mutex_);
- absl::optional<int64_t> last_received_keyframe_packet_ms_
- RTC_GUARDED_BY(mutex_);
- absl::optional<uint32_t> last_received_keyframe_rtp_timestamp_
- RTC_GUARDED_BY(mutex_);
-
absl::optional<uint16_t> newest_inserted_seq_num_ RTC_GUARDED_BY(mutex_);
std::set<uint16_t, DescendingSeqNumComp<uint16_t>> missing_packets_
RTC_GUARDED_BY(mutex_);
diff --git a/modules/video_coding/packet_buffer_unittest.cc b/modules/video_coding/packet_buffer_unittest.cc
index a01b480..97012618 100644
--- a/modules/video_coding/packet_buffer_unittest.cc
+++ b/modules/video_coding/packet_buffer_unittest.cc
@@ -19,7 +19,6 @@
#include "common_video/h264/h264_common.h"
#include "modules/video_coding/frame_object.h"
#include "rtc_base/random.h"
-#include "system_wrappers/include/clock.h"
#include "test/field_trial.h"
#include "test/gmock.h"
#include "test/gtest.h"
@@ -100,10 +99,7 @@
class PacketBufferTest : public ::testing::Test {
protected:
- PacketBufferTest()
- : rand_(0x7732213),
- clock_(0),
- packet_buffer_(&clock_, kStartSize, kMaxSize) {}
+ PacketBufferTest() : rand_(0x7732213), packet_buffer_(kStartSize, kMaxSize) {}
uint16_t Rand() { return rand_.Rand<uint16_t>(); }
@@ -133,7 +129,6 @@
}
Random rand_;
- SimulatedClock clock_;
PacketBuffer packet_buffer_;
};
@@ -616,67 +611,6 @@
EXPECT_THAT(Insert(3, kKeyFrame, kNotFirst, kLast).packets, IsEmpty());
}
-TEST_F(PacketBufferTest, PacketTimestamps) {
- absl::optional<int64_t> packet_ms;
- absl::optional<int64_t> packet_keyframe_ms;
-
- packet_ms = packet_buffer_.LastReceivedPacketMs();
- packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
- EXPECT_FALSE(packet_ms);
- EXPECT_FALSE(packet_keyframe_ms);
-
- int64_t keyframe_ms = clock_.TimeInMilliseconds();
- Insert(100, kKeyFrame, kFirst, kLast, {}, /*timestamp=*/1000);
- packet_ms = packet_buffer_.LastReceivedPacketMs();
- packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
- EXPECT_TRUE(packet_ms);
- EXPECT_TRUE(packet_keyframe_ms);
- EXPECT_EQ(keyframe_ms, *packet_ms);
- EXPECT_EQ(keyframe_ms, *packet_keyframe_ms);
-
- clock_.AdvanceTimeMilliseconds(100);
- int64_t delta_ms = clock_.TimeInMilliseconds();
- Insert(101, kDeltaFrame, kFirst, kLast, {}, /*timestamp=*/2000);
- packet_ms = packet_buffer_.LastReceivedPacketMs();
- packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
- EXPECT_TRUE(packet_ms);
- EXPECT_TRUE(packet_keyframe_ms);
- EXPECT_EQ(delta_ms, *packet_ms);
- EXPECT_EQ(keyframe_ms, *packet_keyframe_ms);
-
- packet_buffer_.Clear();
- packet_ms = packet_buffer_.LastReceivedPacketMs();
- packet_keyframe_ms = packet_buffer_.LastReceivedKeyframePacketMs();
- EXPECT_FALSE(packet_ms);
- EXPECT_FALSE(packet_keyframe_ms);
-}
-
-TEST_F(PacketBufferTest,
- LastReceivedKeyFrameReturnsReceiveTimeOfALastReceivedPacketOfAKeyFrame) {
- clock_.AdvanceTimeMilliseconds(100);
- Insert(/*seq_num=*/100, kKeyFrame, kFirst, kNotLast, {}, /*timestamp=*/1000);
- EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
- clock_.TimeInMilliseconds());
-
- clock_.AdvanceTimeMilliseconds(100);
- Insert(/*seq_num=*/102, kDeltaFrame, kNotFirst, kLast, {},
- /*timestamp=*/1000);
- EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
- clock_.TimeInMilliseconds());
-
- clock_.AdvanceTimeMilliseconds(100);
- Insert(/*seq_num=*/101, kDeltaFrame, kNotFirst, kNotLast, {},
- /*timestamp=*/1000);
- EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
- clock_.TimeInMilliseconds());
-
- clock_.AdvanceTimeMilliseconds(100);
- Insert(/*seq_num=*/103, kDeltaFrame, kFirst, kNotLast, {},
- /*timestamp=*/2000);
- EXPECT_EQ(packet_buffer_.LastReceivedKeyframePacketMs(),
- clock_.TimeInMilliseconds() - 100);
-}
-
TEST_F(PacketBufferTest, IncomingCodecChange) {
auto packet = std::make_unique<PacketBuffer::Packet>();
packet->video_header.is_first_packet_in_frame = true;