Refactor ParseArrayGeometry to use standard Chromium facilities.

content/renderer/media/media_stream_audio_processor.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/media_stream_audio_processor.h"
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/trace_event/trace_event.h"
@@ skipping 75 matching lines @@
 
   return (group_name == "Enabled" || group_name == "DefaultEnabled");
 }
 
 bool IsBeamformingEnabled(const MediaAudioConstraints& audio_constraints) {
   return base::FieldTrialList::FindFullName("ChromebookBeamforming") ==
              "Enabled" ||
          audio_constraints.GetProperty(MediaAudioConstraints::kGoogBeamforming);
 }
 
+void ConfigureBeamforming(webrtc::Config* config,
+                          const std::string& geometry_str) {
+  std::vector<webrtc::Point> geometry = ParseArrayGeometry(geometry_str);
+#if defined(OS_CHROMEOS)
+  if (geometry.size() == 0) {

[emircan, 2015/08/03 18:06:22]

std::vector::empty() can be used here.

[ajm, 2015/08/04 01:16:35]

Done.

+    const std::string board = base::SysInfo::GetLsbReleaseBoard();

[emircan, 2015/08/03 18:06:22]

const std::string& board can avoid a copy.

[ajm, 2015/08/04 01:16:35]

I don't think this can avoid a copy. GetLsbReleaseBoard() returns a std::string
by value. Assigning to a reference here must bind it to a temporary. Whether a
copy occurs comes down to whether RVO takes place or not in the construction of
that temporary.

Do you agree?

+    if (board.find("nyan_kitty") != std::string::npos) {
+      geometry.push_back(webrtc::Point(-0.03f, 0.f, 0.f));
+      geometry.push_back(webrtc::Point(0.03f, 0.f, 0.f));
+    } else if (board.find("peach_pi") != std::string::npos) {
+      geometry.push_back(webrtc::Point(-0.025f, 0.f, 0.f));
+      geometry.push_back(webrtc::Point(0.025f, 0.f, 0.f));
+    } else if (board.find("samus") != std::string::npos) {
+      geometry.push_back(webrtc::Point(-0.032f, 0.f, 0.f));
+      geometry.push_back(webrtc::Point(0.032f, 0.f, 0.f));
+    } else if (board.find("swanky") != std::string::npos) {
+      geometry.push_back(webrtc::Point(-0.026f, 0.f, 0.f));
+      geometry.push_back(webrtc::Point(0.026f, 0.f, 0.f));
+    }
+  }
+#endif
+  config->Set<webrtc::Beamforming>(
+      new webrtc::Beamforming(geometry.size() > 1, geometry));
+}
+
 }  // namespace
 
 // Wraps AudioBus to provide access to the array of channel pointers, since this
 // is the type webrtc::AudioProcessing deals in. The array is refreshed on every
 // channel_ptrs() call, and will be valid until the underlying AudioBus pointers
 // are changed, e.g. through calls to SetChannelData() or SwapChannels().
 //
 // All methods are called on one of the capture or render audio threads
 // exclusively.
 class MediaStreamAudioBus {
@@ skipping 371 matching lines @@
 
   // Experimental options provided at creation.
   webrtc::Config config;
   if (goog_experimental_aec)
     config.Set<webrtc::ExtendedFilter>(new webrtc::ExtendedFilter(true));
   if (goog_experimental_ns)
     config.Set<webrtc::ExperimentalNs>(new webrtc::ExperimentalNs(true));
   if (IsDelayAgnosticAecEnabled())
     config.Set<webrtc::DelayAgnostic>(new webrtc::DelayAgnostic(true));
   if (goog_beamforming) {
-    ConfigureBeamforming(&config, audio_constraints.GetPropertyAsString(
-        MediaAudioConstraints::kGoogArrayGeometry));
+    ConfigureBeamforming(&config,
+                         audio_constraints.GetPropertyAsString(
+                             MediaAudioConstraints::kGoogArrayGeometry));
   }
 
   // Create and configure the webrtc::AudioProcessing.
   audio_processing_.reset(webrtc::AudioProcessing::Create(config));
 
   // Enable the audio processing components.
   if (echo_cancellation) {
     EnableEchoCancellation(audio_processing_.get());
 
     if (playout_data_source_)
@@ skipping 23 matching lines @@
     typing_detector_.reset(new webrtc::TypingDetection());
     EnableTypingDetection(audio_processing_.get(), typing_detector_.get());
   }
 
   if (goog_agc)
     EnableAutomaticGainControl(audio_processing_.get());
 
   RecordProcessingState(AUDIO_PROCESSING_ENABLED);
 }
 
-void MediaStreamAudioProcessor::ConfigureBeamforming(
-    webrtc::Config* config,
-    const std::string& geometry_str) const {
-  std::vector<webrtc::Point> geometry = ParseArrayGeometry(geometry_str);
-#if defined(OS_CHROMEOS)
-  if (geometry.size() == 0) {
-    const std::string board = base::SysInfo::GetLsbReleaseBoard();
-    if (board.find("nyan_kitty") != std::string::npos) {
-      geometry.push_back(webrtc::Point(-0.03f, 0.f, 0.f));
-      geometry.push_back(webrtc::Point(0.03f, 0.f, 0.f));
-    } else if (board.find("peach_pi") != std::string::npos) {
-      geometry.push_back(webrtc::Point(-0.025f, 0.f, 0.f));
-      geometry.push_back(webrtc::Point(0.025f, 0.f, 0.f));
-    } else if (board.find("samus") != std::string::npos) {
-      geometry.push_back(webrtc::Point(-0.032f, 0.f, 0.f));
-      geometry.push_back(webrtc::Point(0.032f, 0.f, 0.f));
-    } else if (board.find("swanky") != std::string::npos) {
-      geometry.push_back(webrtc::Point(-0.026f, 0.f, 0.f));
-      geometry.push_back(webrtc::Point(0.026f, 0.f, 0.f));
-    }
-  }
-#endif
-  config->Set<webrtc::Beamforming>(new webrtc::Beamforming(geometry.size() > 1,
-                                                           geometry));
-}
-
-std::vector<webrtc::Point> MediaStreamAudioProcessor::ParseArrayGeometry(
-    const std::string& geometry_str) const {
-  std::vector<webrtc::Point> result;
-  std::vector<float> values;
-  std::istringstream str(geometry_str);
-  std::copy(std::istream_iterator<float>(str),
-            std::istream_iterator<float>(),
-            std::back_inserter(values));
-  if (values.size() % 3 == 0) {
-    for (size_t i = 0; i < values.size(); i += 3) {
-      result.push_back(webrtc::Point(values[i + 0],
-                                     values[i + 1],
-                                     values[i + 2]));
-    }
-  }
-  return result;
-}
-
 void MediaStreamAudioProcessor::InitializeCaptureFifo(
     const media::AudioParameters& input_format) {
   DCHECK(main_thread_checker_.CalledOnValidThread());
   DCHECK(input_format.IsValid());
   input_format_ = input_format;
 
   // TODO(ajm): For now, we assume fixed parameters for the output when audio
   // processing is enabled, to match the previous behavior. We should either
   // use the input parameters (in which case, audio processing will convert
   // at output) or ideally, have a backchannel from the sink to know what
@@ skipping 140 matching lines @@
   if (echo_information_) {
     echo_information_.get()->UpdateAecDelayStats(ap->echo_cancellation());
   }
 
   // Return 0 if the volume hasn't been changed, and otherwise the new volume.
   return (agc->stream_analog_level() == volume) ?
       0 : agc->stream_analog_level();
 }
 
 }  // namespace content