Skip to content

Commit

Permalink
Ensure that NetEq recovers after a large timestamp jump
Browse files Browse the repository at this point in the history
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
  • Loading branch information
henrik.lundin@webrtc.org committed Sep 22, 2014
1 parent 8877287 commit 6121715
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 14 deletions.
6 changes: 4 additions & 2 deletions webrtc/modules/audio_coding/neteq/decision_logic_normal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ Operations DecisionLogicNormal::GetDecisionSpecialized(
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;
}
}

Expand Down
274 changes: 265 additions & 9 deletions webrtc/modules/audio_coding/neteq/neteq_external_decoder_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,15 @@
#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"

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
Expand All @@ -50,10 +52,9 @@ class NetEqExternalDecoderTest : public ::testing::Test {
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_];
}
Expand Down Expand Up @@ -158,6 +159,8 @@ class NetEqExternalDecoderTest : public ::testing::Test {
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;
Expand All @@ -169,7 +172,7 @@ class NetEqExternalDecoderTest : public ::testing::Test {
} 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) {
Expand All @@ -196,11 +199,12 @@ class NetEqExternalDecoderTest : public ::testing::Test {
}
}

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_;
Expand All @@ -220,4 +224,256 @@ TEST_F(NetEqExternalDecoderTest, RunTest) {
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
11 changes: 8 additions & 3 deletions webrtc/modules/audio_coding/neteq/neteq_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -864,9 +864,6 @@ int NetEqImpl::GetDecision(Operations* operation,

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) {
Expand Down Expand Up @@ -1817,6 +1814,14 @@ int NetEqImpl::ExtractPackets(int required_samples, PacketList* packet_list) {
}
} 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;
}

Expand Down

0 comments on commit 6121715

Please sign in to comment.