Update sampling rate and number of channels of NetEq4 if decoder is changed.

We encounter a sample-underrun if NetEq is initialized with a sampling rate fs =16000 and receive Opus packets with frame-size less than 5 ms. The reason is as follows.

Let say NetEq buffer has 4 packets of Opus each of size 2.5ms this means that internally timestamp of packets incremented by 80 (internally Opus treated as 32 kHz codec). Given the initial sampling rate of NetEq, at the first time that it wants to fetch packets, it targets to fetch 160 samples. Therefore, it will only extracts 2 packets. Decoding these packets give us exactly 160 samples (5 ms at 32 kHz), however, upon decoding the first packet the internal sampling rate will be updated to 32 kHz. So it is expected that sync buffer to deliver 320 samples while it does only have 160 samples (or maybe few more as it starts with some zeros). And we encounter and under-run.

Even if we ignore the under-run  "assert(sync_buffer_->FutureLength() >= expand_->overlap_length())" (neteq_impl.cc::811) is trigered. I'm not sure what happens if we remove this assert perhaps NetEq will work fine in subsequent calls. However the first under-run is blocking ACM2 test to pass.

Here I have a solution to update sample rate as soon as a packet is inserted, if required. It not a very efficient approach as we do the same reset in NetEqImpl::Decode().

It is a bit tricky to reproduce this because the TOT ACM tests do not run ACM2. In https://webrtc-codereview.appspot.com/2192005/ I have a patch to run both ACMs. To reproduce the problem, one can patch that CL and run

$ out/Debug/modules_tests --gtest_filter=AudioCodingModuleTest.TestOpus

Note that we would not encounter any problem if NetEq4 is initiated with 32000 Hz sampling rate. You can test this by setting |kNeteqInitSampleRateHz| to 32000 in webrtc/modules/audio_coding/main/acm2/acm_receiver.cc

BUG=
R=andrew@webrtc.org, henrik.lundin@webrtc.org, kjellander@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/2306004

git-svn-id: http://webrtc.googlecode.com/svn/trunk@4896 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/audio_coding/neteq4/interface/audio_decoder.h

@@ -124,7 +124,7 @@ class AudioDecoder {
   // applicable (e.g., for RED and DTMF/AVT types).
   static AudioDecoder* CreateAudioDecoder(NetEqDecoder codec_type);
 
-  size_t channels() { return channels_; }
+  size_t channels() const { return channels_; }
 
  protected:
   static SpeechType ConvertSpeechType(int16_t type);

webrtc/modules/audio_coding/neteq4/neteq_impl.cc

@@ -465,6 +465,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
     memcpy(&main_header, &packet->header, sizeof(main_header));
   }
 
+  bool update_sample_rate_and_channels = false;
   // Reinitialize NetEq if it's needed (changed SSRC or first call).
   if ((main_header.ssrc != ssrc_) || first_packet_) {
     rtcp_.Init(main_header.sequenceNumber);
@@ -489,6 +490,9 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
 
     // Reset timestamp scaling.
     timestamp_scaler_->Reset();
+
+    // Triger an update of sampling rate and the number of channels.
+    update_sample_rate_and_channels = true;
   }
 
   // Update RTCP statistics, only for regular packets.
@@ -598,6 +602,7 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
   if (ret == PacketBuffer::kFlushed) {
     // Reset DSP timestamp etc. if packet buffer flushed.
     new_codec_ = true;
+    update_sample_rate_and_channels = true;
     LOG_F(LS_WARNING) << "Packet buffer flushed";
   } else if (ret == PacketBuffer::kOversizePacket) {
     LOG_F(LS_WARNING) << "Packet larger than packet buffer";
@@ -615,6 +620,26 @@ int NetEqImpl::InsertPacketInternal(const WebRtcRTPHeader& rtp_header,
     }
   }
 
