Delete ACMGenericCodec::Encode and use AudioEncoder::Encode directly

Move timestamp conversion out of ACMGenericCodec. Also remove lock
from ACMGenericCodec since the instance is always protected by
acm_crit_sect_ in AudioCodingModuleImpl.

Restructuring the code in AudioCodingModuleImpl::Encode to streamline
the use of locks.

R=minyue@webrtc.org

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

Cr-Commit-Position: refs/heads/master@{#8773}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8773 4adac7df-926f-26a2-2b94-8c16560cd09d

webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc

@@ -29,6 +29,7 @@
 #include "webrtc/modules/audio_coding/codecs/red/audio_encoder_copy_red.h"
 #include "webrtc/modules/audio_coding/main/acm2/acm_codec_database.h"
 #include "webrtc/modules/audio_coding/main/acm2/acm_common_defs.h"
+#include "webrtc/system_wrappers/interface/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/interface/trace.h"
 
 namespace webrtc {
@@ -85,9 +86,6 @@ ACMGenericCodec::ACMGenericCodec(const CodecInst& codec_inst,
                                  int red_payload_type)
     : has_internal_fec_(false),
       copy_red_enabled_(enable_red),
-      codec_wrapper_lock_(*RWLockWrapper::CreateRWLock()),
-      last_timestamp_(0xD87F3F9F),
-      unique_id_(0),
       encoder_(NULL),
       bitrate_bps_(0),
       fec_enabled_(false),
@@ -98,12 +96,8 @@ ACMGenericCodec::ACMGenericCodec(const CodecInst& codec_inst,
       opus_dtx_enabled_(false),
       is_opus_(false),
       is_isac_(false),
-      first_frame_(true),
       red_payload_type_(red_payload_type),
       opus_application_set_(false) {
-  memset(&encoder_params_, 0, sizeof(WebRtcACMCodecParams));
-  encoder_params_.codec_inst.pltype = -1;
-
   acm_codec_params_.codec_inst = codec_inst;
   acm_codec_params_.enable_dtx = false;
   acm_codec_params_.enable_vad = false;
@@ -117,7 +111,6 @@ ACMGenericCodec::ACMGenericCodec(const CodecInst& codec_inst,
 }
 
 ACMGenericCodec::~ACMGenericCodec() {
-  delete &codec_wrapper_lock_;
 }
 
 AudioDecoderProxy::AudioDecoderProxy()
@@ -212,42 +205,13 @@ CNG_dec_inst* AudioDecoderProxy::CngDecoderInstance() {
   return decoder_->CngDecoderInstance();
 }
 
-void ACMGenericCodec::Encode(uint32_t input_timestamp,
-                             const int16_t* audio,
-                             uint16_t length_per_channel,
-                             uint8_t audio_channel,
-                             uint8_t* bitstream,
-                             int16_t* bitstream_len_byte,
-                             AudioEncoder::EncodedInfo* encoded_info) {
-  WriteLockScoped wl(codec_wrapper_lock_);
-  CHECK_EQ(length_per_channel, encoder_->SampleRateHz() / 100);
-  rtp_timestamp_ = first_frame_
-                       ? input_timestamp
-                       : last_rtp_timestamp_ +
-                             rtc::CheckedDivExact(
-                                 input_timestamp - last_timestamp_,
-                                 static_cast<uint32_t>(rtc::CheckedDivExact(
-                                     audio_encoder_->SampleRateHz(),
-                                     audio_encoder_->RtpTimestampRateHz())));
-  last_timestamp_ = input_timestamp;
-  last_rtp_timestamp_ = rtp_timestamp_;
-  first_frame_ = false;
-  CHECK_EQ(audio_channel, encoder_->NumChannels());
-
-  encoder_->Encode(rtp_timestamp_, audio, length_per_channel,
-                   2 * MAX_PAYLOAD_SIZE_BYTE, bitstream, encoded_info);
-  *bitstream_len_byte = static_cast<int16_t>(encoded_info->encoded_bytes);
-}
-
 int16_t ACMGenericCodec::EncoderParams(WebRtcACMCodecParams* enc_params) {
-  ReadLockScoped rl(codec_wrapper_lock_);
   *enc_params = acm_codec_params_;
   return 0;
 }
 
 int16_t ACMGenericCodec::InitEncoder(WebRtcACMCodecParams* codec_params,
                                      bool force_initialization) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   bitrate_bps_ = 0;
   loss_rate_ = 0;
   opus_dtx_enabled_ = false;
@@ -431,7 +395,6 @@ OpusApplicationMode ACMGenericCodec::GetOpusApplication(
 }
 
 int16_t ACMGenericCodec::SetBitRate(const int32_t bitrate_bps) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   encoder_->SetTargetBitrate(bitrate_bps);
   bitrate_bps_ = bitrate_bps;
   return 0;
@@ -440,7 +403,6 @@ int16_t ACMGenericCodec::SetBitRate(const int32_t bitrate_bps) {
 int16_t ACMGenericCodec::SetVAD(bool* enable_dtx,
                                 bool* enable_vad,
                                 ACMVADMode* mode) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (is_opus_) {
     *enable_dtx = false;
     *enable_vad = false;
@@ -468,14 +430,12 @@ int16_t ACMGenericCodec::SetVAD(bool* enable_dtx,
 }
 
 void ACMGenericCodec::SetCngPt(int sample_rate_hz, int payload_type) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   SetCngPtInMap(&cng_pt_, sample_rate_hz, payload_type);
   ResetAudioEncoder();
 }
 
 int32_t ACMGenericCodec::SetISACMaxPayloadSize(
     const uint16_t max_payload_len_bytes) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (!is_isac_)
     return -1;  // Needed for tests to pass.
   max_payload_size_bytes_ = max_payload_len_bytes;
@@ -484,7 +444,6 @@ int32_t ACMGenericCodec::SetISACMaxPayloadSize(
 }
 
 int32_t ACMGenericCodec::SetISACMaxRate(const uint32_t max_rate_bps) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (!is_isac_)
     return -1;  // Needed for tests to pass.
   max_rate_bps_ = max_rate_bps;
@@ -493,7 +452,6 @@ int32_t ACMGenericCodec::SetISACMaxRate(const uint32_t max_rate_bps) {
 }
 
 int ACMGenericCodec::SetOpusMaxPlaybackRate(int frequency_hz) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (!is_opus_)
     return -1;  // Needed for tests to pass.
   max_playback_rate_hz_ = frequency_hz;
@@ -502,12 +460,10 @@ int ACMGenericCodec::SetOpusMaxPlaybackRate(int frequency_hz) {
 }
 
 AudioDecoder* ACMGenericCodec::Decoder() {
-  ReadLockScoped rl(codec_wrapper_lock_);
   return decoder_proxy_.IsSet() ? &decoder_proxy_ : nullptr;
 }
 
 int ACMGenericCodec::EnableOpusDtx(bool force_voip) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (!is_opus_)
     return -1;  // Needed for tests to pass.
   if (!force_voip &&
@@ -523,7 +479,6 @@ int ACMGenericCodec::EnableOpusDtx(bool force_voip) {
 }
 
 int ACMGenericCodec::DisableOpusDtx() {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (!is_opus_)
     return -1;  // Needed for tests to pass.
   opus_dtx_enabled_ = false;
@@ -534,7 +489,6 @@ int ACMGenericCodec::DisableOpusDtx() {
 int ACMGenericCodec::SetFEC(bool enable_fec) {
   if (!HasInternalFEC())
     return enable_fec ? -1 : 0;
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (fec_enabled_ != enable_fec) {
     fec_enabled_ = enable_fec;
     ResetAudioEncoder();
@@ -544,7 +498,6 @@ int ACMGenericCodec::SetFEC(bool enable_fec) {
 
 int ACMGenericCodec::SetOpusApplication(OpusApplicationMode application,
                                         bool disable_dtx_if_needed) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   if (opus_dtx_enabled_ && application == kAudio) {
     if (disable_dtx_if_needed) {
       opus_dtx_enabled_ = false;
@@ -560,21 +513,18 @@ int ACMGenericCodec::SetOpusApplication(OpusApplicationMode application,
 }
 
 int ACMGenericCodec::SetPacketLossRate(int loss_rate) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   encoder_->SetProjectedPacketLossRate(loss_rate / 100.0);
   loss_rate_ = loss_rate;
   return 0;
 }
 
 void ACMGenericCodec::EnableCopyRed(bool enable, int red_payload_type) {
-  WriteLockScoped wl(codec_wrapper_lock_);
   copy_red_enabled_ = enable;
   red_payload_type_ = red_payload_type;
   ResetAudioEncoder();
 }
 
-const AudioEncoder* ACMGenericCodec::GetAudioEncoder() const {
+AudioEncoder* ACMGenericCodec::GetAudioEncoder() {
-  WriteLockScoped wl(codec_wrapper_lock_);
   return encoder_;
 }
 

webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h

@@ -34,6 +34,7 @@ namespace webrtc {
 
 struct WebRtcACMCodecParams;
 struct CodecInst;
+class CriticalSectionWrapper;
 
 namespace acm2 {
 
@@ -95,45 +96,6 @@ class ACMGenericCodec {
   //
   ACMGenericCodec* CreateInstance();
 
-  ///////////////////////////////////////////////////////////////////////////
-  // int16_t Encode()
-  // The function is called to perform an encoding of the audio stored in
-  // audio buffer. An encoding is performed only if enough audio, i.e. equal
-  // to the frame-size of the codec, exist. The audio frame will be processed
-  // by VAD and CN/DTX if required. There are few different cases.
-  //
-  // A) Neither VAD nor DTX is active; the frame is encoded by the encoder.
-  //
-  // B) VAD is enabled but not DTX; in this case the audio is processed by VAD
-  //    and encoded by the encoder. The "*encoding_type" will be either
-  //    "kActiveNormalEncode" or "kPassiveNormalEncode" if frame is active or
-  //    passive, respectively.
-  //
-  // C) DTX is enabled; if the codec has internal VAD/DTX we just encode the
-  //    frame by the encoder. Otherwise, the frame is passed through VAD and
-  //    if identified as passive, then it will be processed by CN/DTX. If the
-  //    frame is active it will be encoded by the encoder.
-  //
-  // This function acquires the appropriate locks and calls EncodeSafe() for
-  // the actual processing.
-  //
-  // Outputs:
-  //   -bitstream          : a buffer where bit-stream will be written to.
-  //   -bitstream_len_byte : contains the length of the bit-stream in
-  //                         bytes.
-  //   -timestamp          : contains the RTP timestamp, this is the
-  //                         sampling time of the first sample encoded
-  //                         (measured in number of samples).
-  //
-  //
-  void Encode(uint32_t input_timestamp,
-              const int16_t* audio,
-              uint16_t length_per_channel,
-              uint8_t audio_channel,
-              uint8_t* bitstream,
-              int16_t* bitstream_len_byte,
-              AudioEncoder::EncodedInfo* encoded_info);
-
   ///////////////////////////////////////////////////////////////////////////
   // bool EncoderInitialized();
   //
@@ -269,8 +231,7 @@ class ACMGenericCodec {
   //   -1 if failed, or if this is meaningless for the given codec.
   //    0 if succeeded.
   //
-  int16_t UpdateEncoderSampFreq(uint16_t samp_freq_hz)
+  int16_t UpdateEncoderSampFreq(uint16_t samp_freq_hz);
-      EXCLUSIVE_LOCKS_REQUIRED(codec_wrapper_lock_);
 
   ///////////////////////////////////////////////////////////////////////////
   // EncoderSampFreq()
@@ -284,8 +245,7 @@ class ACMGenericCodec {
   //   -1 if failed to output sampling rate.
   //    0 if the sample rate is returned successfully.
   //
-  int16_t EncoderSampFreq(uint16_t* samp_freq_hz)
+  int16_t EncoderSampFreq(uint16_t* samp_freq_hz);
-      SHARED_LOCKS_REQUIRED(codec_wrapper_lock_);
 
   ///////////////////////////////////////////////////////////////////////////
   // SetISACMaxPayloadSize()
@@ -401,7 +361,6 @@ class ACMGenericCodec {
   //   false otherwise.
   //
   bool HasInternalFEC() const {
-    ReadLockScoped rl(codec_wrapper_lock_);
     return has_internal_fec_;
   }
 
@@ -438,52 +397,38 @@ class ACMGenericCodec {
   // Sets if CopyRed should be enabled.
   void EnableCopyRed(bool enable, int red_payload_type);
 
-  // This method is only for testing.
+  AudioEncoder* GetAudioEncoder();
-  const AudioEncoder* GetAudioEncoder() const;
 
  private:
-  bool has_internal_fec_ GUARDED_BY(codec_wrapper_lock_);
+  bool has_internal_fec_;
 
-  bool copy_red_enabled_ GUARDED_BY(codec_wrapper_lock_);
+  bool copy_red_enabled_;
 
-  WebRtcACMCodecParams encoder_params_ GUARDED_BY(codec_wrapper_lock_);
+  void ResetAudioEncoder();
-
-  // Used to lock wrapper internal data
-  // such as buffers and state variables.
-  RWLockWrapper& codec_wrapper_lock_;
-
-  uint32_t last_timestamp_ GUARDED_BY(codec_wrapper_lock_);
-  uint32_t unique_id_;
-
-  void ResetAudioEncoder() EXCLUSIVE_LOCKS_REQUIRED(codec_wrapper_lock_);
 
   OpusApplicationMode GetOpusApplication(int num_channels,
-                                         bool enable_dtx) const
+                                         bool enable_dtx) const;
-      EXCLUSIVE_LOCKS_REQUIRED(codec_wrapper_lock_);
 
-  rtc::scoped_ptr<AudioEncoder> audio_encoder_ GUARDED_BY(codec_wrapper_lock_);
+  rtc::scoped_ptr<AudioEncoder> audio_encoder_;
-  rtc::scoped_ptr<AudioEncoder> cng_encoder_ GUARDED_BY(codec_wrapper_lock_);
+  rtc::scoped_ptr<AudioEncoder> cng_encoder_;
-  rtc::scoped_ptr<AudioEncoder> red_encoder_ GUARDED_BY(codec_wrapper_lock_);
+  rtc::scoped_ptr<AudioEncoder> red_encoder_;
-  AudioEncoder* encoder_ GUARDED_BY(codec_wrapper_lock_);
+  AudioEncoder* encoder_;
-  AudioDecoderProxy decoder_proxy_ GUARDED_BY(codec_wrapper_lock_);
+  AudioDecoderProxy decoder_proxy_;
-  WebRtcACMCodecParams acm_codec_params_ GUARDED_BY(codec_wrapper_lock_);
+  WebRtcACMCodecParams acm_codec_params_;
-  int bitrate_bps_ GUARDED_BY(codec_wrapper_lock_);
+  int bitrate_bps_;
-  bool fec_enabled_ GUARDED_BY(codec_wrapper_lock_);
+  bool fec_enabled_;
-  int loss_rate_ GUARDED_BY(codec_wrapper_lock_);
+  int loss_rate_;
-  int max_playback_rate_hz_ GUARDED_BY(codec_wrapper_lock_);
+  int max_playback_rate_hz_;
-  int max_payload_size_bytes_ GUARDED_BY(codec_wrapper_lock_);
+  int max_payload_size_bytes_;
-  int max_rate_bps_ GUARDED_BY(codec_wrapper_lock_);
+  int max_rate_bps_;
-  bool opus_dtx_enabled_ GUARDED_BY(codec_wrapper_lock_);
+  bool opus_dtx_enabled_;
-  bool is_opus_ GUARDED_BY(codec_wrapper_lock_);
+  bool is_opus_;
-  bool is_isac_ GUARDED_BY(codec_wrapper_lock_);
+  bool is_isac_;
-  bool first_frame_ GUARDED_BY(codec_wrapper_lock_);
-  uint32_t rtp_timestamp_ GUARDED_BY(codec_wrapper_lock_);
-  uint32_t last_rtp_timestamp_ GUARDED_BY(codec_wrapper_lock_);
   // Map from payload type to CNG sample rate (Hz).
-  std::map<int, int> cng_pt_ GUARDED_BY(codec_wrapper_lock_);
+  std::map<int, int> cng_pt_;
-  int red_payload_type_ GUARDED_BY(codec_wrapper_lock_);
+  int red_payload_type_;
-  OpusApplicationMode opus_application_ GUARDED_BY(codec_wrapper_lock_);
+  OpusApplicationMode opus_application_;
-  bool opus_application_set_ GUARDED_BY(codec_wrapper_lock_);
+  bool opus_application_set_;
 };
 
 }  // namespace acm2

webrtc/modules/audio_coding/main/acm2/acm_generic_codec_test.cc

@@ -17,9 +17,10 @@ namespace acm2 {
 
 namespace {
 const int kDataLengthSamples = 80;
+const int kPacketSizeSamples = 2 * kDataLengthSamples;
 const int16_t kZeroData[kDataLengthSamples] = {0};
 const CodecInst kDefaultCodecInst =
-    {0, "pcmu", 8000, 2 * kDataLengthSamples, 1, 64000};
+    {0, "pcmu", 8000, kPacketSizeSamples, 1, 64000};
 const int kCngPt = 13;
 const int kNoCngPt = 255;
 const int kRedPt = 255;  // Not using RED in this test.
@@ -43,15 +44,13 @@ class AcmGenericCodecTest : public ::testing::Test {
                        uint32_t expected_timestamp,
                        int expected_payload_type,
                        int expected_send_even_if_empty) {
-    uint8_t out[kDataLengthSamples];
+    uint8_t out[kPacketSizeSamples];
-    int16_t out_length;
     AudioEncoder::EncodedInfo encoded_info;
-    codec_->Encode(timestamp_, kZeroData, kDataLengthSamples, 1, out,
+    codec_->GetAudioEncoder()->Encode(timestamp_, kZeroData, kDataLengthSamples,
-                   &out_length, &encoded_info);
+                                      kPacketSizeSamples, out, &encoded_info);
     timestamp_ += kDataLengthSamples;
     EXPECT_TRUE(encoded_info.redundant.empty());
     EXPECT_EQ(expected_out_length, encoded_info.encoded_bytes);
-    EXPECT_EQ(expected_out_length, rtc::checked_cast<size_t>(out_length));
     EXPECT_EQ(expected_timestamp, encoded_info.encoded_timestamp);
     if (expected_payload_type >= 0)
       EXPECT_EQ(expected_payload_type, encoded_info.payload_type);

webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.cc

@@ -145,6 +145,7 @@ AudioCodingModuleImpl::AudioCodingModuleImpl(
       aux_rtp_header_(NULL),
       receiver_initialized_(false),
       first_10ms_data_(false),
+      first_frame_(true),
       callback_crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
       packetization_callback_(NULL),
       vad_callback_(NULL) {
@@ -218,16 +219,9 @@ AudioCodingModuleImpl::~AudioCodingModuleImpl() {
 }
 
 int32_t AudioCodingModuleImpl::Encode(const InputData& input_data) {
-  // Make room for 1 RED payload.
+  uint8_t stream[2 * MAX_PAYLOAD_SIZE_BYTE];  // Make room for 1 RED payload.
-  uint8_t stream[2 * MAX_PAYLOAD_SIZE_BYTE];
-  // TODO(turajs): |length_bytes| & |red_length_bytes| can be of type int if
-  // ACMGenericCodec::Encode() & ACMGenericCodec::GetRedPayload() allows.
-  int16_t length_bytes = 2 * MAX_PAYLOAD_SIZE_BYTE;
-  FrameType frame_type = kAudioFrameSpeech;
-  uint8_t current_payload_type = 0;
-  bool has_data_to_send = false;
-  RTPFragmentationHeader my_fragmentation;
   AudioEncoder::EncodedInfo encoded_info;
+  uint8_t previous_pltype;
 
   // Keep the scope of the ACM critical section limited.
   {
@@ -236,52 +230,63 @@ int32_t AudioCodingModuleImpl::Encode(const InputData& input_data) {
     if (!HaveValidEncoder("Process")) {
       return -1;
     }
-    codecs_[current_send_codec_idx_]->Encode(
+
-        input_data.input_timestamp, input_data.audio,
+    AudioEncoder* audio_encoder =
-        input_data.length_per_channel, input_data.audio_channel, stream,
+        codecs_[current_send_codec_idx_]->GetAudioEncoder();
-        &length_bytes, &encoded_info);
+    // Scale the timestamp to the codec's RTP timestamp rate.
+    uint32_t rtp_timestamp =
+        first_frame_ ? input_data.input_timestamp
+                     : last_rtp_timestamp_ +
+                           rtc::CheckedDivExact(
+                               input_data.input_timestamp - last_timestamp_,
+                               static_cast<uint32_t>(rtc::CheckedDivExact(
+                                   audio_encoder->SampleRateHz(),
+                                   audio_encoder->RtpTimestampRateHz())));
+    last_timestamp_ = input_data.input_timestamp;
+    last_rtp_timestamp_ = rtp_timestamp;
+    first_frame_ = false;
+
+    audio_encoder->Encode(rtp_timestamp, input_data.audio,
+                          input_data.length_per_channel, sizeof(stream), stream,
+                          &encoded_info);
     if (encoded_info.encoded_bytes == 0 && !encoded_info.send_even_if_empty) {
       // Not enough data.
       return 0;
-    } else {
+    }
+    previous_pltype = previous_pltype_;  // Read it while we have the critsect.
+  }
+
+  RTPFragmentationHeader my_fragmentation;
+  ConvertEncodedInfoToFragmentationHeader(encoded_info, &my_fragmentation);
+  FrameType frame_type;
   if (encoded_info.encoded_bytes == 0 && encoded_info.send_even_if_empty) {
     frame_type = kFrameEmpty;
-        current_payload_type = previous_pltype_;
+    encoded_info.payload_type = previous_pltype;
   } else {
     DCHECK_GT(encoded_info.encoded_bytes, 0u);
     frame_type = encoded_info.speech ? kAudioFrameSpeech : kAudioFrameCN;
-        current_payload_type = encoded_info.payload_type;
-        previous_pltype_ = current_payload_type;
-      }
-      has_data_to_send = true;
-
-      ConvertEncodedInfoToFragmentationHeader(encoded_info, &my_fragmentation);
-    }
   }
 
-  if (has_data_to_send) {
+  {
     CriticalSectionScoped lock(callback_crit_sect_);
-
+    if (packetization_callback_) {
-    if (packetization_callback_ != NULL) {
-      if (my_fragmentation.fragmentationVectorSize > 0) {
-        // Callback with payload data, including redundant data (RED).
       packetization_callback_->SendData(
-            frame_type, current_payload_type, encoded_info.encoded_timestamp,
+          frame_type, encoded_info.payload_type, encoded_info.encoded_timestamp,
-            stream, length_bytes, &my_fragmentation);
+          stream, encoded_info.encoded_bytes,
-      } else {
+          my_fragmentation.fragmentationVectorSize > 0 ? &my_fragmentation
-        // Callback with payload data.
+              : nullptr);
-        packetization_callback_->SendData(frame_type, current_payload_type,
-                                          encoded_info.encoded_timestamp,
-                                          stream, length_bytes, NULL);
-      }
     }
 
-    if (vad_callback_ != NULL) {
+    if (vad_callback_) {
       // Callback with VAD decision.
       vad_callback_->InFrameType(frame_type);
     }
   }
-  return length_bytes;
+  {
+    CriticalSectionScoped lock(acm_crit_sect_);
+    previous_pltype_ = encoded_info.payload_type;
+  }
+  return static_cast<int32_t>(encoded_info.encoded_bytes);
 }
 
 /////////////////////////////////////////

webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h

@@ -346,6 +346,10 @@ class AudioCodingModuleImpl : public AudioCodingModule {
   AudioFrame preprocess_frame_ GUARDED_BY(acm_crit_sect_);
   bool first_10ms_data_ GUARDED_BY(acm_crit_sect_);
 
+  bool first_frame_ GUARDED_BY(acm_crit_sect_);
+  uint32_t last_timestamp_ GUARDED_BY(acm_crit_sect_);
+  uint32_t last_rtp_timestamp_ GUARDED_BY(acm_crit_sect_);
+
   CriticalSectionWrapper* callback_crit_sect_;
   AudioPacketizationCallback* packetization_callback_
       GUARDED_BY(callback_crit_sect_);

webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc

@@ -44,7 +44,6 @@ 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 {
@@ -84,6 +83,7 @@ class PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
       : num_calls_(0),
         last_frame_type_(kFrameEmpty),
         last_payload_type_(-1),
+        last_timestamp_(0),
         crit_sect_(CriticalSectionWrapper::CreateCriticalSection()) {}
 
   int32_t SendData(FrameType frame_type,
@@ -96,6 +96,7 @@ class PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
     ++num_calls_;
     last_frame_type_ = frame_type;
     last_payload_type_ = payload_type;
+    last_timestamp_ = timestamp;
     last_payload_vec_.assign(payload_data, payload_data + payload_len_bytes);
     return 0;
   }
@@ -120,6 +121,11 @@ class PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
     return last_payload_type_;
   }
 
+  uint32_t last_timestamp() const {
+    CriticalSectionScoped lock(crit_sect_.get());
+    return last_timestamp_;
+  }
+
   void SwapBuffers(std::vector<uint8_t>* payload) {
     CriticalSectionScoped lock(crit_sect_.get());
     last_payload_vec_.swap(*payload);
@@ -129,6 +135,7 @@ class PacketizationCallbackStubOldApi : public AudioPacketizationCallback {
   int num_calls_ GUARDED_BY(crit_sect_);
   FrameType last_frame_type_ GUARDED_BY(crit_sect_);
   int last_payload_type_ GUARDED_BY(crit_sect_);
+  uint32_t last_timestamp_ GUARDED_BY(crit_sect_);
   std::vector<uint8_t> last_payload_vec_ GUARDED_BY(crit_sect_);
   const rtc::scoped_ptr<CriticalSectionWrapper> crit_sect_;
 };
@@ -138,8 +145,7 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
   AudioCodingModuleTestOldApi()
       : id_(1),
         rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)),
-        clock_(Clock::GetRealTimeClock()),
+        clock_(Clock::GetRealTimeClock()) {}
-        packet_size_samples_(kUseDefaultPacketSize) {}
 
   ~AudioCodingModuleTestOldApi() {}
 
@@ -160,16 +166,17 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
            input_frame_.samples_per_channel_ * sizeof(input_frame_.data_[0]));
 
     ASSERT_EQ(0, acm_->RegisterTransportCallback(&packet_cb_));
+
+    SetUpL16Codec();
+  }
+
+  // Set up L16 codec.
+  virtual void SetUpL16Codec() {
+    ASSERT_EQ(0, AudioCodingModule::Codec("L16", &codec_, kSampleRateHz, 1));
+    codec_.pltype = kPayloadType;
   }
 
   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_));
     ASSERT_EQ(0, acm_->RegisterSendCodec(codec_));
   }
@@ -193,7 +200,6 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
 
   virtual void InsertAudio() {
     ASSERT_GE(acm_->Add10MsData(input_frame_), 0);
-    VerifyEncoding();
     input_frame_.timestamp_ += kNumSamples10ms;
   }
 
@@ -203,6 +209,11 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
         << "Last encoded packet was " << last_length << " bytes.";
   }
 
+  virtual void InsertAudioAndVerifyEncoding() {
+    InsertAudio();
+    VerifyEncoding();
+  }
+
   const int id_;
   rtc::scoped_ptr<RtpUtility> rtp_utility_;
   rtc::scoped_ptr<AudioCodingModule> acm_;
@@ -211,7 +222,6 @@ class AudioCodingModuleTestOldApi : public ::testing::Test {
   AudioFrame input_frame_;
   CodecInst codec_;
   Clock* clock_;
-  int packet_size_samples_;
 };
 
 // Check if the statistics are initialized correctly. Before any call to ACM
@@ -307,19 +317,54 @@ TEST_F(AudioCodingModuleTestOldApi, FailOnZeroDesiredFrequency) {
 // Also checks that the frame type is kAudioFrameSpeech.
 TEST_F(AudioCodingModuleTestOldApi, TransportCallbackIsInvokedForEachPacket) {
   const int k10MsBlocksPerPacket = 3;
-  packet_size_samples_ = k10MsBlocksPerPacket * kSampleRateHz / 100;
+  codec_.pacsize = 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();
+    InsertAudioAndVerifyEncoding();
   }
   EXPECT_EQ(kLoops / k10MsBlocksPerPacket, packet_cb_.num_calls());
   EXPECT_EQ(kAudioFrameSpeech, packet_cb_.last_frame_type());
 }
 
+// Verifies that the RTP timestamp series is not reset when the codec is
+// changed.
+TEST_F(AudioCodingModuleTestOldApi, TimestampSeriesContinuesWhenCodecChanges) {
+  RegisterCodec();  // This registers the default codec.
+  uint32_t expected_ts = input_frame_.timestamp_;
+  int blocks_per_packet = codec_.pacsize / (kSampleRateHz / 100);
+  // Encode 5 packets of the first codec type.
+  const int kNumPackets1 = 5;
+  for (int j = 0; j < kNumPackets1; ++j) {
+    for (int i = 0; i < blocks_per_packet; ++i) {
+      EXPECT_EQ(j, packet_cb_.num_calls());
+      InsertAudio();
+    }
+    EXPECT_EQ(j + 1, packet_cb_.num_calls());
+    EXPECT_EQ(expected_ts, packet_cb_.last_timestamp());
+    expected_ts += codec_.pacsize;
+  }
+
+  // Change codec.
+  ASSERT_EQ(0, AudioCodingModule::Codec("ISAC", &codec_, kSampleRateHz, 1));
+  RegisterCodec();
+  blocks_per_packet = codec_.pacsize / (kSampleRateHz / 100);
+  // Encode another 5 packets.
+  const int kNumPackets2 = 5;
+  for (int j = 0; j < kNumPackets2; ++j) {
+    for (int i = 0; i < blocks_per_packet; ++i) {
+      EXPECT_EQ(kNumPackets1 + j, packet_cb_.num_calls());
+      InsertAudio();
+    }
+    EXPECT_EQ(kNumPackets1 + j + 1, packet_cb_.num_calls());
+    EXPECT_EQ(expected_ts, packet_cb_.last_timestamp());
+    expected_ts += codec_.pacsize;
+  }
+}
+
 // 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
@@ -367,7 +412,7 @@ class AudioCodingModuleTestWithComfortNoiseOldApi
     for (int i = 0; i < kLoops; ++i) {
       int num_calls_before = packet_cb_.num_calls();
       EXPECT_EQ(i / blocks_per_packet, num_calls_before);
-      InsertAudio();
+      InsertAudioAndVerifyEncoding();
       int num_calls = packet_cb_.num_calls();
       if (num_calls == num_calls_before + 1) {
         EXPECT_EQ(expectation[num_calls - 1].ix, i);
@@ -389,7 +434,7 @@ class AudioCodingModuleTestWithComfortNoiseOldApi
 TEST_F(AudioCodingModuleTestWithComfortNoiseOldApi,
        TransportCallbackTestForComfortNoiseRegisterCngLast) {
   const int k10MsBlocksPerPacket = 3;
-  packet_size_samples_ = k10MsBlocksPerPacket * kSampleRateHz / 100;
+  codec_.pacsize = k10MsBlocksPerPacket * kSampleRateHz / 100;
   RegisterCodec();
   const int kCngPayloadType = 105;
   RegisterCngCodec(kCngPayloadType);
@@ -400,7 +445,7 @@ TEST_F(AudioCodingModuleTestWithComfortNoiseOldApi,
 TEST_F(AudioCodingModuleTestWithComfortNoiseOldApi,
        TransportCallbackTestForComfortNoiseRegisterCngFirst) {
   const int k10MsBlocksPerPacket = 3;
-  packet_size_samples_ = k10MsBlocksPerPacket * kSampleRateHz / 100;
+  codec_.pacsize = k10MsBlocksPerPacket * kSampleRateHz / 100;
   const int kCngPayloadType = 105;
   RegisterCngCodec(kCngPayloadType);
   RegisterCodec();
@@ -487,7 +532,7 @@ class AudioCodingModuleMtTestOldApi : public AudioCodingModuleTestOldApi {
       test_complete_->Set();
     }
     ++send_count_;
-    InsertAudio();
+    InsertAudioAndVerifyEncoding();
     if (TestDone()) {
       test_complete_->Set();
     }
@@ -593,7 +638,6 @@ class AcmIsacMtTestOldApi : public AudioCodingModuleMtTestOldApi {
     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();