Fixed the render queue item size preallocation and verification, moved constant declarations, remov…

webrtc/modules/audio_processing/echo_cancellation_impl.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 37 matching lines @@
       return AudioProcessing::kBadStreamParameterWarning;
     default:
       // AEC_UNSPECIFIED_ERROR
       // AEC_UNINITIALIZED_ERROR
       // AEC_NULL_POINTER_ERROR
       return AudioProcessing::kUnspecifiedError;
   }
 }
 }  // namespace
 
-const size_t EchoCancellationImpl::kAllowedValuesOfSamplesPerFrame1;
-const size_t EchoCancellationImpl::kAllowedValuesOfSamplesPerFrame2;
-
 EchoCancellationImpl::EchoCancellationImpl(const AudioProcessing* apm,
                                            CriticalSectionWrapper* crit)
     : ProcessingComponent(),
       apm_(apm),
       crit_(crit),
       drift_compensation_enabled_(false),
       metrics_enabled_(false),
       suppression_level_(kModerateSuppression),
       stream_drift_samples_(0),
       was_stream_drift_set_(false),
       stream_has_echo_(false),
       delay_logging_enabled_(false),
       extended_filter_enabled_(false),
       delay_agnostic_enabled_(false),
-      render_queue_element_max_size_(0) {
-  AllocateRenderQueue();
-}
+      render_queue_element_max_size_(0) {}
 
 EchoCancellationImpl::~EchoCancellationImpl() {}
 
 int EchoCancellationImpl::ProcessRenderAudio(const AudioBuffer* audio) {
   if (!is_component_enabled()) {
     return apm_->kNoError;
   }
 
   assert(audio->num_frames_per_band() <= 160);
   assert(audio->num_channels() == apm_->num_reverse_channels());
@@ skipping 289 matching lines @@
   if (err != apm_->kNoError || !is_component_enabled()) {
     return err;
   }
 
   AllocateRenderQueue();
 
   return apm_->kNoError;
 }
 
 void EchoCancellationImpl::AllocateRenderQueue() {
-  const size_t max_frame_size = std::max<size_t>(
-      kAllowedValuesOfSamplesPerFrame1, kAllowedValuesOfSamplesPerFrame2);
+  const size_t max_frame_size =
+      std::max<size_t>(static_cast<size_t>(kAllowedValuesOfSamplesPerFrame1),

[the sun, 2015/11/17 13:44:37]

It seems wasteful to me to do this at runtime. Why not keep one constant (in the
anonymous namespace above) and add a comment that it is the larger of the two
possbilities?

[peah-webrtc, 2015/11/18 05:53:46]

Good point! 

Done.

+                       static_cast<size_t>(kAllowedValuesOfSamplesPerFrame2));
 
   const size_t new_render_queue_element_max_size = std::max<size_t>(
       static_cast<size_t>(1), max_frame_size * num_handles_required());
 
   // Reallocate the queue if the queue item size is too small to fit the
   // data to put in the queue.
   if (new_render_queue_element_max_size > render_queue_element_max_size_) {

[the sun, 2015/11/17 13:44:37]

super nit: it's a little nicer to keep the operands in the same order as the
assignment below.

[peah-webrtc, 2015/11/18 05:53:46]

Agree. And the comparison is also more readable in itself done in that
direction.

Done.

     render_queue_element_max_size_ = new_render_queue_element_max_size;
 
     std::vector<float> template_queue_element(render_queue_element_max_size_);
 
     render_signal_queue_.reset(
         new SwapQueue<std::vector<float>, RenderQueueItemVerifier<float>>(
             kMaxNumFramesToBuffer, template_queue_element,

[the sun, 2015/11/17 13:44:37]

Move kMaxNumFramesToBuffer into the anonymous namespace above instead.

[peah-webrtc, 2015/11/18 05:53:46]

Done.

             RenderQueueItemVerifier<float>(render_queue_element_max_size_)));
+
+    render_queue_buffer_.resize(render_queue_element_max_size_);
+    capture_queue_buffer_.resize(render_queue_element_max_size_);
   } else {
     render_signal_queue_->Clear();
   }
-
-  render_queue_buffer_.resize(new_render_queue_element_max_size);
-  capture_queue_buffer_.resize(new_render_queue_element_max_size);
 }
 
 void EchoCancellationImpl::SetExtraOptions(const Config& config) {
   extended_filter_enabled_ = config.Get<ExtendedFilter>().enabled;
   delay_agnostic_enabled_ = config.Get<DelayAgnostic>().enabled;
   Configure();
 }
 
 void* EchoCancellationImpl::CreateHandle() const {
   return WebRtcAec_Create();
@@ skipping 34 matching lines @@
 int EchoCancellationImpl::num_handles_required() const {
   return apm_->num_output_channels() *
          apm_->num_reverse_channels();
 }
 
 int EchoCancellationImpl::GetHandleError(void* handle) const {
   assert(handle != NULL);
   return AudioProcessing::kUnspecifiedError;
 }
 }  // namespace webrtc