+  if (update_sample_rate_and_channels && !packet_buffer_->Empty()) {
+    // We do not use |current_rtp_payload_type_| to |set payload_type|, but
+    // get the next RTP header from |packet_buffer_| to obtain the payload type.
+    // The reason for it is the following corner case. If NetEq receives a
+    // CNG packet with a sample rate different than the current CNG then it
+    // flushes its buffer, assuming send codec must have been changed. However,
+    // payload type of the hypothetically new send codec is not known.
+    const RTPHeader* rtp_header = packet_buffer_->NextRtpHeader();
+    assert(rtp_header);
+    int payload_type = rtp_header->payloadType;
+    AudioDecoder* decoder = decoder_database_->GetDecoder(payload_type);
+    assert(decoder);  // Payloads are already checked to be valid.
+    const DecoderDatabase::DecoderInfo* decoder_info =
+        decoder_database_->GetDecoderInfo(payload_type);
+    assert(decoder_info);
+    if (decoder_info->fs_hz != fs_hz_ ||
+        decoder->channels() != algorithm_buffer_->Channels())
+      SetSampleRateAndChannels(decoder_info->fs_hz, decoder->channels());
+  }
+
   // TODO(hlundin): Move this code to DelayManager class.
   const DecoderDatabase::DecoderInfo* dec_info =
           decoder_database_->GetDecoderInfo(main_header.payloadType);
@@ -1110,7 +1135,14 @@ int NetEqImpl::Decode(PacketList* packet_list, Operations* operation,
           PacketBuffer::DeleteAllPackets(packet_list);
           return kDecoderNotFound;
         }
+        // We should have correct sampling rate and number of channels. They
+        // are set when packets are inserted.
+        if (decoder_info->fs_hz != fs_hz_ ||
+            decoder->channels() != algorithm_buffer_->Channels()) {
+          LOG_F(LS_ERROR) << "Sampling rate or number of channels mismatch.";
+          assert(false);
           SetSampleRateAndChannels(decoder_info->fs_hz, decoder->channels());
+        }
         sync_buffer_->set_end_timestamp(timestamp_);
         playout_timestamp_ = timestamp_;
       }

webrtc/modules/audio_coding/neteq4/neteq_impl_unittest.cc

