Only split into bands when the reverse stream is analyzed in the APM

webrtc/modules/audio_processing/audio_processing_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 1128 matching lines @@
     // have super-wb.
     return true;
   }
   return false;
 }
 
 bool AudioProcessingImpl::is_rev_processed() const {
   return constants_.intelligibility_enabled;
 }
 
+bool AudioProcessingImpl::is_rev_analyzed() const {
+  return is_rev_processed() ||

[peah-webrtc, 2016/03/30 13:01:11]

As I see it, the purpose of this method is to avoid having to do reverse stream
synthesis when it is not necessary, right? That is a great change indeed.

I think this is an excellent time to simplify this scheme.

Currently we have:
-fwd_analysis_needed()
-fwd_synthesis_needed()
-rev_synthesis_needed()
-rev_analysis_needed()
-is_data_processed()
-output_copy_needed()
-is_rev_processed() 
-is_rev_analyzed() 
Which basically all have the purpose of determining whether 
-an output copy is needed for the stream, 
-whether a synthesis needs to be done.
-whether an analysis needs to be done.

It should be possible to simplify this quite a lot. 
For instance:
-It would make sense to have matching name to their reverse stream counterparts
for the methods:
--is_data_processed()
--output_copy_needed() 
-The methods rev_analysis_needed() and is_rev_analyzed() do different things,
but have names which indicate that they are exactly the same thing (there should
be no case where the reverse stream has not been analyzed even though it has
been flagged that it needs to be analyzed).
-is_rev_processed indicates that there is only reverse side processing being
done if that is true. That is, however, not really the case as the
band-splitting filter merge needed for the other components also modifies the
stream. Therefore it would make sense to bundle these methods into one method.

[aluebs-webrtc, 2016/03/31 01:00:18]

No, this change is to avoid doing reverse stream analysis when it is not
necessary.
I think that rev_analysis_needed and is_rev_analyzed were easier to read in
different functions, but the naming was probably chosen poorly by me. Anyway, I
unified them, since the new function was only used in one place.
I definitively agree that we can simplify these functions, but I am hesitant to
mix that kind of refactoring into a CL that is supposed to change the behavior,
like this one. Although renaming I think is ok, there is no reverse counterpart
to output_copy_needed, because it is not as simple as you describe it.
Unfortunately, because the AnalyzeReverseStream API is not deprecated yet, it we
need to keep track manually of the output_copy and conversions needed.
is_rev_processed indicates there is reverse processing done, but not necessarily
only processing or not only in the reverse stream. And the merging would not
happen if no processing was done, so it still holds up.

+         public_submodules_->echo_cancellation->is_enabled() ||
+         public_submodules_->echo_control_mobile->is_enabled() ||
+         public_submodules_->gain_control->is_enabled();
+}
+
 bool AudioProcessingImpl::rev_synthesis_needed() const {
   return (is_rev_processed() &&
           is_multi_band(formats_.rev_proc_format.sample_rate_hz()));
 }
 
 bool AudioProcessingImpl::rev_analysis_needed() const {
-  return is_multi_band(formats_.rev_proc_format.sample_rate_hz());
+  return is_rev_analyzed() &&
+         is_multi_band(formats_.rev_proc_format.sample_rate_hz());
 }
 
 bool AudioProcessingImpl::render_check_rev_conversion_needed() const {
   return rev_conversion_needed();
 }
 
 bool AudioProcessingImpl::rev_conversion_needed() const {
   return (formats_.api_format.reverse_input_stream() !=
           formats_.api_format.reverse_output_stream());
 }
@@ skipping 274 matching lines @@
   debug_dump_.capture.event_msg->mutable_config()->CopyFrom(config);
 
   RETURN_ON_ERR(WriteMessageToDebugFile(debug_dump_.debug_file.get(),
                                         &debug_dump_.num_bytes_left_for_log_,
                                         &crit_debug_, &debug_dump_.capture));
   return kNoError;
 }
 #endif  // WEBRTC_AUDIOPROC_DEBUG_DUMP
 
 }  // namespace webrtc