Clean up MapSetting().

- Add assert(false) for "impossible" cases.
- Remove tests for invalid enum values.
- Modify MapError() to look the same way.

BUG=
TEST=audioproc_unittest

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

git-svn-id: http://webrtc.googlecode.com/svn/trunk@1631 4adac7df-926f-26a2-2b94-8c16560cd09d

src/modules/audio_processing/echo_cancellation_impl.cc

@@ -33,22 +33,18 @@ WebRtc_Word16 MapSetting(EchoCancellation::SuppressionLevel level) {
     case EchoCancellation::kHighSuppression:
       return kAecNlpAggressive;
   }
-  // TODO(mflodman) Needed for gcc to compile and assert can't be added due to
+  assert(false);
-  // ApmTest triggers this.
   return -1;
 }
 
-int MapError(int err) {
+AudioProcessing::Error MapError(int err) {
   switch (err) {
     case AEC_UNSUPPORTED_FUNCTION_ERROR:
       return AudioProcessing::kUnsupportedFunctionError;
-      break;
     case AEC_BAD_PARAMETER_ERROR:
       return AudioProcessing::kBadParameterError;
-      break;
     case AEC_BAD_PARAMETER_WARNING:
       return AudioProcessing::kBadStreamParameterWarning;
-      break;
     default:
       // AEC_UNSPECIFIED_ERROR
       // AEC_UNINITIALIZED_ERROR

src/modules/audio_processing/echo_control_mobile_impl.cc

@@ -37,12 +37,11 @@ WebRtc_Word16 MapSetting(EchoControlMobile::RoutingMode mode) {
     case EchoControlMobile::kLoudSpeakerphone:
       return 4;
   }
-  // TODO(mflodman) Needed for gcc to compile and assert can't be added due to
+  assert(false);
-  // ApmTest triggers this.
   return -1;
 }
 
-int MapError(int err) {
+AudioProcessing::Error MapError(int err) {
   switch (err) {
     case AECM_UNSUPPORTED_FUNCTION_ERROR:
       return AudioProcessing::kUnsupportedFunctionError;

src/modules/audio_processing/gain_control_impl.cc

@@ -22,34 +22,17 @@ namespace webrtc {
 
 typedef void Handle;
 
-/*template <class T>
-class GainControlHandle : public ComponentHandle<T> {
-  public:
-    GainControlHandle();
-    virtual ~GainControlHandle();
-
-    virtual int Create();
-    virtual T* ptr() const;
-
-  private:
-    T* handle;
-};*/
-
 namespace {
 WebRtc_Word16 MapSetting(GainControl::Mode mode) {
   switch (mode) {
     case GainControl::kAdaptiveAnalog:
       return kAgcModeAdaptiveAnalog;
-      break;
     case GainControl::kAdaptiveDigital:
       return kAgcModeAdaptiveDigital;
-      break;
     case GainControl::kFixedDigital:
       return kAgcModeFixedDigital;
-      break;
   }
-  // TODO(mflodman) Needed for gcc to compile and assert can't be added due to
+  assert(false);
-  // ApmTest triggers this.
   return -1;
 }
 }  // namespace

src/modules/audio_processing/include/audio_processing.h

@@ -1,5 +1,5 @@
 /*
- *  Copyright (c) 2011 The WebRTC project authors. All Rights Reserved.
+ *  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
@@ -216,8 +216,8 @@ class AudioProcessing : public Module {
     int minimum;  // Long-term minimum.
   };
 
-  // Fatal errors.
+  enum Error {
-  enum Errors {
+    // Fatal errors.
     kNoError = 0,
     kUnspecifiedError = -1,
     kCreationFailedError = -2,
@@ -230,14 +230,12 @@ class AudioProcessing : public Module {
     kBadNumberChannelsError = -9,
     kFileError = -10,
     kStreamParameterNotSetError = -11,
-    kNotEnabledError = -12
+    kNotEnabledError = -12,
-  };
 
-  // Warnings are non-fatal.
+    // Warnings are non-fatal.
-  enum Warnings {
     // This results when a set_stream_ parameter is out of range. Processing
     // will continue, but the parameter may have been truncated.
-    kBadStreamParameterWarning = -13,
+    kBadStreamParameterWarning = -13
   };
 
   // Inherited from Module.

src/modules/audio_processing/noise_suppression_impl.cc

@@ -42,8 +42,7 @@ int MapSetting(NoiseSuppression::Level level) {
     case NoiseSuppression::kVeryHigh:
       return 3;
   }
-  // TODO(mflodman) Needed for gcc to compile and assert can't be added due to
+  assert(false);
-  // ApmTest triggers this.
   return -1;
 }
 }  // namespace

src/modules/audio_processing/test/unit_test.cc

@@ -573,14 +573,6 @@ TEST_F(ApmTest, EchoCancellation) {
         apm_->echo_cancellation()->device_sample_rate_hz());
   }
 
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->echo_cancellation()->set_suppression_level(
-          static_cast<EchoCancellation::SuppressionLevel>(-1)));
-
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->echo_cancellation()->set_suppression_level(
-          static_cast<EchoCancellation::SuppressionLevel>(4)));
-
   EchoCancellation::SuppressionLevel level[] = {
     EchoCancellation::kLowSuppression,
     EchoCancellation::kModerateSuppression,
@@ -631,13 +623,6 @@ TEST_F(ApmTest, EchoControlMobile) {
   EXPECT_EQ(apm_->kNoError, apm_->echo_control_mobile()->Enable(true));
   EXPECT_TRUE(apm_->echo_control_mobile()->is_enabled());
 
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->echo_control_mobile()->set_routing_mode(
-      static_cast<EchoControlMobile::RoutingMode>(-1)));
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->echo_control_mobile()->set_routing_mode(
-      static_cast<EchoControlMobile::RoutingMode>(5)));
-
   // Toggle routing modes
   EchoControlMobile::RoutingMode mode[] = {
       EchoControlMobile::kQuietEarpieceOrHeadset,
@@ -694,12 +679,6 @@ TEST_F(ApmTest, EchoControlMobile) {
 
 TEST_F(ApmTest, GainControl) {
   // Testing gain modes
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->gain_control()->set_mode(static_cast<GainControl::Mode>(-1)));
-
-  EXPECT_EQ(apm_->kBadParameterError,
-      apm_->gain_control()->set_mode(static_cast<GainControl::Mode>(3)));
-
   EXPECT_EQ(apm_->kNoError,
       apm_->gain_control()->set_mode(
       apm_->gain_control()->mode()));
@@ -795,18 +774,7 @@ TEST_F(ApmTest, GainControl) {
 }
 
 TEST_F(ApmTest, NoiseSuppression) {
-  // Tesing invalid suppression levels
+  // Test valid suppression levels.
-
-    // TODO(mflodman) Check at these failures.
-//  EXPECT_EQ(apm_->kBadParameterError,
-//      apm_->noise_suppression()->set_level(
-//          static_cast<NoiseSuppression::Level>(-1)));
-
-//  EXPECT_EQ(apm_->kBadParameterError,
-//      apm_->noise_suppression()->set_level(
-//          static_cast<NoiseSuppression::Level>(5)));
-
-  // Tesing valid suppression levels
   NoiseSuppression::Level level[] = {
     NoiseSuppression::kLow,
     NoiseSuppression::kModerate,
@@ -819,7 +787,7 @@ TEST_F(ApmTest, NoiseSuppression) {
     EXPECT_EQ(level[i], apm_->noise_suppression()->level());
   }
 
-  // Turing NS on/off
+  // Turn NS on/off
   EXPECT_EQ(apm_->kNoError, apm_->noise_suppression()->Enable(true));
   EXPECT_TRUE(apm_->noise_suppression()->is_enabled());
   EXPECT_EQ(apm_->kNoError, apm_->noise_suppression()->Enable(false));
@@ -827,7 +795,7 @@ TEST_F(ApmTest, NoiseSuppression) {
 }
 
 TEST_F(ApmTest, HighPassFilter) {
-  // Turing HP filter on/off
+  // Turn HP filter on/off
   EXPECT_EQ(apm_->kNoError, apm_->high_pass_filter()->Enable(true));
   EXPECT_TRUE(apm_->high_pass_filter()->is_enabled());
   EXPECT_EQ(apm_->kNoError, apm_->high_pass_filter()->Enable(false));
@@ -835,7 +803,7 @@ TEST_F(ApmTest, HighPassFilter) {
 }
 
 TEST_F(ApmTest, LevelEstimator) {
-  // Turning level estimator on/off
+  // Turn level estimator on/off
   EXPECT_EQ(apm_->kNoError, apm_->level_estimator()->Enable(false));
   EXPECT_FALSE(apm_->level_estimator()->is_enabled());
 
@@ -918,17 +886,7 @@ TEST_F(ApmTest, VoiceDetection) {
             apm_->voice_detection()->set_stream_has_voice(false));
   EXPECT_FALSE(apm_->voice_detection()->stream_has_voice());
 
-  // Tesing invalid likelihoods
+  // Test valid likelihoods
-  // TODO(mflodman) Check at these failures.
-//  EXPECT_EQ(apm_->kBadParameterError,
-//      apm_->voice_detection()->set_likelihood(
-//          static_cast<VoiceDetection::Likelihood>(-1)));
-
-//  EXPECT_EQ(apm_->kBadParameterError,
-//      apm_->voice_detection()->set_likelihood(
-//          static_cast<VoiceDetection::Likelihood>(5)));
-
-  // Tesing valid likelihoods
   VoiceDetection::Likelihood likelihood[] = {
       VoiceDetection::kVeryLowLikelihood,
       VoiceDetection::kLowLikelihood,
@@ -942,11 +900,11 @@ TEST_F(ApmTest, VoiceDetection) {
   }
 
   /* TODO(bjornv): Enable once VAD supports other frame lengths than 10 ms
-  // Tesing invalid frame sizes
+  // Test invalid frame sizes
   EXPECT_EQ(apm_->kBadParameterError,
       apm_->voice_detection()->set_frame_size_ms(12));
 
-  // Tesing valid frame sizes
+  // Test valid frame sizes
   for (int i = 10; i <= 30; i += 10) {
     EXPECT_EQ(apm_->kNoError,
         apm_->voice_detection()->set_frame_size_ms(i));
@@ -954,7 +912,7 @@ TEST_F(ApmTest, VoiceDetection) {
   }
   */
 
-  // Turing VAD on/off
+  // Turn VAD on/off
   EXPECT_EQ(apm_->kNoError, apm_->voice_detection()->Enable(true));
   EXPECT_TRUE(apm_->voice_detection()->is_enabled());
   EXPECT_EQ(apm_->kNoError, apm_->voice_detection()->Enable(false));

src/modules/audio_processing/voice_detection_impl.cc

@@ -27,24 +27,18 @@ int MapSetting(VoiceDetection::Likelihood likelihood) {
   switch (likelihood) {
     case VoiceDetection::kVeryLowLikelihood:
       return 3;
-      break;
     case VoiceDetection::kLowLikelihood:
       return 2;
-      break;
     case VoiceDetection::kModerateLikelihood:
       return 1;
-      break;
     case VoiceDetection::kHighLikelihood:
       return 0;
-      break;
   }
-  // TODO(mflodman) Needed for gcc to compile and assert can't be added due to
+  assert(false);
-  // ApmTest triggers this.
   return -1;
 }
 }  // namespace
 
-
 VoiceDetectionImpl::VoiceDetectionImpl(const AudioProcessingImpl* apm)
   : ProcessingComponent(apm),
     apm_(apm),