From 78b41a09bd87e5be674ed854e8cc6c00e62ee108 Mon Sep 17 00:00:00 2001 From: "turaj@webrtc.org" Date: Fri, 22 Nov 2013 20:27:07 +0000 Subject: [PATCH] Fix issues with sequence number wrap-around in jitter statistics. Related CL for NetEq 3 is https://code.google.com/p/webrtc/source/detail?r=5150 Jitter statistics was not very sensitive to timestamp warp-around, and NetEqDecodingTest.TimestampWrap *DID NOT* fail before fixes applied. However, we still keep the test. The criteria for the tests are not satisfied for first few packets, before any wrap-around happens. We could either relax the bound or ignore the first few packets. We chose the latter. BUG=2662 TEST=modules_unittests,trybots R=tina.legrand@webrtc.org Review URL: https://webrtc-codereview.appspot.com/4219004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@5164 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../audio_coding/neteq4/delay_manager.cc | 18 ++- .../audio_coding/neteq4/neteq_unittest.cc | 110 ++++++++++++++++++ 2 files changed, 117 insertions(+), 11 deletions(-) diff --git a/webrtc/modules/audio_coding/neteq4/delay_manager.cc b/webrtc/modules/audio_coding/neteq4/delay_manager.cc index 63ed52506..e80b9de51 100644 --- a/webrtc/modules/audio_coding/neteq4/delay_manager.cc +++ b/webrtc/modules/audio_coding/neteq4/delay_manager.cc @@ -17,6 +17,7 @@ #include "webrtc/common_audio/signal_processing/include/signal_processing_library.h" #include "webrtc/modules/audio_coding/neteq4/delay_peak_detector.h" +#include "webrtc/modules/interface/module_common_types.h" #include "webrtc/system_wrappers/interface/logging.h" namespace webrtc { @@ -85,10 +86,9 @@ int DelayManager::Update(uint16_t sequence_number, } // Try calculating packet length from current and previous timestamps. - // TODO(hlundin): Take care of wrap-around. Not done yet due to legacy - // bit-exactness. int packet_len_ms; - if ((timestamp <= last_timestamp_) || (sequence_number <= last_seq_no_)) { + if (!IsNewerTimestamp(timestamp, last_timestamp_) || + !IsNewerSequenceNumber(sequence_number, last_seq_no_)) { // Wrong timestamp or sequence order; use stored value. packet_len_ms = packet_len_ms_; } else { @@ -111,18 +111,14 @@ int DelayManager::Update(uint16_t sequence_number, } // Check for discontinuous packet sequence and re-ordering. - if (sequence_number > last_seq_no_ + 1) { - // TODO(hlundin): Take care of wrap-around. Not done yet due to legacy - // bit-exactness. + if (IsNewerSequenceNumber(sequence_number, last_seq_no_ + 1)) { // Compensate for gap in the sequence numbers. Reduce IAT with the // expected extra time due to lost packets, but ensure that the IAT is // not negative. - iat_packets -= sequence_number - last_seq_no_ - 1; + iat_packets -= static_cast(sequence_number - last_seq_no_ - 1); iat_packets = std::max(iat_packets, 0); - } else if (sequence_number < last_seq_no_) { - // TODO(hlundin): Take care of wrap-around. - // Compensate for re-ordering. - iat_packets += last_seq_no_ + 1 - sequence_number; + } else if (!IsNewerSequenceNumber(sequence_number, last_seq_no_)) { + iat_packets += static_cast(last_seq_no_ + 1 - sequence_number); } // Saturate IAT at maximum value. diff --git a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc index a8cff5ed3..23a070de1 100644 --- a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc +++ b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc @@ -18,6 +18,7 @@ #include // memset #include +#include #include #include @@ -214,6 +215,10 @@ class NetEqDecodingTest : public ::testing::Test { void CheckBgnOff(int sampling_rate, NetEqBackgroundNoiseMode bgn_mode); + void WrapTest(uint16_t start_seq_no, uint32_t start_timestamp, + const std::set& drop_seq_numbers, + bool expect_seq_no_wrap, bool expect_timestamp_wrap); + NetEq* neteq_; FILE* rtp_fp_; unsigned int sim_clock_; @@ -1104,4 +1109,109 @@ TEST_F(NetEqDecodingTest, } } +void NetEqDecodingTest::WrapTest(uint16_t start_seq_no, + uint32_t start_timestamp, + const std::set& drop_seq_numbers, + bool expect_seq_no_wrap, + bool expect_timestamp_wrap) { + uint16_t seq_no = start_seq_no; + uint32_t timestamp = start_timestamp; + const int kBlocksPerFrame = 3; // Number of 10 ms blocks per frame. + const int kFrameSizeMs = kBlocksPerFrame * kTimeStepMs; + const int kSamples = kBlockSize16kHz * kBlocksPerFrame; + const int kPayloadBytes = kSamples * sizeof(int16_t); + double next_input_time_ms = 0.0; + int16_t decoded[kBlockSize16kHz]; + int num_channels; + int samples_per_channel; + NetEqOutputType output_type; + uint32_t receive_timestamp = 0; + + // Insert speech for 1 second. + const int kSpeechDurationMs = 2000; + int packets_inserted = 0; + uint16_t last_seq_no; + uint32_t last_timestamp; + bool timestamp_wrapped = false; + bool seq_no_wrapped = false; + for (double t_ms = 0; t_ms < kSpeechDurationMs; t_ms += 10) { + // Each turn in this for loop is 10 ms. + while (next_input_time_ms <= t_ms) { + // Insert one 30 ms speech frame. + uint8_t payload[kPayloadBytes] = {0}; + WebRtcRTPHeader rtp_info; + PopulateRtpInfo(seq_no, timestamp, &rtp_info); + if (drop_seq_numbers.find(seq_no) == drop_seq_numbers.end()) { + // This sequence number was not in the set to drop. Insert it. + ASSERT_EQ(0, + neteq_->InsertPacket(rtp_info, payload, kPayloadBytes, + receive_timestamp)); + ++packets_inserted; + } + NetEqNetworkStatistics network_stats; + ASSERT_EQ(0, neteq_->NetworkStatistics(&network_stats)); + + // Due to internal NetEq logic, preferred buffer-size is about 4 times the + // packet size for first few packets. Therefore we refrain from checking + // the criteria. + if (packets_inserted > 4) { + // Expect preferred and actual buffer size to be no more than 2 frames. + EXPECT_LE(network_stats.preferred_buffer_size_ms, kFrameSizeMs * 2); + EXPECT_LE(network_stats.current_buffer_size_ms, kFrameSizeMs * 2); + } + last_seq_no = seq_no; + last_timestamp = timestamp; + + ++seq_no; + timestamp += kSamples; + receive_timestamp += kSamples; + next_input_time_ms += static_cast(kFrameSizeMs); + + seq_no_wrapped |= seq_no < last_seq_no; + timestamp_wrapped |= timestamp < last_timestamp; + } + // Pull out data once. + ASSERT_EQ(0, neteq_->GetAudio(kBlockSize16kHz, decoded, + &samples_per_channel, &num_channels, + &output_type)); + ASSERT_EQ(kBlockSize16kHz, samples_per_channel); + ASSERT_EQ(1, num_channels); + + // Expect delay (in samples) to be less than 2 packets. + EXPECT_LE(timestamp - neteq_->PlayoutTimestamp(), + static_cast(kSamples * 2)); + + } + // Make sure we have actually tested wrap-around. + ASSERT_EQ(expect_seq_no_wrap, seq_no_wrapped); + ASSERT_EQ(expect_timestamp_wrap, timestamp_wrapped); +} + +TEST_F(NetEqDecodingTest, SequenceNumberWrap) { + // Start with a sequence number that will soon wrap. + std::set drop_seq_numbers; // Don't drop any packets. + WrapTest(0xFFFF - 10, 0, drop_seq_numbers, true, false); +} + +TEST_F(NetEqDecodingTest, SequenceNumberWrapAndDrop) { + // Start with a sequence number that will soon wrap. + std::set drop_seq_numbers; + drop_seq_numbers.insert(0xFFFF); + drop_seq_numbers.insert(0x0); + WrapTest(0xFFFF - 10, 0, drop_seq_numbers, true, false); +} + +TEST_F(NetEqDecodingTest, TimestampWrap) { + // Start with a timestamp that will soon wrap. + std::set drop_seq_numbers; + WrapTest(0, 0xFFFFFFFF - 3000, drop_seq_numbers, false, true); +} + +TEST_F(NetEqDecodingTest, TimestampAndSequenceNumberWrap) { + // Start with a timestamp and a sequence number that will wrap at the same + // time. + std::set drop_seq_numbers; + WrapTest(0xFFFF - 10, 0xFFFFFFFF - 5000, drop_seq_numbers, true, true); +} + } // namespace