Deflake VideoSendStreamTest::TestNackRetransmission.
Rewrites some of the logic to better takine account RTX padding and potential acking from transport cc. This should make it both more robust and a bit faster. Bug: webrtc:381216373 Change-Id: I1a395c6bd86445ba3e63d79bdc766c7ff582e2a0 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/370060 Commit-Queue: Erik Språng <sprang@webrtc.org> Reviewed-by: Harald Alvestrand <hta@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43483}
This commit is contained in:
parent
7401d5531c
commit
00ec2afc80
@ -188,7 +188,8 @@ class VideoSendStreamTest : public test::CallTest {
|
||||
}
|
||||
|
||||
protected:
|
||||
void TestNackRetransmission(uint32_t retransmit_ssrc,
|
||||
void TestNackRetransmission(uint32_t media_ssrc,
|
||||
uint32_t retransmit_ssrc,
|
||||
uint8_t retransmit_payload_type);
|
||||
void TestPacketFragmentationSize(VideoFormat format, bool with_fec);
|
||||
|
||||
@ -1002,15 +1003,17 @@ TEST_F(VideoSendStreamTest, SupportsFlexfecWithMultithreadedH264) {
|
||||
}
|
||||
|
||||
void VideoSendStreamTest::TestNackRetransmission(
|
||||
uint32_t media_ssrc,
|
||||
uint32_t retransmit_ssrc,
|
||||
uint8_t retransmit_payload_type) {
|
||||
class NackObserver : public test::SendTest {
|
||||
public:
|
||||
explicit NackObserver(uint32_t retransmit_ssrc,
|
||||
explicit NackObserver(uint32_t media_ssrc,
|
||||
uint32_t retransmit_ssrc,
|
||||
uint8_t retransmit_payload_type)
|
||||
: SendTest(test::VideoTestConstants::kDefaultTimeout),
|
||||
send_count_(0),
|
||||
retransmit_count_(0),
|
||||
media_ssrc_(media_ssrc),
|
||||
retransmit_ssrc_(retransmit_ssrc),
|
||||
retransmit_payload_type_(retransmit_payload_type) {}
|
||||
|
||||
@ -1019,19 +1022,60 @@ void VideoSendStreamTest::TestNackRetransmission(
|
||||
RtpPacket rtp_packet;
|
||||
EXPECT_TRUE(rtp_packet.Parse(packet));
|
||||
|
||||
// NACK packets two times at some arbitrary points.
|
||||
const int kNackedPacketsAtOnceCount = 3;
|
||||
const int kRetransmitTarget = kNackedPacketsAtOnceCount * 2;
|
||||
uint16_t sequence_number = rtp_packet.SequenceNumber();
|
||||
if (rtp_packet.payload_size() >= 2 &&
|
||||
rtp_packet.Ssrc() == retransmit_ssrc_ &&
|
||||
retransmit_ssrc_ != media_ssrc_) {
|
||||
// Assume correct RTX packet. Extract original sequence number.
|
||||
rtc::ArrayView<const uint8_t> payload = rtp_packet.payload();
|
||||
sequence_number = (payload[0] << 8) + payload[1];
|
||||
}
|
||||
|
||||
// Skip padding packets because they will never be retransmitted.
|
||||
if (rtp_packet.payload_size() == 0) {
|
||||
if (auto it = pending_retransmission_.find(sequence_number) !=
|
||||
pending_retransmission_.end()) {
|
||||
// Count each observer retransmission only once, so that any RTX-based
|
||||
// padding doesn't double count.
|
||||
pending_retransmission_.erase(it);
|
||||
++retransmit_count_;
|
||||
if (retransmit_count_ >= 3) {
|
||||
// Three unique retransmissions observed, should be enough for anyone.
|
||||
observation_complete_.Set();
|
||||
}
|
||||
return SEND_PACKET;
|
||||
}
|
||||
|
||||
++send_count_;
|
||||
// Skip padding packets and RTX packets, they will never be retransmitted.
|
||||
if (rtp_packet.payload_size() == 0 ||
|
||||
(rtp_packet.Ssrc() == retransmit_ssrc_ &&
|
||||
retransmit_ssrc_ != media_ssrc_)) {
|
||||
return SEND_PACKET;
|
||||
}
|
||||
|
||||
// NACK packets at arbitrary points.
|
||||
if (send_count_ % 25 == 0) {
|
||||
// Immediately add any new media packet to the pending set.
|
||||
const Timestamp now = env_.clock().CurrentTime();
|
||||
pending_retransmission_.emplace(sequence_number, now);
|
||||
|
||||
// Find all requests we have not yet gotten a retransmission for,
|
||||
// filtering out only those entries which have not be sent or requested
|
||||
// within the last 50ms. A grace period is needed since the sender will
|
||||
// not respond to a NACK within the first RTT of sending a message.
|
||||
const TimeDelta kNackInterval = TimeDelta::Millis(50);
|
||||
const size_t kMaxNackSize = 100;
|
||||
|
||||
std::vector<uint16_t> sequence_numbers;
|
||||
for (auto& kv : pending_retransmission_) {
|
||||
if (now - kv.second >= kNackInterval) {
|
||||
sequence_numbers.push_back(kv.first);
|
||||
kv.second = now;
|
||||
|
||||
if (sequence_numbers.size() >= kMaxNackSize) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (!sequence_numbers.empty()) {
|
||||
// Inject a NACK message for the found sequence numbers.
|
||||
RTCPSender::Configuration config;
|
||||
config.outgoing_transport = transport_adapter_.get();
|
||||
config.rtcp_report_interval = TimeDelta::Millis(kRtcpIntervalMs);
|
||||
@ -1043,53 +1087,14 @@ void VideoSendStreamTest::TestNackRetransmission(
|
||||
rtcp_sender.SetRemoteSSRC(test::VideoTestConstants::kVideoSendSsrcs[0]);
|
||||
|
||||
RTCPSender::FeedbackState feedback_state;
|
||||
uint16_t nack_sequence_numbers[kNackedPacketsAtOnceCount];
|
||||
int nack_count = 0;
|
||||
for (uint16_t sequence_number :
|
||||
sequence_numbers_pending_retransmission_) {
|
||||
if (nack_count < kNackedPacketsAtOnceCount) {
|
||||
nack_sequence_numbers[nack_count++] = sequence_number;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpNack, nack_count,
|
||||
nack_sequence_numbers));
|
||||
EXPECT_EQ(0, rtcp_sender.SendRTCP(feedback_state, kRtcpNack,
|
||||
sequence_numbers.size(),
|
||||
sequence_numbers.data()));
|
||||
}
|
||||
|
||||
uint16_t sequence_number = rtp_packet.SequenceNumber();
|
||||
if (rtp_packet.Ssrc() == retransmit_ssrc_ &&
|
||||
retransmit_ssrc_ != test::VideoTestConstants::kVideoSendSsrcs[0]) {
|
||||
// Not kVideoSendSsrcs[0], assume correct RTX packet. Extract sequence
|
||||
// number.
|
||||
const uint8_t* rtx_header = rtp_packet.payload().data();
|
||||
sequence_number = (rtx_header[0] << 8) + rtx_header[1];
|
||||
}
|
||||
|
||||
auto it = sequence_numbers_pending_retransmission_.find(sequence_number);
|
||||
if (it == sequence_numbers_pending_retransmission_.end()) {
|
||||
// Not currently pending retransmission. Add it to retransmission queue
|
||||
// if media and limit not reached.
|
||||
if (rtp_packet.Ssrc() == test::VideoTestConstants::kVideoSendSsrcs[0] &&
|
||||
rtp_packet.payload_size() > 0 &&
|
||||
retransmit_count_ +
|
||||
sequence_numbers_pending_retransmission_.size() <
|
||||
kRetransmitTarget) {
|
||||
sequence_numbers_pending_retransmission_.insert(sequence_number);
|
||||
return DROP_PACKET;
|
||||
}
|
||||
} else {
|
||||
// Packet is a retransmission, remove it from queue and check if done.
|
||||
sequence_numbers_pending_retransmission_.erase(it);
|
||||
if (++retransmit_count_ == kRetransmitTarget) {
|
||||
EXPECT_EQ(retransmit_ssrc_, rtp_packet.Ssrc());
|
||||
EXPECT_EQ(retransmit_payload_type_, rtp_packet.PayloadType());
|
||||
observation_complete_.Set();
|
||||
}
|
||||
}
|
||||
|
||||
return SEND_PACKET;
|
||||
// Drop media packet, otherwise transport feeback may indirectly ack the
|
||||
// packet and remove it from the packet history.
|
||||
return DROP_PACKET;
|
||||
}
|
||||
|
||||
void ModifyVideoConfigs(
|
||||
@ -1112,12 +1117,13 @@ void VideoSendStreamTest::TestNackRetransmission(
|
||||
|
||||
const Environment env_ = CreateEnvironment();
|
||||
std::unique_ptr<internal::TransportAdapter> transport_adapter_;
|
||||
int send_count_;
|
||||
int retransmit_count_;
|
||||
const uint32_t media_ssrc_;
|
||||
const uint32_t retransmit_ssrc_;
|
||||
const uint8_t retransmit_payload_type_;
|
||||
std::set<uint16_t> sequence_numbers_pending_retransmission_;
|
||||
} test(retransmit_ssrc, retransmit_payload_type);
|
||||
// Map from sequence number to timestamp for transmission or last NACK.
|
||||
std::map<uint16_t, Timestamp> pending_retransmission_;
|
||||
} test(media_ssrc, retransmit_ssrc, retransmit_payload_type);
|
||||
|
||||
RunBaseTest(&test);
|
||||
}
|
||||
@ -1125,12 +1131,14 @@ void VideoSendStreamTest::TestNackRetransmission(
|
||||
TEST_F(VideoSendStreamTest, RetransmitsNack) {
|
||||
// Normal NACKs should use the send SSRC.
|
||||
TestNackRetransmission(test::VideoTestConstants::kVideoSendSsrcs[0],
|
||||
test::VideoTestConstants::kVideoSendSsrcs[0],
|
||||
test::VideoTestConstants::kFakeVideoSendPayloadType);
|
||||
}
|
||||
|
||||
TEST_F(VideoSendStreamTest, RetransmitsNackOverRtx) {
|
||||
// NACKs over RTX should use a separate SSRC.
|
||||
TestNackRetransmission(test::VideoTestConstants::kSendRtxSsrcs[0],
|
||||
TestNackRetransmission(test::VideoTestConstants::kVideoSendSsrcs[0],
|
||||
test::VideoTestConstants::kSendRtxSsrcs[0],
|
||||
test::VideoTestConstants::kSendRtxPayloadType);
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user