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
diff --git a/webrtc/modules/audio_coding/neteq4/delay_manager.cc b/webrtc/modules/audio_coding/neteq4/delay_manager.cc
index 63ed525..e80b9de 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 @@
}
// 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 @@
}
// 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<uint16_t>(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<uint16_t>(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 a8cff5e..23a070d 100644
--- a/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq4/neteq_unittest.cc
@@ -18,6 +18,7 @@
#include <string.h> // memset
#include <cmath>
+#include <set>
#include <string>
#include <vector>
@@ -214,6 +215,10 @@
void CheckBgnOff(int sampling_rate, NetEqBackgroundNoiseMode bgn_mode);
+ void WrapTest(uint16_t start_seq_no, uint32_t start_timestamp,
+ const std::set<uint16_t>& 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 @@
}
}
+void NetEqDecodingTest::WrapTest(uint16_t start_seq_no,
+ uint32_t start_timestamp,
+ const std::set<uint16_t>& 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<double>(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<uint32_t>(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<uint16_t> 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<uint16_t> 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<uint16_t> 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<uint16_t> drop_seq_numbers;
+ WrapTest(0xFFFF - 10, 0xFFFFFFFF - 5000, drop_seq_numbers, true, true);
+}
+
} // namespace