Simplify webrtc::voe::MonitorModule and remove the .cc file.

Simplify webrtc::voe::MonitorModule and remove the .cc file.
The class basically implements a timer and can be replaced with a PostDelayedTask call down the line.

BUG=none

Review-Url: https://codereview.webrtc.org/2722613002
Cr-Commit-Position: refs/heads/master@{#16891}
Committed: https://chromium.googlesource.com/external/webrtc/+/b1175bb101a3c9682db929ae476732f4c43fb9a4

[tommi, 2017-02-27 18:19:48]

tommi@webrtc.org changed reviewers:
+ henrika@webrtc.org

[tommi, 2017-02-27 18:19:54]

The CQ bit was checked by tommi@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-27 18:20:03]

Dry run: 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, 2017-02-27 18:43:26]

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

[commit-bot: I haz the power, 2017-02-27 18:43:27]

Dry run: This issue passed the CQ dry run.

[henrika_webrtc, 2017-02-28 08:24:21]

Nice cleanup.

LGTM % the fact that it might even be possible to remove the class completely.

https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
File webrtc/voice_engine/monitor_module.h (right):

https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
webrtc/voice_engine/monitor_module.h:30: void Process() override {
observer_->OnPeriodicProcess(); }
To me it looks like TransmitMixer::OnPeridodicProcess contains e.g
VE_TYPING_NOISE_OFF_WARNING and VE_SATURATION_WARNING. Instead of improving this
class even more (by using delayed PostTask), it might be worth considering if it
can be removed completely. Henrik L should know if any of the callbacks are used
today.

[tommi, 2017-02-28 08:38:39]

Add TODO

[tommi, 2017-02-28 08:38:43]

https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
File webrtc/voice_engine/monitor_module.h (right):

https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
webrtc/voice_engine/monitor_module.h:30: void Process() override {
observer_->OnPeriodicProcess(); }
On 2017/02/28 08:24:21, henrika_webrtc wrote:
> To me it looks like TransmitMixer::OnPeridodicProcess contains e.g
> VE_TYPING_NOISE_OFF_WARNING and VE_SATURATION_WARNING. Instead of improving
this
> class even more (by using delayed PostTask), it might be worth considering if
it
> can be removed completely. Henrik L should know if any of the callbacks are
used
> today.

Good point... I looked around for VE_SATURATION_WARNING and all I can find is
this:

    printf("  SATURATION WARNING \n");

in voe_cmd_test.cc

It would be good to delete this if it's not being used.  We'd get rid of one
lock inside of TransmitMixer::ProcessAudio.  Added a TODO for now in this file,
with details.

[tommi, 2017-02-28 08:39:08]

The CQ bit was checked by tommi@webrtc.org

[tommi, 2017-02-28 08:39:08]

The patchset sent to the CQ was uploaded after l-g-t-m from henrika@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2722613002/#ps20001 (title:
"Add TODO")

[commit-bot: I haz the power, 2017-02-28 08:39:13]

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, 2017-02-28 08:56:24]

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

[commit-bot: I haz the power, 2017-02-28 08:56:25]

Try jobs failed on following builders:
  linux_tsan2 on master.tryserver.webrtc (JOB_FAILED, no build URL)

[tommi, 2017-02-28 08:59:47]

The CQ bit was checked by tommi@webrtc.org

[commit-bot: I haz the power, 2017-02-28 08:59:55]

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, 2017-02-28 09:16:49]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1488272387837710,
"parent_rev": "985371bda999c6db51286586c5850d2ff58f3511", "commit_rev":
"b1175bb101a3c9682db929ae476732f4c43fb9a4"}

[commit-bot: I haz the power, 2017-02-28 09:16:52]

Description was changed from

==========
Simplify webrtc::voe::MonitorModule and remove the .cc file.
The class basically implements a timer and can be replaced with a
PostDelayedTask call down the line.

BUG=none
==========

to

==========
Simplify webrtc::voe::MonitorModule and remove the .cc file.
The class basically implements a timer and can be replaced with a
PostDelayedTask call down the line.

BUG=none

Review-Url: https://codereview.webrtc.org/2722613002
Cr-Commit-Position: refs/heads/master@{#16891}
Committed:
https://chromium.googlesource.com/external/webrtc/+/b1175bb101a3c9682db929ae4...
==========

[commit-bot: I haz the power, 2017-02-28 09:16:53]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/external/webrtc/+/b1175bb101a3c9682db929ae4...

[the sun, 2017-02-28 10:11:19]

On 2017/02/28 08:38:43, tommi (webrtc) wrote:
>
https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
> File webrtc/voice_engine/monitor_module.h (right):
> 
>
https://codereview.webrtc.org/2722613002/diff/1/webrtc/voice_engine/monitor_m...
> webrtc/voice_engine/monitor_module.h:30: void Process() override {
> observer_->OnPeriodicProcess(); }
> On 2017/02/28 08:24:21, henrika_webrtc wrote:
> > To me it looks like TransmitMixer::OnPeridodicProcess contains e.g
> > VE_TYPING_NOISE_OFF_WARNING and VE_SATURATION_WARNING. Instead of improving
> this
> > class even more (by using delayed PostTask), it might be worth considering
if
> it
> > can be removed completely. Henrik L should know if any of the callbacks are
> used
> > today.
> 
> Good point... I looked around for VE_SATURATION_WARNING and all I can find is
> this:
> 
>     printf("  SATURATION WARNING \n");
> 
> in voe_cmd_test.cc
> 
> It would be good to delete this if it's not being used.  We'd get rid of one
> lock inside of TransmitMixer::ProcessAudio.  Added a TODO for now in this
file,
> with details.

Yes, voe_cmd_test is reaching EOL with the removal of the legacy APIs.

Thanks for the cleanup Tommi!