Added offline data logpoints and logging functionality to the gain controller

In order to be able to analyze the AGC behavior on
aecdump recordings in an efficient manner, it is
important to be able to use a standardized analysis
script. For this to be feasible, data log points should
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564
Committed: https://crrev.com/135259ac8fac1fd88e948742e6ab473425a43d88
Cr-Commit-Position: refs/heads/master@{#14812}

[peah-webrtc, 2016-10-28 08:33:24]

Description was changed from

==========
Added offline data logpoints and logging functionality to the gain controller

In order to be able to analyze the AGC behavior on aecdump recordings in an
efficient manner, it is important to be able to use a standardized analysis
script. For this to be feasible, data log points should be present.

This CL adds those logpoints as well as the framework needed to for those
to work.

BUG=webrtc:6564
==========

to

==========
In order to be able to analyze the AGC behavior on 
aecdump recordings in an efficient manner, it is 
important to be able to use a standardized analysis
script. For this to be feasible, data log points should 
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564
==========

[peah-webrtc, 2016-10-28 08:37:49]

peah@webrtc.org changed reviewers:
+ ivoc@webrtc.org

[peah-webrtc, 2016-10-28 08:37:50]

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/audio_processing_impl.cc (right):

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/audio_processing_impl.cc:446: 
I moved the initialization of the gain_control to be closer to the
initialization of other gain control functionality to make the code easier to
read. This has no functional impact though.

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/audio_processing_impl.cc:459:
public_submodules_->gain_control_for_experimental_agc->Initialize();
This is needed to ensure that a new set of data dumps are created when the APM
is re-initialized.

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/gain_control_for_experimental_agc.h
(right):

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/gain_control_for_experimental_agc.h:40:
~GainControlForExperimentalAgc() override;
This was required by the compiler as this CL adds dynamically allocated data.

[peah-webrtc, 2016-10-28 08:50:38]

Patchset #2 (id:20001) has been deleted

[ivoc, 2016-10-28 09:20:10]

LGTM with two questions below.

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/gain_control_impl.cc (right):

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/gain_control_impl.cc:94: data_dumper_(new
ApmDataDumper(instance_counter_)),
The experimental AGC and the regular AGC could end up using the same values for
the instance_counter_, but I guess they're not active at the same time, are
they?

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/gain_control_impl.cc:246:
data_dumper_->DumpRaw("gain_control_set_stream_analog_level", 1, &level);
Shouldn't there be a way to enable/disable this logging in the regular AGC?

[peah-webrtc, 2016-10-28 09:43:41]

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
File webrtc/modules/audio_processing/gain_control_impl.cc (right):

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/gain_control_impl.cc:94: data_dumper_(new
ApmDataDumper(instance_counter_)),
On 2016/10/28 09:20:10, ivoc wrote:
> The experimental AGC and the regular AGC could end up using the same values
for
> the instance_counter_, but I guess they're not active at the same time, are
> they?

They are not in practice active at the same time.
I did, however, for the reason you state name the log files differently in the
experimental and the regular AGC so there will never be any clash.

https://codereview.webrtc.org/2457783003/diff/1/webrtc/modules/audio_processi...
webrtc/modules/audio_processing/gain_control_impl.cc:246:
data_dumper_->DumpRaw("gain_control_set_stream_analog_level", 1, &level);
On 2016/10/28 09:20:10, ivoc wrote:
> Shouldn't there be a way to enable/disable this logging in the regular AGC?

There is. What happens is that this method call expands into an empty method
call if the WEBRTC_APM_DEBUG_DUMP flag is not set. This build flag is only set
to 1 if apm_debug_dump=true when generating the build flags.

Therefore this logging is disabled even when using WebRTC in the default way.

Note that the experimental AGC is actually the standard analog AGC that we use.
That naming is very misleading.

[peah-webrtc, 2016-10-28 09:43:46]

The CQ bit was checked by peah@webrtc.org

[commit-bot: I haz the power, 2016-10-28 09:43:57]

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

[commit-bot: I haz the power, 2016-10-28 10:12:13]

Description was changed from

==========
In order to be able to analyze the AGC behavior on 
aecdump recordings in an efficient manner, it is 
important to be able to use a standardized analysis
script. For this to be feasible, data log points should 
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564
==========

to

==========
In order to be able to analyze the AGC behavior on 
aecdump recordings in an efficient manner, it is 
important to be able to use a standardized analysis
script. For this to be feasible, data log points should 
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564
==========

[commit-bot: I haz the power, 2016-10-28 10:12:14]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-10-28 10:12:25]

Description was changed from

==========
In order to be able to analyze the AGC behavior on 
aecdump recordings in an efficient manner, it is 
important to be able to use a standardized analysis
script. For this to be feasible, data log points should 
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564
==========

to

==========
In order to be able to analyze the AGC behavior on
aecdump recordings in an efficient manner, it is
important to be able to use a standardized analysis
script. For this to be feasible, data log points should
be present.

This CL adds those logpoints as well as the framework
needed to for those to work.

BUG=webrtc:6564

Committed: https://crrev.com/135259ac8fac1fd88e948742e6ab473425a43d88
Cr-Commit-Position: refs/heads/master@{#14812}
==========

[commit-bot: I haz the power, 2016-10-28 10:12:25]

Patchset 1 (id:??) landed as
https://crrev.com/135259ac8fac1fd88e948742e6ab473425a43d88
Cr-Commit-Position: refs/heads/master@{#14812}