@@ -159,7 +159,7 @@ TEST_F(NetEqImplTest, InsertPacket) {
   EXPECT_CALL(*decoder_database_, IsDtmf(kPayloadType))
       .WillRepeatedly(Return(false));  // This is not DTMF.
   EXPECT_CALL(*decoder_database_, GetDecoder(kPayloadType))
-      .Times(2)
+      .Times(3)
       .WillRepeatedly(Return(&mock_decoder));
   EXPECT_CALL(*decoder_database_, IsComfortNoise(kPayloadType))
       .WillRepeatedly(Return(false));  // This is not CNG.
@@ -183,6 +183,9 @@ TEST_F(NetEqImplTest, InsertPacket) {
   // index) is a pointer, and the variable pointed to is set to kPayloadType.
   // Also invoke the function DeletePacketsAndReturnOk to properly delete all
   // packets in the list (to avoid memory leaks in the test).
+  EXPECT_CALL(*packet_buffer_, NextRtpHeader())
+      .Times(1)
+      .WillOnce(Return(&rtp_header.header));
 
   // Expectations for DTMF buffer.
   EXPECT_CALL(*dtmf_buffer_, Flush())

webrtc/modules/audio_coding/neteq4/neteq_unittest.cc

@@ -21,6 +21,7 @@
 #include <string>
 #include <vector>
 
+#include "gflags/gflags.h"
 #include "gtest/gtest.h"
 #include "webrtc/modules/audio_coding/neteq4/test/NETEQTEST_RTPpacket.h"
 #include "webrtc/modules/audio_coding/codecs/pcm16b/include/pcm16b.h"
@@ -28,6 +29,8 @@
 #include "webrtc/test/testsupport/gtest_disable.h"
 #include "webrtc/typedefs.h"
 
+DEFINE_bool(gen_ref, false, "Generate reference files.");
+
 namespace webrtc {
 
 static bool IsAllZero(const int16_t* buf, int buf_length) {
@@ -313,7 +316,7 @@ void NetEqDecodingTest::DecodeAndCompare(const std::string &rtp_file,
 
   std::string ref_out_file = "";
   if (ref_file.empty()) {
-    ref_out_file = webrtc::test::OutputPath() + "neteq_out.pcm";
+    ref_out_file = webrtc::test::OutputPath() + "neteq_universal_ref.pcm";
   }
   RefFiles ref_files(ref_file, ref_out_file);
 
@@ -508,12 +511,17 @@ TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(MAYBE_TestBitExactness)) {
   // For Visual Studio 2012 and later, we will have to use the generic reference
   // file, rather than the windows-specific one.
   const std::string kInputRefFile = webrtc::test::ProjectRootPath() +
-      "resources/audio_coding/neteq_universal_ref.pcm";
+      "resources/audio_coding/neteq4_universal_ref.pcm";
 #else
   const std::string kInputRefFile =
-      webrtc::test::ResourcePath("audio_coding/neteq_universal_ref", "pcm");
+      webrtc::test::ResourcePath("audio_coding/neteq4_universal_ref", "pcm");
 #endif
+
+  if (FLAGS_gen_ref) {
+    DecodeAndCompare(kInputRtpFile, "");
+  } else {
     DecodeAndCompare(kInputRtpFile, kInputRefFile);
+  }
 }
 
 TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(TestNetworkStatistics)) {
@@ -523,14 +531,18 @@ TEST_F(NetEqDecodingTest, DISABLED_ON_ANDROID(TestNetworkStatistics)) {
   // For Visual Studio 2012 and later, we will have to use the generic reference
   // file, rather than the windows-specific one.
   const std::string kNetworkStatRefFile = webrtc::test::ProjectRootPath() +
-      "resources/audio_coding/neteq_network_stats.dat";
+      "resources/audio_coding/neteq4_network_stats.dat";
 #else
   const std::string kNetworkStatRefFile =
-      webrtc::test::ResourcePath("audio_coding/neteq_network_stats", "dat");
+      webrtc::test::ResourcePath("audio_coding/neteq4_network_stats", "dat");
 #endif
   const std::string kRtcpStatRefFile =
-      webrtc::test::ResourcePath("audio_coding/neteq_rtcp_stats", "dat");
+      webrtc::test::ResourcePath("audio_coding/neteq4_rtcp_stats", "dat");
+  if (FLAGS_gen_ref) {
+    DecodeAndCheckStats(kInputRtpFile, "", "");
+  } else {
     DecodeAndCheckStats(kInputRtpFile, kNetworkStatRefFile, kRtcpStatRefFile);
+  }
 }
 
 // TODO(hlundin): Re-enable test once the statistics interface is up and again.

webrtc/modules/modules.gyp

@@ -90,6 +90,7 @@
             '<(rbe_components_path)/remote_bitrate_estimator_components.gyp:rbe_components',
             '<(DEPTH)/testing/gmock.gyp:gmock',
             '<(DEPTH)/testing/gtest.gyp:gtest',
+            '<(DEPTH)/third_party/gflags/gflags.gyp:gflags',
             '<(webrtc_root)/common_audio/common_audio.gyp:common_audio',
             '<(webrtc_root)/modules/video_coding/codecs/vp8/vp8.gyp:webrtc_vp8',
             '<(webrtc_root)/system_wrappers/source/system_wrappers.gyp:system_wrappers',

webrtc/tools/update_resources.py

@@ -18,7 +18,7 @@ import tarfile
 import tempfile
 import urllib2
 
-DESIRED_VERSION = 16
+DESIRED_VERSION = 17
 REMOTE_URL_BASE = 'http://commondatastorage.googleapis.com/webrtc-resources'
 VERSION_FILENAME = 'webrtc-resources-version'
 FILENAME_PREFIX = 'webrtc-resources-'