Add a unit test for callbacks with empty frames and fix bug in code
This change adds a couple of new tests that verify that callbacks
with frame type kFrameEmpty are sent in between comfort noise packets.
This used to be the case until r8268, and with the fix included in
this CL is once again so.
COAUTHOR=kwiberg@webrtc.org
R=minyue@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/37229004
Cr-Commit-Position: refs/heads/master@{#8353}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8353 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/audio_coding/codecs/audio_encoder.h b/webrtc/modules/audio_coding/codecs/audio_encoder.h
index 6fc5827..91fff13 100644
--- a/webrtc/modules/audio_coding/codecs/audio_encoder.h
+++ b/webrtc/modules/audio_coding/codecs/audio_encoder.h
@@ -24,11 +24,15 @@
public:
struct EncodedInfoLeaf {
EncodedInfoLeaf()
- : encoded_bytes(0), encoded_timestamp(0), payload_type(0) {}
+ : encoded_bytes(0),
+ encoded_timestamp(0),
+ payload_type(0),
+ send_even_if_empty(false) {}
size_t encoded_bytes;
uint32_t encoded_timestamp;
int payload_type;
+ bool send_even_if_empty;
};
// This is the main struct for auxiliary encoding information. Each encoded
diff --git a/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng.cc b/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
index 82ca9ee..c85f604 100644
--- a/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
+++ b/webrtc/modules/audio_coding/codecs/cng/audio_encoder_cng.cc
@@ -154,6 +154,7 @@
return_val = EncodePassive(encoded, &info->encoded_bytes);
info->encoded_timestamp = first_timestamp_in_buffer_;
info->payload_type = cng_payload_type_;
+ info->send_even_if_empty = true;
last_frame_active_ = false;
break;
}
diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
index c65e053..19c3956 100644
--- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
+++ b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc
@@ -1211,7 +1211,7 @@
*bitstream_len_byte = static_cast<int16_t>(encoded_info->encoded_bytes);
if (encoded_info->encoded_bytes == 0) {
*encoding_type = kNoEncoding;
- return 0;
+ return encoded_info->send_even_if_empty ? 1 : 0;
}
*timestamp = encoded_info->encoded_timestamp;
diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc
index 1837be6..8d0f318 100644
--- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc
+++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc
@@ -294,7 +294,7 @@
} else {
switch (encoding_type) {
case kNoEncoding: {
- FATAL() << "This case is no longer valid.";
+ current_payload_type = previous_pltype_;
frame_type = kFrameEmpty;
length_bytes = 0;
break;
diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc
index 56f4096..329edd3 100644
--- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc
+++ b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc
@@ -37,12 +37,15 @@
namespace webrtc {
+namespace {
const int kSampleRateHz = 16000;
const int kNumSamples10ms = kSampleRateHz / 100;
const int kFrameSizeMs = 10; // Multiple of 10.
const int kFrameSizeSamples = kFrameSizeMs / 10 * kNumSamples10ms;
const int kPayloadSizeBytes = kFrameSizeSamples * sizeof(int16_t);
const uint8_t kPayloadType = 111;
+const int kUseDefaultPacketSize = -1;
+} // namespace
class RtpUtility {
public:
@@ -75,10 +78,12 @@
uint8_t payload_type_;
};
-class PacketizationCallbackStub : public AudioPacketizationCallback {
+class PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
public:
- PacketizationCallbackStub()
+ PacketizationCallbackStubOldApi()
: num_calls_(0),
+ last_frame_type_(kFrameEmpty),
+ last_payload_type_(-1),
crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) {}
virtual int32_t SendData(
@@ -90,6 +95,8 @@
const RTPFragmentationHeader* fragmentation) OVERRIDE {
CriticalSectionScoped lock(crit_sect_.get());
++num_calls_;
+ last_frame_type_ = frame_type;
+ last_payload_type_ = payload_type;
last_payload_vec_.assign(payload_data, payload_data + payload_len_bytes);
return 0;
}
@@ -104,6 +111,16 @@
return last_payload_vec_.size();
}
+ FrameType last_frame_type() const {
+ CriticalSectionScoped lock(crit_sect_.get());
+ return last_frame_type_;
+ }
+
+ int last_payload_type() const {
+ CriticalSectionScoped lock(crit_sect_.get());
+ return last_payload_type_;
+ }
+
void SwapBuffers(std::vector<uint8_t>* payload) {
CriticalSectionScoped lock(crit_sect_.get());
last_payload_vec_.swap(*payload);
@@ -111,6 +128,8 @@
private:
int num_calls_ GUARDED_BY(crit_sect_);
+ FrameType last_frame_type_ GUARDED_BY(crit_sect_);
+ int last_payload_type_ GUARDED_BY(crit_sect_);
std::vector<uint8_t> last_payload_vec_ GUARDED_BY(crit_sect_);
const scoped_ptr<CriticalSectionWrapper> crit_sect_;
};
@@ -120,7 +139,8 @@
AudioCodingModuleTestOldApi()
: id_(1),
rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)),
- clock_(Clock::GetRealTimeClock()) {}
+ clock_(Clock::GetRealTimeClock()),
+ packet_size_samples_(kUseDefaultPacketSize) {}
~AudioCodingModuleTestOldApi() {}
@@ -129,8 +149,6 @@
void SetUp() {
acm_.reset(AudioCodingModule::Create(id_, clock_));
- RegisterCodec();
-
rtp_utility_->Populate(&rtp_header_);
input_frame_.sample_rate_hz_ = kSampleRateHz;
@@ -148,6 +166,9 @@
virtual void RegisterCodec() {
AudioCodingModule::Codec("L16", &codec_, kSampleRateHz, 1);
codec_.pltype = kPayloadType;
+ if (packet_size_samples_ != kUseDefaultPacketSize) {
+ codec_.pacsize = packet_size_samples_;
+ }
// Register L16 codec in ACM.
ASSERT_EQ(0, acm_->RegisterReceiveCodec(codec_));
@@ -186,16 +207,18 @@
const int id_;
scoped_ptr<RtpUtility> rtp_utility_;
scoped_ptr<AudioCodingModule> acm_;
- PacketizationCallbackStub packet_cb_;
+ PacketizationCallbackStubOldApi packet_cb_;
WebRtcRTPHeader rtp_header_;
AudioFrame input_frame_;
CodecInst codec_;
Clock* clock_;
+ int packet_size_samples_;
};
// Check if the statistics are initialized correctly. Before any call to ACM
// all fields have to be zero.
TEST_F(AudioCodingModuleTestOldApi, DISABLED_ON_ANDROID(InitializedToZero)) {
+ RegisterCodec();
AudioDecodingCallStats stats;
acm_->GetDecodingCallStatistics(&stats);
EXPECT_EQ(0, stats.calls_to_neteq);
@@ -210,6 +233,7 @@
// should result in generating silence, check the associated field.
TEST_F(AudioCodingModuleTestOldApi,
DISABLED_ON_ANDROID(SilenceGeneratorCalled)) {
+ RegisterCodec();
AudioDecodingCallStats stats;
const int kInitialDelay = 100;
@@ -233,6 +257,7 @@
// simulate packet loss and check if PLC and PLC-to-CNG statistics are
// correctly updated.
TEST_F(AudioCodingModuleTestOldApi, DISABLED_ON_ANDROID(NetEqCalls)) {
+ RegisterCodec();
AudioDecodingCallStats stats;
const int kNumNormalCalls = 10;
@@ -279,6 +304,86 @@
EXPECT_EQ(-1, acm_->PlayoutData10Ms(0, &audio_frame));
}
+// Checks that the transport callback is invoked once for each speech packet.
+// Also checks that the frame type is kAudioFrameSpeech.
+TEST_F(AudioCodingModuleTestOldApi, TransportCallbackIsInvokedForEachPacket) {
+ const int k10MsBlocksPerPacket = 3;
+ packet_size_samples_ = k10MsBlocksPerPacket * kSampleRateHz / 100;
+ RegisterCodec();
+ const int kLoops = 10;
+ for (int i = 0; i < kLoops; ++i) {
+ EXPECT_EQ(i / k10MsBlocksPerPacket, packet_cb_.num_calls());
+ if (packet_cb_.num_calls() > 0)
+ EXPECT_EQ(kAudioFrameSpeech, packet_cb_.last_frame_type());
+ InsertAudio();
+ Encode();
+ }
+ EXPECT_EQ(kLoops / k10MsBlocksPerPacket, packet_cb_.num_calls());
+ EXPECT_EQ(kAudioFrameSpeech, packet_cb_.last_frame_type());
+}
+
+// Introduce this class to set different expectations on the number of encoded
+// bytes. This class expects all encoded packets to be 9 bytes (matching one
+// CNG SID frame) or 0 bytes. This test depends on |input_frame_| containing
+// (near-)zero values.
+class AudioCodingModuleTestWithComfortNoiseOldApi
+ : public AudioCodingModuleTestOldApi {
+ protected:
+ void Encode() override {
+ int32_t encoded_bytes = acm_->Process();
+ // Expect either one packet with 9 comfort noise parameters, or no packet
+ // at all.
+ EXPECT_TRUE(encoded_bytes == 9 || encoded_bytes == 0);
+ }
+};
+
+// Checks that the transport callback is invoked once for frame period of the
+// underlying speech encoder, even when comfort noise is produced.
+// Also checks that the frame type is kAudioFrameCN or kFrameEmpty.
+TEST_F(AudioCodingModuleTestWithComfortNoiseOldApi,
+ TransportCallbackTestForComfortNoise) {
+ const int k10MsBlocksPerPacket = 3;
+ packet_size_samples_ = k10MsBlocksPerPacket * kSampleRateHz / 100;
+ RegisterCodec();
+ ASSERT_EQ(0, acm_->SetVAD(true, true));
+ const int kLoops = 40;
+ // This array defines the expected frame types, and when they should arrive.
+ // We expect a frame to arrive each time the speech encoder would have
+ // produced a packet, and once every 100 ms the frame should be non-empty,
+ // that is contain comfort noise.
+ const struct {
+ int ix;
+ FrameType type;
+ } expectation[] = {{2, kAudioFrameCN},
+ {5, kFrameEmpty},
+ {8, kFrameEmpty},
+ {11, kAudioFrameCN},
+ {14, kFrameEmpty},
+ {17, kFrameEmpty},
+ {20, kAudioFrameCN},
+ {23, kFrameEmpty},
+ {26, kFrameEmpty},
+ {29, kFrameEmpty},
+ {32, kAudioFrameCN},
+ {35, kFrameEmpty},
+ {38, kFrameEmpty}};
+ for (int i = 0; i < kLoops; ++i) {
+ int num_calls_before = packet_cb_.num_calls();
+ EXPECT_EQ(i / k10MsBlocksPerPacket, num_calls_before);
+ InsertAudio();
+ Encode();
+ int num_calls = packet_cb_.num_calls();
+ if (num_calls == num_calls_before + 1) {
+ EXPECT_EQ(expectation[num_calls - 1].ix, i);
+ EXPECT_EQ(expectation[num_calls - 1].type, packet_cb_.last_frame_type())
+ << "Wrong frame type for lap " << i;
+ EXPECT_EQ(98, packet_cb_.last_payload_type()); // Default CNG-wb type.
+ } else {
+ EXPECT_EQ(num_calls, num_calls_before);
+ }
+ }
+}
+
// A multi-threaded test for ACM. This base class is using the PCM16b 16 kHz
// codec, while the derive class AcmIsacMtTest is using iSAC.
class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi {
@@ -312,6 +417,7 @@
void SetUp() {
AudioCodingModuleTestOldApi::SetUp();
+ RegisterCodec(); // Must be called before the threads start below.
StartThreads();
}
@@ -440,6 +546,7 @@
void SetUp() {
AudioCodingModuleTestOldApi::SetUp();
+ RegisterCodec(); // Must be called before the threads start below.
// Set up input audio source to read from specified file, loop after 5
// seconds, and deliver blocks of 10 ms.
@@ -461,10 +568,11 @@
StartThreads();
}
- virtual void RegisterCodec() {
+ void RegisterCodec() override {
static_assert(kSampleRateHz == 16000, "test designed for iSAC 16 kHz");
AudioCodingModule::Codec("ISAC", &codec_, kSampleRateHz, 1);
codec_.pltype = kPayloadType;
+ ASSERT_EQ(kUseDefaultPacketSize, packet_size_samples_);
// Register iSAC codec in ACM, effectively unregistering the PCM16B codec
// registered in AudioCodingModuleTestOldApi::SetUp();
