Parallel iterator bash script for the quality assessment package.

Parallel iterator bash script. The Python files are empty ones, just a placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
NOTRY=True

Review-Url: https://codereview.webrtc.org/2711923002
Cr-Commit-Position: refs/heads/master@{#16827}
Committed: https://chromium.googlesource.com/external/webrtc/+/306d1bfec0d3dc11d770a513cff3de941a796553

[AleBzk, 2017-02-23 13:50:12]

Description was changed from

==========
Parallel iterator bash script, quality assessment package, scripts.


Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts


Sample audio files for the APM quality assessment toolbox

BUG=
==========

to

==========
Parallel iterator bash script, quality assessment package, scripts.

Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts

Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

[AleBzk, 2017-02-23 13:50:12]

alessiob@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[AleBzk, 2017-02-23 13:51:18]

Description was changed from

==========
Parallel iterator bash script, quality assessment package, scripts.

Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts

Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

to

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.

Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts

Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

[AleBzk, 2017-02-23 13:52:57]

Hi Henrik,
This is another compact CL. The Python files are empty, the main thing here are
the new build file, the single extra line in the existing audio_processing build
file, and the bash script that calls the Python ones.

[aleloi, 2017-02-23 14:48:24]

aleloi@webrtc.org changed reviewers:
+ aleloi@webrtc.org

[aleloi, 2017-02-23 14:48:25]

Drive-by comments

I've stubbornly refused to learn bash, so not many comments on the shell script.

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/BUILD.gn (right):

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/BUILD.gn:436:
"test/py_quality_assessment:py_quality_assessment",
Just "test/py_quality_assessment" is enough. GN interprets it as
'test/py_quality_assessment:py_quality_assessment'. Syntax sugar or something.

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/test/py_quality_assessment/BUILD.gn
(right):

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/BUILD.gn:15: ]
I suggest adding 'visibility = [ ":" ]' (hope the syntax is correct) here and in
:apm_configs. It means that the targets cannot be depended upon outside of this
file.

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/BUILD.gn:34:
copy("py_quality_assessment") {
Please move this to the beginning of the file.

See
https://chromium.googlesource.com/chromium/src/+/master/tools/gn/docs/style_g...


Extract:

'Most BUILD files should have a target with the same name of the directory. This
target should be the first target.'

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
File webrtc/modules/audio_processing/test/py_quality_assessment/README (right):

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/README:9: reference
one used for evaluation.
I suggest renaming into README.md and rewriting in markdown. Then the Gitiles
code browser will format it better.

Compare https://chromium.googlesource.com/external/webrtc/+/master/ (the readme
is displayed)
and https://chromium.googlesource.com/external/webrtc/+/master/tools-webrtc/cpu/
(it's not).


Also, perhaps add a note that all the code hasn't landed yet?

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
File
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh
(right):

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:22:
OUTPUT_PATH=~/apm_quality_assessment-example
I don't know if it's good practice to place output in the home directory. No
strong opinions either way.

https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:63:
google-chrome "file://${RESULTS_FILE}" > /dev/null 2>&1 &
I think 'sensible-browser' is better. We shouldn't force Chrome on everyone.
Also because it's not called google-chrome on all systems.

[AleBzk, 2017-02-24 09:08:30]

Hi Alex,

thanks a lot!

I addressed all your comments except the following (for which I provided an
answer):
https://codereview.webrtc.org/2711923002/diff/20001/webrtc/modules/audio_proc...

Alessio

[hlundin-webrtc, 2017-02-24 09:57:44]

Nice. I have a few comments/questions.

https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
File
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh
(right):

https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:10:
# Customize probing signals, noise sources and scores if needed.
Does this script assume that your cwd is something particular?

https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:22:
OUTPUT_PATH=~/apm_quality_assessment-example
I'm not sure I like that this is written outside of the repository tree. Can you
create this in the out folder?

[aleloi, 2017-02-24 10:05:08]

lgtm on BUILD files and general structure. The bash also looks fine to me, but I
don't know enough to say anything meaningful.

[AleBzk, 2017-02-24 10:37:29]

Thanks so much! I addressed the output path comment, while for the working path
I will add more details in README.md in one of the next CLs.

On 2017/02/24 09:57:44, hlundin-webrtc wrote:
> Nice. I have a few comments/questions.
> 
>
https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
> File
>
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh
> (right):
> 
>
https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
>
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:10:
> # Customize probing signals, noise sources and scores if needed.
> Does this script assume that your cwd is something particular?
> 
>
https://codereview.webrtc.org/2711923002/diff/40001/webrtc/modules/audio_proc...
>
webrtc/modules/audio_processing/test/py_quality_assessment/apm_quality_assessment.sh:22:
> OUTPUT_PATH=~/apm_quality_assessment-example
> I'm not sure I like that this is written outside of the repository tree. Can
you
> create this in the out folder?

[hlundin-webrtc, 2017-02-24 11:47:43]

lgtm

[aleloi, 2017-02-24 11:56:53]

Minor thing: Can you please update the CL description? In particular, remove
this line:

"Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts
"

[AleBzk, 2017-02-24 12:23:28]

Description was changed from

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.

Merge branch 'master' of https://chromium.googlesource.com/external/webrtc into
apm_qa-scripts

Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

to

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

[AleBzk, 2017-02-24 12:23:44]

The CQ bit was checked by alessiob@webrtc.org

[AleBzk, 2017-02-24 12:23:45]

The patchset sent to the CQ was uploaded after l-g-t-m from aleloi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2711923002/#ps80001 (title:
"README file for the default output folder")

[commit-bot: I haz the power, 2017-02-24 12:23:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[AleBzk, 2017-02-24 12:44:59]

Description was changed from

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=7218
==========

to

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
==========

[commit-bot: I haz the power, 2017-02-24 12:50:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-24 12:50:55]

Try jobs failed on following builders:
  linux_tsan2 on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/20175)

[AleBzk, 2017-02-24 12:56:37]

The CQ bit was checked by alessiob@webrtc.org

[commit-bot: I haz the power, 2017-02-24 12:56:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[hlundin-webrtc, 2017-02-24 13:03:18]

Description was changed from

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
==========

to

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
NOTRY=True
==========

[commit-bot: I haz the power, 2017-02-24 13:32:24]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1487940997775230,
"parent_rev": "7a1798d637ee34f84c3dc1b0a70ad5a62ab7ae9b", "commit_rev":
"306d1bfec0d3dc11d770a513cff3de941a796553"}

[commit-bot: I haz the power, 2017-02-24 13:32:26]

Description was changed from

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
NOTRY=True
==========

to

==========
Parallel iterator bash script. The Python files are empty ones, just a
placeholder for the code they will host.
Sample audio files for the APM quality assessment toolbox

BUG=webrtc:7218
NOTRY=True

Review-Url: https://codereview.webrtc.org/2711923002
Cr-Commit-Position: refs/heads/master@{#16827}
Committed:
https://chromium.googlesource.com/external/webrtc/+/306d1bfec0d3dc11d770a513c...
==========

[commit-bot: I haz the power, 2017-02-24 13:32:27]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/306d1bfec0d3dc11d770a513c...