From 1fddd6185dfccabcddf669a7ec923467aebe1e84 Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Fri, 30 May 2014 17:28:50 +0000 Subject: [PATCH] Add a Reset() method to AudioFrame. This method is introduced to try to avoid inconsistent resetting of AudioFrame members to default/uninitialized values. Use it at the call points of DownConvertToCodecFormat(). Results in the following minor functional changes: - speech_activity_ is set to its uninitialized value. AFAICT, this member isn't used at all in the capture path. - timestamp_ is switched from -1 to 0. This member doesn't appear to be used either in the capture path, but left a TODO for wu to change the default value to better represent the uninitialized state. Bonus: Don't copy the frame on error in RemixAndResample(). An error indicates a logical fault (as pointed out by the asserts) that we should not attempt to recover from. BUG=3111 R=turaj@webrtc.org, wu@webrtc.org Review URL: https://webrtc-codereview.appspot.com/21519007 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6289 4adac7df-926f-26a2-2b94-8c16560cd09d --- .../modules/interface/module_common_types.h | 34 +++++++++++++------ webrtc/voice_engine/utility.cc | 6 +--- webrtc/voice_engine/utility.h | 12 ++++--- 3 files changed, 32 insertions(+), 20 deletions(-) diff --git a/webrtc/modules/interface/module_common_types.h b/webrtc/modules/interface/module_common_types.h index 14139adff6..9d00de773f 100644 --- a/webrtc/modules/interface/module_common_types.h +++ b/webrtc/modules/interface/module_common_types.h @@ -667,6 +667,10 @@ class AudioFrame { AudioFrame(); virtual ~AudioFrame() {} + // Resets all members to their default state (except does not modify the + // contents of |data_|). + void Reset(); + // |interleaved_| is not changed by this method. void UpdateFrame(int id, uint32_t timestamp, const int16_t* data, int samples_per_channel, int sample_rate_hz, @@ -687,6 +691,7 @@ class AudioFrame { // RTP timestamp of the first sample in the AudioFrame. uint32_t timestamp_; // NTP time of the estimated capture time in local timebase in milliseconds. + // -1 represents an uninitialized value. int64_t ntp_time_ms_; int16_t data_[kMaxDataSizeSamples]; int samples_per_channel_; @@ -706,17 +711,24 @@ class AudioFrame { }; inline AudioFrame::AudioFrame() - : id_(-1), - timestamp_(0), - ntp_time_ms_(0), - data_(), - samples_per_channel_(0), - sample_rate_hz_(0), - num_channels_(1), - speech_type_(kUndefined), - vad_activity_(kVadUnknown), - energy_(0xffffffff), - interleaved_(true) {} + : data_() { + Reset(); +} + +inline void AudioFrame::Reset() { + id_ = -1; + // TODO(wu): Zero is a valid value for |timestamp_|. We should initialize + // to an invalid value, or add a new member to indicate invalidity. + timestamp_ = 0; + ntp_time_ms_ = -1; + samples_per_channel_ = 0; + sample_rate_hz_ = 0; + num_channels_ = 0; + speech_type_ = kUndefined; + vad_activity_ = kVadUnknown; + energy_ = 0xffffffff; + interleaved_ = true; +} inline void AudioFrame::UpdateFrame(int id, uint32_t timestamp, const int16_t* data, diff --git a/webrtc/voice_engine/utility.cc b/webrtc/voice_engine/utility.cc index b7eb885c5a..04f1f2c1ff 100644 --- a/webrtc/voice_engine/utility.cc +++ b/webrtc/voice_engine/utility.cc @@ -43,7 +43,6 @@ void RemixAndResample(const AudioFrame& src_frame, if (resampler->InitializeIfNeeded(src_frame.sample_rate_hz_, dst_frame->sample_rate_hz_, audio_ptr_num_channels) == -1) { - dst_frame->CopyFrom(src_frame); LOG_FERR3(LS_ERROR, InitializeIfNeeded, src_frame.sample_rate_hz_, dst_frame->sample_rate_hz_, audio_ptr_num_channels); assert(false); @@ -54,7 +53,6 @@ void RemixAndResample(const AudioFrame& src_frame, int out_length = resampler->Resample(audio_ptr, src_length, dst_frame->data_, AudioFrame::kMaxDataSizeSamples); if (out_length == -1) { - dst_frame->CopyFrom(src_frame); LOG_FERR3(LS_ERROR, Resample, audio_ptr, src_length, dst_frame->data_); assert(false); } @@ -81,6 +79,7 @@ void DownConvertToCodecFormat(const int16_t* src_data, assert(samples_per_channel <= kMaxMonoDataSizeSamples); assert(num_channels == 1 || num_channels == 2); assert(codec_num_channels == 1 || codec_num_channels == 2); + dst_af->Reset(); // Never upsample the capture signal here. This should be done at the // end of the send chain. @@ -116,9 +115,6 @@ void DownConvertToCodecFormat(const int16_t* src_data, dst_af->samples_per_channel_ = out_length / num_channels; dst_af->sample_rate_hz_ = destination_rate; dst_af->num_channels_ = num_channels; - dst_af->timestamp_ = -1; - dst_af->speech_type_ = AudioFrame::kNormalSpeech; - dst_af->vad_activity_ = AudioFrame::kVadUnknown; } void MixWithSat(int16_t target[], diff --git a/webrtc/voice_engine/utility.h b/webrtc/voice_engine/utility.h index 127bdba066..38206959b2 100644 --- a/webrtc/voice_engine/utility.h +++ b/webrtc/voice_engine/utility.h @@ -25,10 +25,9 @@ class AudioFrame; namespace voe { // Upmix or downmix and resample the audio in |src_frame| to |dst_frame|. -// Expects |dst_frame| to have its |num_channels_| and |sample_rate_hz_| set to -// the desired values. Updates |samples_per_channel_| accordingly. -// -// On failure, returns -1 and copies |src_frame| to |dst_frame|. +// Expects |dst_frame| to have its sample rate and channels members set to the +// desired values. Updates the samples per channel member accordingly. No other +// members will be changed. void RemixAndResample(const AudioFrame& src_frame, PushResampler* resampler, AudioFrame* dst_frame); @@ -37,6 +36,11 @@ void RemixAndResample(const AudioFrame& src_frame, // specified by |codec_num_channels| and |codec_rate_hz|. |mono_buffer| is // temporary space and must be of sufficient size to hold the downmixed source // audio (recommend using a size of kMaxMonoDataSizeSamples). +// +// |dst_af| will have its data and format members (sample rate, channels and +// samples per channel) set appropriately. No other members will be changed. +// TODO(ajm): For now, this still calls Reset() on |dst_af|. Remove this, as +// it shouldn't be needed. void DownConvertToCodecFormat(const int16_t* src_data, int samples_per_channel, int num_channels,