APM QA refactoring: render stream support, echo path simulation, new export engine

webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py

Index: webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py
diff --git a/webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py b/webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py
index 3c618b460581a06f2b5e223974970879447bd747..614c4998bc8bd48bfca778fb7ca211eb968a9bbb 100644
--- a/webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py
+++ b/webrtc/modules/audio_processing/test/py_quality_assessment/quality_assessment/audioproc_wrapper.py
@@ -22,7 +22,8 @@ class AudioProcWrapper(object):
   """
 
   OUTPUT_FILENAME = 'output.wav'
-  _AUDIOPROC_F_BIN_PATH = os.path.abspath('../audioproc_f')
+  _AUDIOPROC_F_BIN_PATH = os.path.abspath(os.path.join(

[peah-webrtc, 2017/04/21 05:28:45]

What does this mean? Is the location of audioproc_f hardcoded? Would it not be
better to have it either as part of the searchpath, and/or allow a specific path
to be specified on the command line?

[AleBzk, 2017/04/24 14:30:41]

True, that would be better, but I didn't want to add too much boilerplate.
Instead, I made audioproc_f a dependency (see the BUILD file).
Since we know the relative path between audioproc_f and this wrapper Python
file, I see safe leaving everything as it is now. WDYT?

[peah-webrtc, 2017/05/03 08:10:07]

Sounds good!

+      os.pardir, 'audioproc_f'))
 
   def __init__(self):
     self._config = None
@@ -36,14 +37,17 @@ class AudioProcWrapper(object):
   def output_filepath(self):
     return self._output_signal_filepath
 
-  def Run(self, config_filepath, input_filepath, output_path):
+  def Run(self, config_filepath, input_filepath, output_path,
+          reverse_input_filepath=None):

[peah-webrtc, 2017/04/21 05:28:45]

I'd prefer if capture and render is used instead of forward and reverse. I think
those make more sense, and the plan is to change the code to use that naming
instead.

[AleBzk, 2017/04/24 14:30:41]

Done, including changes in the rest of the APM QA tool.

As discussed offline, I won't touch audioproc_f, which currently uses the
forward/reverse naming convention.
Switching to the capture/render naming convention may break things, hence
changes in audioproc_f are left for separate CLs.

     """Run audioproc_f.
 
     Args:
       config_filepath: path to the configuration file specifing the arguments
                        for audioproc_f.
-      input_filepath: path to the audio track input file.
+      input_filepath: path to the forward audio track input file (aka near-end).
       output_path: path of the audio track output file.
+      reverse_input_filepath: path to the reverse audio track input file (aka
+                              far-end).
     """
     # Init.
     self._input_signal_filepath = input_filepath
@@ -62,6 +66,8 @@ class AudioProcWrapper(object):
     # Set remaining parametrs.
     self._config['-i'] = self._input_signal_filepath
     self._config['-o'] = self._output_signal_filepath
+    if reverse_input_filepath is not None:

[peah-webrtc, 2017/04/21 05:28:45]

Can't this be simplified to
 if reverse_input_filepath:

?

[AleBzk, 2017/04/24 14:30:41]

Personally I don't like implicit casts in this case, but what you suggest also
covers the case of an empty string.
Let's do this: I will improve the code checking if the file exists and raising
an exception if it doesn't.

[peah-webrtc, 2017/05/03 08:10:07]

Sounds good! I'm definitely not a python Guru but I have the impression that the
check falsy (not false) values is very common. 
Both variants are fine with me though.

+      self._config['-ri'] = reverse_input_filepath
 
     # Build arguments list.
     args = [self._AUDIOPROC_F_BIN_PATH]