Ensure that NetEq recovers after a large timestamp jump
Before this change it could happen that a large jump in timestamp (a
jump not correlated to wall-clock change) caused the audio to go silent
without recovering. The reason was that all incoming packets after the
jump were considered too old compared to the last decoded packet, and
were deleted. With CL changes two things:
1. If the only available packet in the buffer is an old packet, NetEq
will do Expand instead of immediate reset. This is to avoid that one
late packet triggers a reset.
2. Old packets are discarded only when the decision to decode a packet
has been taken. This is to allow the buffer to grow and eventually
flush if no decodable packet has been found for some time.
This CL also includes a new unit test for this situation.
BUG=3785
R=minyue@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/22709004
git-svn-id: http://webrtc.googlecode.com/svn/trunk@7255 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/audio_coding/neteq/decision_logic_normal.cc b/webrtc/modules/audio_coding/neteq/decision_logic_normal.cc
index 97a8843..9e42204 100644
--- a/webrtc/modules/audio_coding/neteq/decision_logic_normal.cc
+++ b/webrtc/modules/audio_coding/neteq/decision_logic_normal.cc
@@ -76,8 +76,10 @@
available_timestamp, play_dtmf);
} else {
// This implies that available_timestamp < target_timestamp, which can
- // happen when a new stream or codec is received. Signal for a reset.
- return kUndefined;
+ // happen when a new stream or codec is received. Do Expand instead, and
+ // wait for a newer packet to arrive, or for the buffer to flush (resulting
+ // in a master reset).
+ return kExpand;
}
}
diff --git a/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc b/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc
index 7761525..6a8eafa 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc
@@ -19,6 +19,7 @@
#include "webrtc/modules/audio_coding/neteq/mock/mock_external_decoder_pcm16b.h"
#include "webrtc/modules/audio_coding/neteq/tools/input_audio_file.h"
#include "webrtc/modules/audio_coding/neteq/tools/rtp_generator.h"
+#include "webrtc/system_wrappers/interface/compile_assert.h"
#include "webrtc/system_wrappers/interface/scoped_ptr.h"
#include "webrtc/test/testsupport/fileutils.h"
#include "webrtc/test/testsupport/gtest_disable.h"
@@ -26,6 +27,7 @@
namespace webrtc {
using ::testing::_;
+using ::testing::Return;
// This test encodes a few packets of PCM16b 32 kHz data and inserts it into two
// different NetEq instances. The first instance uses the internal version of
@@ -50,10 +52,9 @@
payload_size_bytes_(0),
last_send_time_(0),
last_arrival_time_(0) {
- NetEq::Config config;
- config.sample_rate_hz = sample_rate_hz_;
- neteq_external_ = NetEq::Create(config);
- neteq_ = NetEq::Create(config);
+ config_.sample_rate_hz = sample_rate_hz_;
+ neteq_external_ = NetEq::Create(config_);
+ neteq_ = NetEq::Create(config_);
input_ = new int16_t[frame_size_samples_];
encoded_ = new uint8_t[2 * frame_size_samples_];
}
@@ -158,6 +159,8 @@
EXPECT_EQ(output_size_samples_, samples_per_channel);
}
+ virtual int NumExpectedDecodeCalls(int num_loops) const { return num_loops; }
+
void RunTest(int num_loops) {
// Get next input packets (mono and multi-channel).
int next_send_time;
@@ -169,7 +172,7 @@
} while (Lost()); // If lost, immediately read the next packet.
EXPECT_CALL(*external_decoder_, Decode(_, payload_size_bytes_, _, _))
- .Times(num_loops);
+ .Times(NumExpectedDecodeCalls(num_loops));
int time_now = 0;
for (int k = 0; k < num_loops; ++k) {
@@ -196,11 +199,12 @@
}
}
- const int sample_rate_hz_;
- const int samples_per_ms_;
+ NetEq::Config config_;
+ int sample_rate_hz_;
+ int samples_per_ms_;
const int frame_size_ms_;
- const int frame_size_samples_;
- const int output_size_samples_;
+ int frame_size_samples_;
+ int output_size_samples_;
NetEq* neteq_external_;
NetEq* neteq_;
scoped_ptr<MockExternalPcm16B> external_decoder_;
@@ -220,4 +224,256 @@
RunTest(100); // Run 100 laps @ 10 ms each in the test loop.
}
+class LargeTimestampJumpTest : public NetEqExternalDecoderTest {
+ protected:
+ enum TestStates {
+ kInitialPhase,
+ kNormalPhase,
+ kExpandPhase,
+ kFadedExpandPhase,
+ kRecovered
+ };
+
+ LargeTimestampJumpTest()
+ : NetEqExternalDecoderTest(), test_state_(kInitialPhase) {
+ sample_rate_hz_ = 8000;
+ samples_per_ms_ = sample_rate_hz_ / 1000;
+ frame_size_samples_ = frame_size_ms_ * samples_per_ms_;
+ output_size_samples_ = frame_size_ms_ * samples_per_ms_;
+ EXPECT_CALL(*external_decoder_, Die()).Times(1);
+ external_decoder_.reset(new MockExternalPcm16B(kDecoderPCM16B));
+ }
+
+ void SetUp() OVERRIDE {
+ const std::string file_name =
+ webrtc::test::ResourcePath("audio_coding/testfile32kHz", "pcm");
+ input_file_.reset(new test::InputAudioFile(file_name));
+ assert(sample_rate_hz_ == 8000);
+ NetEqDecoder decoder = kDecoderPCM16B;
+ EXPECT_CALL(*external_decoder_, Init());
+ EXPECT_CALL(*external_decoder_, HasDecodePlc())
+ .WillRepeatedly(Return(false));
+ // NetEq is not allowed to delete the external decoder (hence Times(0)).
+ EXPECT_CALL(*external_decoder_, Die()).Times(0);
+ ASSERT_EQ(NetEq::kOK,
+ neteq_external_->RegisterExternalDecoder(
+ external_decoder_.get(), decoder, kPayloadType));
+ ASSERT_EQ(NetEq::kOK, neteq_->RegisterPayloadType(decoder, kPayloadType));
+ }
+
+ void InsertPackets(int next_arrival_time) OVERRIDE {
+ // Insert packet in external decoder instance.
+ EXPECT_CALL(*external_decoder_,
+ IncomingPacket(_,
+ payload_size_bytes_,
+ rtp_header_.header.sequenceNumber,
+ rtp_header_.header.timestamp,
+ next_arrival_time));
+ ASSERT_EQ(
+ NetEq::kOK,
+ neteq_external_->InsertPacket(
+ rtp_header_, encoded_, payload_size_bytes_, next_arrival_time));
+ }
+
+ void GetOutputAudio() OVERRIDE {
+ NetEqOutputType output_type;
+ int samples_per_channel;
+ int num_channels;
+ // Get audio from external decoder instance.
+ ASSERT_EQ(NetEq::kOK,
+ neteq_external_->GetAudio(kMaxBlockSize,
+ output_external_,
+ &samples_per_channel,
+ &num_channels,
+ &output_type));
+ EXPECT_EQ(1, num_channels);
+ EXPECT_EQ(output_size_samples_, samples_per_channel);
+ UpdateState(output_type);
+ }
+
+ virtual void UpdateState(NetEqOutputType output_type) {
+ switch (test_state_) {
+ case kInitialPhase: {
+ if (output_type == kOutputNormal) {
+ test_state_ = kNormalPhase;
+ }
+ break;
+ }
+ case kNormalPhase: {
+ if (output_type == kOutputPLC) {
+ test_state_ = kExpandPhase;
+ }
+ break;
+ }
+ case kExpandPhase: {
+ if (output_type == kOutputPLCtoCNG) {
+ test_state_ = kFadedExpandPhase;
+ }
+ break;
+ }
+ case kFadedExpandPhase: {
+ if (output_type == kOutputNormal) {
+ test_state_ = kRecovered;
+ }
+ break;
+ }
+ case kRecovered: {
+ break;
+ }
+ }
+ }
+
+ void VerifyOutput(size_t num_samples) const OVERRIDE {
+ if (test_state_ == kExpandPhase || test_state_ == kFadedExpandPhase) {
+ // Don't verify the output in this phase of the test.
+ return;
+ }
+ for (size_t i = 0; i < num_samples; ++i) {
+ if (output_external_[i] != 0)
+ return;
+ }
+ EXPECT_TRUE(false)
+ << "Expected at least one non-zero sample in each output block.";
+ }
+
+ int NumExpectedDecodeCalls(int num_loops) const OVERRIDE {
+ // Some packets won't be decoded because of the buffer being flushed after
+ // the timestamp jump.
+ return num_loops - (config_.max_packets_in_buffer + 1);
+ }
+
+ TestStates test_state_;
+};
+
+TEST_F(LargeTimestampJumpTest, JumpLongerThanHalfRange) {
+ // Set the timestamp series to start at 2880, increase to 7200, then jump to
+ // 2869342376. The sequence numbers start at 42076 and increase by 1 for each
+ // packet, also when the timestamp jumps.
+ static const uint16_t kStartSeqeunceNumber = 42076;
+ static const uint32_t kStartTimestamp = 2880;
+ static const uint32_t kJumpFromTimestamp = 7200;
+ static const uint32_t kJumpToTimestamp = 2869342376;
+ COMPILE_ASSERT(kJumpFromTimestamp < kJumpToTimestamp,
+ timestamp_jump_should_not_result_in_wrap);
+ COMPILE_ASSERT(
+ static_cast<uint32_t>(kJumpToTimestamp - kJumpFromTimestamp) > 0x7FFFFFFF,
+ jump_should_be_larger_than_half_range);
+ // Replace the default RTP generator with one that jumps in timestamp.
+ rtp_generator_.reset(new test::TimestampJumpRtpGenerator(samples_per_ms_,
+ kStartSeqeunceNumber,
+ kStartTimestamp,
+ kJumpFromTimestamp,
+ kJumpToTimestamp));
+
+ RunTest(130); // Run 130 laps @ 10 ms each in the test loop.
+ EXPECT_EQ(kRecovered, test_state_);
+}
+
+TEST_F(LargeTimestampJumpTest, JumpLongerThanHalfRangeAndWrap) {
+ // Make a jump larger than half the 32-bit timestamp range. Set the start
+ // timestamp such that the jump will result in a wrap around.
+ static const uint16_t kStartSeqeunceNumber = 42076;
+ // Set the jump length slightly larger than 2^31.
+ static const uint32_t kStartTimestamp = 3221223116;
+ static const uint32_t kJumpFromTimestamp = 3221223216;
+ static const uint32_t kJumpToTimestamp = 1073744278;
+ COMPILE_ASSERT(kJumpToTimestamp < kJumpFromTimestamp,
+ timestamp_jump_should_result_in_wrap);
+ COMPILE_ASSERT(
+ static_cast<uint32_t>(kJumpToTimestamp - kJumpFromTimestamp) > 0x7FFFFFFF,
+ jump_should_be_larger_than_half_range);
+ // Replace the default RTP generator with one that jumps in timestamp.
+ rtp_generator_.reset(new test::TimestampJumpRtpGenerator(samples_per_ms_,
+ kStartSeqeunceNumber,
+ kStartTimestamp,
+ kJumpFromTimestamp,
+ kJumpToTimestamp));
+
+ RunTest(130); // Run 130 laps @ 10 ms each in the test loop.
+ EXPECT_EQ(kRecovered, test_state_);
+}
+
+class ShortTimestampJumpTest : public LargeTimestampJumpTest {
+ protected:
+ void UpdateState(NetEqOutputType output_type) OVERRIDE {
+ switch (test_state_) {
+ case kInitialPhase: {
+ if (output_type == kOutputNormal) {
+ test_state_ = kNormalPhase;
+ }
+ break;
+ }
+ case kNormalPhase: {
+ if (output_type == kOutputPLC) {
+ test_state_ = kExpandPhase;
+ }
+ break;
+ }
+ case kExpandPhase: {
+ if (output_type == kOutputNormal) {
+ test_state_ = kRecovered;
+ }
+ break;
+ }
+ case kRecovered: {
+ break;
+ }
+ default: { FAIL(); }
+ }
+ }
+
+ int NumExpectedDecodeCalls(int num_loops) const OVERRIDE {
+ // Some packets won't be decoded because of the timestamp jump.
+ return num_loops - 2;
+ }
+};
+
+TEST_F(ShortTimestampJumpTest, JumpShorterThanHalfRange) {
+ // Make a jump shorter than half the 32-bit timestamp range. Set the start
+ // timestamp such that the jump will not result in a wrap around.
+ static const uint16_t kStartSeqeunceNumber = 42076;
+ // Set the jump length slightly smaller than 2^31.
+ static const uint32_t kStartTimestamp = 4711;
+ static const uint32_t kJumpFromTimestamp = 4811;
+ static const uint32_t kJumpToTimestamp = 2147483747;
+ COMPILE_ASSERT(kJumpFromTimestamp < kJumpToTimestamp,
+ timestamp_jump_should_not_result_in_wrap);
+ COMPILE_ASSERT(
+ static_cast<uint32_t>(kJumpToTimestamp - kJumpFromTimestamp) < 0x7FFFFFFF,
+ jump_should_be_smaller_than_half_range);
+ // Replace the default RTP generator with one that jumps in timestamp.
+ rtp_generator_.reset(new test::TimestampJumpRtpGenerator(samples_per_ms_,
+ kStartSeqeunceNumber,
+ kStartTimestamp,
+ kJumpFromTimestamp,
+ kJumpToTimestamp));
+
+ RunTest(130); // Run 130 laps @ 10 ms each in the test loop.
+ EXPECT_EQ(kRecovered, test_state_);
+}
+
+TEST_F(ShortTimestampJumpTest, JumpShorterThanHalfRangeAndWrap) {
+ // Make a jump shorter than half the 32-bit timestamp range. Set the start
+ // timestamp such that the jump will result in a wrap around.
+ static const uint16_t kStartSeqeunceNumber = 42076;
+ // Set the jump length slightly smaller than 2^31.
+ static const uint32_t kStartTimestamp = 3221227827;
+ static const uint32_t kJumpFromTimestamp = 3221227927;
+ static const uint32_t kJumpToTimestamp = 1073739567;
+ COMPILE_ASSERT(kJumpToTimestamp < kJumpFromTimestamp,
+ timestamp_jump_should_result_in_wrap);
+ COMPILE_ASSERT(
+ static_cast<uint32_t>(kJumpToTimestamp - kJumpFromTimestamp) < 0x7FFFFFFF,
+ jump_should_be_smaller_than_half_range);
+ // Replace the default RTP generator with one that jumps in timestamp.
+ rtp_generator_.reset(new test::TimestampJumpRtpGenerator(samples_per_ms_,
+ kStartSeqeunceNumber,
+ kStartTimestamp,
+ kJumpFromTimestamp,
+ kJumpToTimestamp));
+
+ RunTest(130); // Run 130 laps @ 10 ms each in the test loop.
+ EXPECT_EQ(kRecovered, test_state_);
+}
+
} // namespace webrtc
diff --git a/webrtc/modules/audio_coding/neteq/neteq_impl.cc b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
index a184fc3..d714733 100644
--- a/webrtc/modules/audio_coding/neteq/neteq_impl.cc
+++ b/webrtc/modules/audio_coding/neteq/neteq_impl.cc
@@ -864,9 +864,6 @@
assert(sync_buffer_.get());
uint32_t end_timestamp = sync_buffer_->end_timestamp();
- if (!new_codec_) {
- packet_buffer_->DiscardOldPackets(end_timestamp);
- }
const RTPHeader* header = packet_buffer_->NextRtpHeader();
if (decision_logic_->CngRfc3389On() || last_mode_ == kModeRfc3389Cng) {
@@ -1817,6 +1814,14 @@
}
} while (extracted_samples < required_samples && next_packet_available);
+ if (extracted_samples > 0) {
+ // Delete old packets only when we are going to decode something. Otherwise,
+ // we could end up in the situation where we never decode anything, since
+ // all incoming packets are considered too old but the buffer will also
+ // never be flooded and flushed.
+ packet_buffer_->DiscardOldPackets(timestamp_);
+ }
+
return extracted_samples;
}
