From fc562e0a56385ee9afa2ef0e5a5842798210cc23 Mon Sep 17 00:00:00 2001 From: "henrik.lundin@webrtc.org" Date: Wed, 18 Mar 2015 07:32:13 +0000 Subject: [PATCH] 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 --- .../main/acm2/acm_generic_codec.cc | 54 +-------- .../main/acm2/acm_generic_codec.h | 109 +++++------------- .../main/acm2/acm_generic_codec_test.cc | 11 +- .../main/acm2/audio_coding_module_impl.cc | 89 +++++++------- .../main/acm2/audio_coding_module_impl.h | 4 + .../audio_coding_module_unittest_oldapi.cc | 82 ++++++++++--- 6 files changed, 148 insertions(+), 201 deletions(-) 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 3a6a6efa23..cf407dbed2 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.cc +++ b/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(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(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 { - WriteLockScoped wl(codec_wrapper_lock_); +AudioEncoder* ACMGenericCodec::GetAudioEncoder() { return encoder_; } diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h index 8849647c94..93fda43b81 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec.h +++ b/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) - EXCLUSIVE_LOCKS_REQUIRED(codec_wrapper_lock_); + int16_t UpdateEncoderSampFreq(uint16_t samp_freq_hz); /////////////////////////////////////////////////////////////////////////// // 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) - SHARED_LOCKS_REQUIRED(codec_wrapper_lock_); + int16_t EncoderSampFreq(uint16_t* samp_freq_hz); /////////////////////////////////////////////////////////////////////////// // 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. - const AudioEncoder* GetAudioEncoder() const; + AudioEncoder* GetAudioEncoder(); 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_); - - // 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_); + void ResetAudioEncoder(); OpusApplicationMode GetOpusApplication(int num_channels, - bool enable_dtx) const - EXCLUSIVE_LOCKS_REQUIRED(codec_wrapper_lock_); + bool enable_dtx) const; - rtc::scoped_ptr audio_encoder_ GUARDED_BY(codec_wrapper_lock_); - rtc::scoped_ptr cng_encoder_ GUARDED_BY(codec_wrapper_lock_); - rtc::scoped_ptr red_encoder_ GUARDED_BY(codec_wrapper_lock_); - AudioEncoder* encoder_ GUARDED_BY(codec_wrapper_lock_); - AudioDecoderProxy decoder_proxy_ GUARDED_BY(codec_wrapper_lock_); - WebRtcACMCodecParams acm_codec_params_ GUARDED_BY(codec_wrapper_lock_); - int bitrate_bps_ GUARDED_BY(codec_wrapper_lock_); - bool fec_enabled_ GUARDED_BY(codec_wrapper_lock_); - int loss_rate_ GUARDED_BY(codec_wrapper_lock_); - int max_playback_rate_hz_ GUARDED_BY(codec_wrapper_lock_); - int max_payload_size_bytes_ GUARDED_BY(codec_wrapper_lock_); - int max_rate_bps_ GUARDED_BY(codec_wrapper_lock_); - bool opus_dtx_enabled_ GUARDED_BY(codec_wrapper_lock_); - bool is_opus_ GUARDED_BY(codec_wrapper_lock_); - bool is_isac_ GUARDED_BY(codec_wrapper_lock_); - 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_); + rtc::scoped_ptr audio_encoder_; + rtc::scoped_ptr cng_encoder_; + rtc::scoped_ptr red_encoder_; + AudioEncoder* encoder_; + AudioDecoderProxy decoder_proxy_; + WebRtcACMCodecParams acm_codec_params_; + int bitrate_bps_; + bool fec_enabled_; + int loss_rate_; + int max_playback_rate_hz_; + int max_payload_size_bytes_; + int max_rate_bps_; + bool opus_dtx_enabled_; + bool is_opus_; + bool is_isac_; // Map from payload type to CNG sample rate (Hz). - std::map cng_pt_ GUARDED_BY(codec_wrapper_lock_); - int red_payload_type_ GUARDED_BY(codec_wrapper_lock_); - OpusApplicationMode opus_application_ GUARDED_BY(codec_wrapper_lock_); - bool opus_application_set_ GUARDED_BY(codec_wrapper_lock_); + std::map cng_pt_; + int red_payload_type_; + OpusApplicationMode opus_application_; + bool opus_application_set_; }; } // namespace acm2 diff --git a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_test.cc b/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_test.cc index e4563027f4..43682a0e1e 100644 --- a/webrtc/modules/audio_coding/main/acm2/acm_generic_codec_test.cc +++ b/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]; - int16_t out_length; + uint8_t out[kPacketSizeSamples]; AudioEncoder::EncodedInfo encoded_info; - codec_->Encode(timestamp_, kZeroData, kDataLengthSamples, 1, out, - &out_length, &encoded_info); + codec_->GetAudioEncoder()->Encode(timestamp_, kZeroData, kDataLengthSamples, + 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(out_length)); EXPECT_EQ(expected_timestamp, encoded_info.encoded_timestamp); if (expected_payload_type >= 0) EXPECT_EQ(expected_payload_type, encoded_info.payload_type); 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 718992f41d..aebe4e0cb1 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 @@ -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]; - // 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; + uint8_t stream[2 * MAX_PAYLOAD_SIZE_BYTE]; // Make room for 1 RED payload. 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, - input_data.length_per_channel, input_data.audio_channel, stream, - &length_bytes, &encoded_info); + + AudioEncoder* audio_encoder = + codecs_[current_send_codec_idx_]->GetAudioEncoder(); + // 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(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 { - if (encoded_info.encoded_bytes == 0 && encoded_info.send_even_if_empty) { - frame_type = kFrameEmpty; - current_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); } + previous_pltype = previous_pltype_; // Read it while we have the critsect. } - if (has_data_to_send) { - CriticalSectionScoped lock(callback_crit_sect_); + 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; + encoded_info.payload_type = previous_pltype; + } else { + DCHECK_GT(encoded_info.encoded_bytes, 0u); + frame_type = encoded_info.speech ? kAudioFrameSpeech : kAudioFrameCN; + } - 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, - stream, length_bytes, &my_fragmentation); - } else { - // Callback with payload data. - packetization_callback_->SendData(frame_type, current_payload_type, - encoded_info.encoded_timestamp, - stream, length_bytes, NULL); - } + { + CriticalSectionScoped lock(callback_crit_sect_); + if (packetization_callback_) { + packetization_callback_->SendData( + frame_type, encoded_info.payload_type, encoded_info.encoded_timestamp, + stream, encoded_info.encoded_bytes, + my_fragmentation.fragmentationVectorSize > 0 ? &my_fragmentation + : nullptr); } - 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(encoded_info.encoded_bytes); } ///////////////////////////////////////// diff --git a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h b/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h index 0b61752567..237d0ade0f 100644 --- a/webrtc/modules/audio_coding/main/acm2/audio_coding_module_impl.h +++ b/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_); 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 136e414330..829db2d5e7 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 @@ -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* 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 last_payload_vec_ GUARDED_BY(crit_sect_); const rtc::scoped_ptr crit_sect_; }; @@ -138,8 +145,7 @@ class AudioCodingModuleTestOldApi : public ::testing::Test { AudioCodingModuleTestOldApi() : id_(1), rtp_utility_(new RtpUtility(kFrameSizeSamples, kPayloadType)), - clock_(Clock::GetRealTimeClock()), - packet_size_samples_(kUseDefaultPacketSize) {} + clock_(Clock::GetRealTimeClock()) {} ~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 rtp_utility_; rtc::scoped_ptr 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();