Eliminate NackModule dependency on VCMPacket

Bug: None
Change-Id: I1d4ecac123c888f2315aeb2f717ee756a472036e
Reviewed-on: https://webrtc-review.googlesource.com/95420
Reviewed-by: Philip Eliasson <philipel@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#24387}
This commit is contained in:
Niels Möller
2018-08-22 10:36:35 +02:00
committed by Commit Bot
parent 631cafafcc
commit 8dad9b414a
4 changed files with 47 additions and 98 deletions

View File

@ -108,12 +108,6 @@ int NackModule::OnReceivedPacket(uint16_t seq_num, bool is_keyframe) {
return 0; return 0;
} }
int NackModule::OnReceivedPacket(const VCMPacket& packet) {
return OnReceivedPacket(
packet.seqNum,
packet.is_first_packet_in_frame && packet.frameType == kVideoFrameKey);
}
void NackModule::ClearUpTo(uint16_t seq_num) { void NackModule::ClearUpTo(uint16_t seq_num) {
rtc::CritScope lock(&crit_); rtc::CritScope lock(&crit_);
nack_list_.erase(nack_list_.begin(), nack_list_.lower_bound(seq_num)); nack_list_.erase(nack_list_.begin(), nack_list_.lower_bound(seq_num));

View File

@ -18,7 +18,6 @@
#include "modules/include/module.h" #include "modules/include/module.h"
#include "modules/include/module_common_types.h" #include "modules/include/module_common_types.h"
#include "modules/video_coding/histogram.h" #include "modules/video_coding/histogram.h"
#include "modules/video_coding/packet.h"
#include "rtc_base/criticalsection.h" #include "rtc_base/criticalsection.h"
#include "rtc_base/numerics/sequence_number_util.h" #include "rtc_base/numerics/sequence_number_util.h"
#include "rtc_base/thread_annotations.h" #include "rtc_base/thread_annotations.h"
@ -33,7 +32,6 @@ class NackModule : public Module {
KeyFrameRequestSender* keyframe_request_sender); KeyFrameRequestSender* keyframe_request_sender);
int OnReceivedPacket(uint16_t seq_num, bool is_keyframe); int OnReceivedPacket(uint16_t seq_num, bool is_keyframe);
int OnReceivedPacket(const VCMPacket& packet);
void ClearUpTo(uint16_t seq_num); void ClearUpTo(uint16_t seq_num);
void UpdateRtt(int64_t rtt_ms); void UpdateRtt(int64_t rtt_ms);
void Clear(); void Clear();

View File

@ -40,53 +40,38 @@ class TestNackModule : public ::testing::Test,
}; };
TEST_F(TestNackModule, NackOnePacket) { TEST_F(TestNackModule, NackOnePacket) {
VCMPacket packet; nack_module_.OnReceivedPacket(1, false);
packet.seqNum = 1; nack_module_.OnReceivedPacket(3, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 3;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1u, sent_nacks_.size()); EXPECT_EQ(1u, sent_nacks_.size());
EXPECT_EQ(2, sent_nacks_[0]); EXPECT_EQ(2, sent_nacks_[0]);
} }
TEST_F(TestNackModule, WrappingSeqNum) { TEST_F(TestNackModule, WrappingSeqNum) {
VCMPacket packet; nack_module_.OnReceivedPacket(0xfffe, false);
packet.seqNum = 0xfffe; nack_module_.OnReceivedPacket(1, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 1;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(2u, sent_nacks_.size()); EXPECT_EQ(2u, sent_nacks_.size());
EXPECT_EQ(0xffff, sent_nacks_[0]); EXPECT_EQ(0xffff, sent_nacks_[0]);
EXPECT_EQ(0, sent_nacks_[1]); EXPECT_EQ(0, sent_nacks_[1]);
} }
TEST_F(TestNackModule, WrappingSeqNumClearToKeyframe) { TEST_F(TestNackModule, WrappingSeqNumClearToKeyframe) {
VCMPacket packet; nack_module_.OnReceivedPacket(0xfffe, false);
packet.seqNum = 0xfffe; nack_module_.OnReceivedPacket(1, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 1;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(2u, sent_nacks_.size()); EXPECT_EQ(2u, sent_nacks_.size());
EXPECT_EQ(0xffff, sent_nacks_[0]); EXPECT_EQ(0xffff, sent_nacks_[0]);
EXPECT_EQ(0, sent_nacks_[1]); EXPECT_EQ(0, sent_nacks_[1]);
sent_nacks_.clear(); sent_nacks_.clear();
packet.frameType = kVideoFrameKey; nack_module_.OnReceivedPacket(2, true);
packet.is_first_packet_in_frame = true;
packet.seqNum = 2;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(0u, sent_nacks_.size()); EXPECT_EQ(0u, sent_nacks_.size());
packet.seqNum = 501; nack_module_.OnReceivedPacket(501, true);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(498u, sent_nacks_.size()); EXPECT_EQ(498u, sent_nacks_.size());
for (int seq_num = 3; seq_num < 501; ++seq_num) for (int seq_num = 3; seq_num < 501; ++seq_num)
EXPECT_EQ(seq_num, sent_nacks_[seq_num - 3]); EXPECT_EQ(seq_num, sent_nacks_[seq_num - 3]);
sent_nacks_.clear(); sent_nacks_.clear();
packet.frameType = kVideoFrameDelta; nack_module_.OnReceivedPacket(1001, false);
packet.seqNum = 1001;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(499u, sent_nacks_.size()); EXPECT_EQ(499u, sent_nacks_.size());
for (int seq_num = 502; seq_num < 1001; ++seq_num) for (int seq_num = 502; seq_num < 1001; ++seq_num)
EXPECT_EQ(seq_num, sent_nacks_[seq_num - 502]); EXPECT_EQ(seq_num, sent_nacks_[seq_num - 502]);
@ -106,8 +91,7 @@ TEST_F(TestNackModule, WrappingSeqNumClearToKeyframe) {
// It will then clear all nacks up to the next keyframe (seq num 2), // It will then clear all nacks up to the next keyframe (seq num 2),
// thus removing 0xffff and 0 from the nack list. // thus removing 0xffff and 0 from the nack list.
sent_nacks_.clear(); sent_nacks_.clear();
packet.seqNum = 1004; nack_module_.OnReceivedPacket(1004, false);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(2u, sent_nacks_.size()); EXPECT_EQ(2u, sent_nacks_.size());
EXPECT_EQ(1002, sent_nacks_[0]); EXPECT_EQ(1002, sent_nacks_[0]);
EXPECT_EQ(1003, sent_nacks_[1]); EXPECT_EQ(1003, sent_nacks_[1]);
@ -123,8 +107,7 @@ TEST_F(TestNackModule, WrappingSeqNumClearToKeyframe) {
// Adding packet 1007 will cause the nack module to overflow again, thus // Adding packet 1007 will cause the nack module to overflow again, thus
// clearing everything up to 501 which is the next keyframe. // clearing everything up to 501 which is the next keyframe.
packet.seqNum = 1007; nack_module_.OnReceivedPacket(1007, false);
nack_module_.OnReceivedPacket(packet);
sent_nacks_.clear(); sent_nacks_.clear();
clock_->AdvanceTimeMilliseconds(100); clock_->AdvanceTimeMilliseconds(100);
nack_module_.Process(); nack_module_.Process();
@ -163,11 +146,8 @@ TEST_F(TestNackModule, DontBurstOnTimeSkip) {
} }
TEST_F(TestNackModule, ResendNack) { TEST_F(TestNackModule, ResendNack) {
VCMPacket packet; nack_module_.OnReceivedPacket(1, false);
packet.seqNum = 1; nack_module_.OnReceivedPacket(3, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 3;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1u, sent_nacks_.size()); EXPECT_EQ(1u, sent_nacks_.size());
EXPECT_EQ(2, sent_nacks_[0]); EXPECT_EQ(2, sent_nacks_[0]);
@ -189,19 +169,15 @@ TEST_F(TestNackModule, ResendNack) {
nack_module_.Process(); nack_module_.Process();
EXPECT_EQ(4u, sent_nacks_.size()); EXPECT_EQ(4u, sent_nacks_.size());
packet.seqNum = 2; nack_module_.OnReceivedPacket(2, false);
nack_module_.OnReceivedPacket(packet);
clock_->AdvanceTimeMilliseconds(50); clock_->AdvanceTimeMilliseconds(50);
nack_module_.Process(); nack_module_.Process();
EXPECT_EQ(4u, sent_nacks_.size()); EXPECT_EQ(4u, sent_nacks_.size());
} }
TEST_F(TestNackModule, ResendPacketMaxRetries) { TEST_F(TestNackModule, ResendPacketMaxRetries) {
VCMPacket packet; nack_module_.OnReceivedPacket(1, false);
packet.seqNum = 1; nack_module_.OnReceivedPacket(3, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 3;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1u, sent_nacks_.size()); EXPECT_EQ(1u, sent_nacks_.size());
EXPECT_EQ(2, sent_nacks_[0]); EXPECT_EQ(2, sent_nacks_[0]);
@ -217,53 +193,35 @@ TEST_F(TestNackModule, ResendPacketMaxRetries) {
} }
TEST_F(TestNackModule, TooLargeNackList) { TEST_F(TestNackModule, TooLargeNackList) {
VCMPacket packet; nack_module_.OnReceivedPacket(0, false);
packet.seqNum = 0; nack_module_.OnReceivedPacket(1001, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 1001;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1000u, sent_nacks_.size()); EXPECT_EQ(1000u, sent_nacks_.size());
EXPECT_EQ(0, keyframes_requested_); EXPECT_EQ(0, keyframes_requested_);
packet.seqNum = 1003; nack_module_.OnReceivedPacket(1003, false);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1000u, sent_nacks_.size()); EXPECT_EQ(1000u, sent_nacks_.size());
EXPECT_EQ(1, keyframes_requested_); EXPECT_EQ(1, keyframes_requested_);
packet.seqNum = 1004; nack_module_.OnReceivedPacket(1004, false);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1000u, sent_nacks_.size()); EXPECT_EQ(1000u, sent_nacks_.size());
EXPECT_EQ(1, keyframes_requested_); EXPECT_EQ(1, keyframes_requested_);
} }
TEST_F(TestNackModule, TooLargeNackListWithKeyFrame) { TEST_F(TestNackModule, TooLargeNackListWithKeyFrame) {
VCMPacket packet; nack_module_.OnReceivedPacket(0, false);
packet.seqNum = 0; nack_module_.OnReceivedPacket(1, true);
nack_module_.OnReceivedPacket(packet); nack_module_.OnReceivedPacket(1001, false);
packet.seqNum = 1;
packet.is_first_packet_in_frame = true;
packet.frameType = kVideoFrameKey;
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 1001;
packet.is_first_packet_in_frame = false;
packet.frameType = kVideoFrameKey;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(999u, sent_nacks_.size()); EXPECT_EQ(999u, sent_nacks_.size());
EXPECT_EQ(0, keyframes_requested_); EXPECT_EQ(0, keyframes_requested_);
packet.seqNum = 1003; nack_module_.OnReceivedPacket(1003, false);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1000u, sent_nacks_.size()); EXPECT_EQ(1000u, sent_nacks_.size());
EXPECT_EQ(0, keyframes_requested_); EXPECT_EQ(0, keyframes_requested_);
packet.seqNum = 1005; nack_module_.OnReceivedPacket(1005, false);
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(1000u, sent_nacks_.size()); EXPECT_EQ(1000u, sent_nacks_.size());
EXPECT_EQ(1, keyframes_requested_); EXPECT_EQ(1, keyframes_requested_);
} }
TEST_F(TestNackModule, ClearUpTo) { TEST_F(TestNackModule, ClearUpTo) {
VCMPacket packet; nack_module_.OnReceivedPacket(0, false);
packet.seqNum = 0; nack_module_.OnReceivedPacket(100, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 100;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(99u, sent_nacks_.size()); EXPECT_EQ(99u, sent_nacks_.size());
sent_nacks_.clear(); sent_nacks_.clear();
@ -275,11 +233,8 @@ TEST_F(TestNackModule, ClearUpTo) {
} }
TEST_F(TestNackModule, ClearUpToWrap) { TEST_F(TestNackModule, ClearUpToWrap) {
VCMPacket packet; nack_module_.OnReceivedPacket(0xfff0, false);
packet.seqNum = 0xfff0; nack_module_.OnReceivedPacket(0xf, false);
nack_module_.OnReceivedPacket(packet);
packet.seqNum = 0xf;
nack_module_.OnReceivedPacket(packet);
EXPECT_EQ(30u, sent_nacks_.size()); EXPECT_EQ(30u, sent_nacks_.size());
sent_nacks_.clear(); sent_nacks_.clear();
@ -291,27 +246,20 @@ TEST_F(TestNackModule, ClearUpToWrap) {
} }
TEST_F(TestNackModule, PacketNackCount) { TEST_F(TestNackModule, PacketNackCount) {
VCMPacket packet; EXPECT_EQ(0, nack_module_.OnReceivedPacket(0, false));
packet.seqNum = 0; EXPECT_EQ(0, nack_module_.OnReceivedPacket(2, false));
EXPECT_EQ(0, nack_module_.OnReceivedPacket(packet)); EXPECT_EQ(1, nack_module_.OnReceivedPacket(1, false));
packet.seqNum = 2;
EXPECT_EQ(0, nack_module_.OnReceivedPacket(packet));
packet.seqNum = 1;
EXPECT_EQ(1, nack_module_.OnReceivedPacket(packet));
sent_nacks_.clear(); sent_nacks_.clear();
nack_module_.UpdateRtt(100); nack_module_.UpdateRtt(100);
packet.seqNum = 5; EXPECT_EQ(0, nack_module_.OnReceivedPacket(5, false));
EXPECT_EQ(0, nack_module_.OnReceivedPacket(packet));
clock_->AdvanceTimeMilliseconds(100); clock_->AdvanceTimeMilliseconds(100);
nack_module_.Process(); nack_module_.Process();
clock_->AdvanceTimeMilliseconds(100); clock_->AdvanceTimeMilliseconds(100);
nack_module_.Process(); nack_module_.Process();
packet.seqNum = 3; EXPECT_EQ(3, nack_module_.OnReceivedPacket(3, false));
EXPECT_EQ(3, nack_module_.OnReceivedPacket(packet)); EXPECT_EQ(3, nack_module_.OnReceivedPacket(4, false));
packet.seqNum = 4; EXPECT_EQ(0, nack_module_.OnReceivedPacket(4, false));
EXPECT_EQ(3, nack_module_.OnReceivedPacket(packet));
EXPECT_EQ(0, nack_module_.OnReceivedPacket(packet));
} }
} // namespace webrtc } // namespace webrtc

View File

@ -199,9 +199,18 @@ int32_t RtpVideoStreamReceiver::OnReceivedPayloadData(
WebRtcRTPHeader rtp_header_with_ntp = *rtp_header; WebRtcRTPHeader rtp_header_with_ntp = *rtp_header;
rtp_header_with_ntp.ntp_time_ms = rtp_header_with_ntp.ntp_time_ms =
ntp_estimator_.Estimate(rtp_header->header.timestamp); ntp_estimator_.Estimate(rtp_header->header.timestamp);
VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp); VCMPacket packet(payload_data, payload_size, rtp_header_with_ntp);
packet.timesNacked = if (nack_module_) {
nack_module_ ? nack_module_->OnReceivedPacket(packet) : -1; const bool is_keyframe =
rtp_header->video_header().is_first_packet_in_frame &&
rtp_header->frameType == kVideoFrameKey;
packet.timesNacked = nack_module_->OnReceivedPacket(
rtp_header->header.sequenceNumber, is_keyframe);
} else {
packet.timesNacked = -1;
}
packet.receive_time_ms = clock_->TimeInMilliseconds(); packet.receive_time_ms = clock_->TimeInMilliseconds();
if (packet.sizeBytes == 0) { if (packet.sizeBytes == 0) {