Pass event log to ANA.

Pass event log to ANA.

BUG=webrtc:6845
Review-Url: https://codereview.webrtc.org/2553413002
Cr-Commit-Position: refs/heads/master@{#16052}
Committed: https://chromium.googlesource.com/external/webrtc/+/bf279fc4b90d8c1c40570ddd361c69831107fbbf

[michaelt, 2016-12-07 10:14:18]

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

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

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, 2016-12-07 10:18:18]

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

[commit-bot: I haz the power, 2016-12-07 10:18:18]

Dry run: Try jobs failed on following builders:
  ios_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/20763)

[michaelt, 2016-12-07 10:29:14]

Patchset #1 (id:1) has been deleted

[michaelt, 2016-12-07 10:29:49]

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

[commit-bot: I haz the power, 2016-12-07 10:29:58]

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, 2016-12-07 10:34:37]

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

[commit-bot: I haz the power, 2016-12-07 10:34:38]

Dry run: Try jobs failed on following builders:
  mac_baremetal on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/16900)

[michaelt, 2016-12-07 12:08:31]

Patchset #1 (id:20001) has been deleted

[michaelt, 2016-12-07 12:08:47]

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

[commit-bot: I haz the power, 2016-12-07 12:08:50]

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

[michaelt, 2016-12-07 12:17:50]

Description was changed from

==========
Pass event log to ANA.

BUG=webrtc:6845
==========

to

==========
Pass event log to ANA.

BUG=webrtc:6845
==========

[michaelt, 2016-12-07 12:17:50]

michaelt@webrtc.org changed reviewers:
+ minyue@webrtc.org, solenberg@webrtc.org

[michaelt, 2016-12-07 12:22:00]

Hi,
Here the first CL regarding logging the decisions of ANA.

[commit-bot: I haz the power, 2016-12-07 12:22:42]

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

[commit-bot: I haz the power, 2016-12-07 12:22:42]

Dry run: This issue passed the CQ dry run.

[minyue-webrtc, 2016-12-07 12:31:07]

On 2016/12/07 12:22:00, michaelt wrote:
> Hi,
> Here the first CL regarding logging the decisions of ANA.

Good work. Would you upload the next CLs to show how this is going to work?

[the sun, 2017-01-12 10:11:50]

solenberg@webrtc.org changed reviewers:
+ ossu@webrtc.org

[the sun, 2017-01-12 10:11:51]

I think it would make the relationship between RtcEventLog/Channel/AudioEncoder
simpler if the RtcEventLog* was handled to the encoder when it is created,
instead of when ANA is enabled.

ossu@, WDYT? Can you provide input on how to best propagate the RtcEventLog*,
with the upcoming changes for injectable codecs in mind.

[michaelt, 2017-01-12 10:32:23]

Passing the event_log to the AudioEncoder on construction would make the CL more
complex.
Since I would have to pass it first from channel to CodecManager then to
RentACodec before i could pass it to the audio encoder.
The pointer of the event log will be hold in the audio network adapter. And ANA
is at the moment the only class which will use the event log in the
AudioEncoder.
So i see no pressing reason to pass the event log at the construction of the
AudioEncoder.

[michaelt, 2017-01-12 10:49:43]

The goal of the cl is to pass the eventlog to the audio network adapter, not to
the AudioEncoder.

[ossu, 2017-01-12 12:21:58]

On 2017/01/12 10:11:51, the sun wrote:
> I think it would make the relationship between
RtcEventLog/Channel/AudioEncoder
> simpler if the RtcEventLog* was handled to the encoder when it is created,
> instead of when ANA is enabled.
> 
> ossu@, WDYT? Can you provide input on how to best propagate the RtcEventLog*,
> with the upcoming changes for injectable codecs in mind.

At SomePointInTheFuture(tm), I believe we might want to bundle things like the
RtcEventLog and the Clock and have them universally available to WebRTC
components, i.e. by sending along some sort of context upon construction. Then
the AudioEncoder instance would have these things available and could use them
when setting up its audio network adaptor.

For now, I think this is good enough. There's nothing in this change that goes
against my injectable audio encoder work and I believe it will be easier to
change how these things are passed after my changes. So for me, this can go
ahead, and once encoder creation is changed we can revisit if necessary.

[ossu, 2017-01-12 12:23:24]

Oh, right, I'm a reviewer. LGTM. :)

[the sun, 2017-01-12 13:14:59]

On 2017/01/12 12:21:58, ossu wrote:
> On 2017/01/12 10:11:51, the sun wrote:
> > I think it would make the relationship between
> RtcEventLog/Channel/AudioEncoder
> > simpler if the RtcEventLog* was handled to the encoder when it is created,
> > instead of when ANA is enabled.
> > 
> > ossu@, WDYT? Can you provide input on how to best propagate the
RtcEventLog*,
> > with the upcoming changes for injectable codecs in mind.
> 
> At SomePointInTheFuture(tm), I believe we might want to bundle things like the
> RtcEventLog and the Clock and have them universally available to WebRTC
> components, i.e. by sending along some sort of context upon construction. Then
> the AudioEncoder instance would have these things available and could use them
> when setting up its audio network adaptor.

FYI: The Clock interface is being deprecated (is it even used here anymore?) so
we won't be passing that around in a context.

> For now, I think this is good enough. There's nothing in this change that goes
> against my injectable audio encoder work and I believe it will be easier to
> change how these things are passed after my changes. So for me, this can go
> ahead, and once encoder creation is changed we can revisit if necessary.

sgtm

[the sun, 2017-01-12 13:15:37]

lgtm % comment

https://codereview.webrtc.org/2553413002/diff/40001/webrtc/modules/audio_codi...
File
webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h
(right):

https://codereview.webrtc.org/2553413002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h:32:
RtcEventLog* event_log;
Init to nullptr either here or in ctor init list.

[michaelt, 2017-01-12 13:53:42]

https://codereview.webrtc.org/2553413002/diff/40001/webrtc/modules/audio_codi...
File
webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h
(right):

https://codereview.webrtc.org/2553413002/diff/40001/webrtc/modules/audio_codi...
webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h:32:
RtcEventLog* event_log;
On 2017/01/12 13:15:36, the sun wrote:
> Init to nullptr either here or in ctor init list.

Done.

[michaelt, 2017-01-12 13:53:58]

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

[commit-bot: I haz the power, 2017-01-12 13:54:07]

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-01-12 13:58:15]

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

[commit-bot: I haz the power, 2017-01-12 13:58:16]

Dry run: Try jobs failed on following builders:
  ios32_sim_ios9_dbg on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_ios9_dbg/buil...)

[michaelt, 2017-01-12 14:08:29]

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

[commit-bot: I haz the power, 2017-01-12 14:08:36]

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-01-12 18:34:32]

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

[commit-bot: I haz the power, 2017-01-12 18:34:33]

Dry run: This issue passed the CQ dry run.

[michaelt, 2017-01-12 18:34:46]

The CQ bit was checked by michaelt@webrtc.org

[michaelt, 2017-01-12 18:34:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, ossu@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2553413002/#ps80001 (title:
"Fix unittest build")

[commit-bot: I haz the power, 2017-01-12 18:34:52]

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-01-12 18:39:41]

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

[commit-bot: I haz the power, 2017-01-12 18:39:43]

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

[michaelt, 2017-01-12 18:41:53]

The CQ bit was checked by michaelt@webrtc.org

[commit-bot: I haz the power, 2017-01-12 18:42:03]

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-01-12 18:45:32]

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

[commit-bot: I haz the power, 2017-01-12 18:45:33]

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

[michaelt, 2017-01-12 19:46:52]

The CQ bit was checked by michaelt@webrtc.org

[commit-bot: I haz the power, 2017-01-12 19:46: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, 2017-01-12 19:51:05]

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

[commit-bot: I haz the power, 2017-01-12 19:51:06]

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

[michaelt, 2017-01-12 19:54:38]

The CQ bit was checked by michaelt@webrtc.org

[commit-bot: I haz the power, 2017-01-12 19:54:46]

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-01-12 19:58:56]

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

[commit-bot: I haz the power, 2017-01-12 19:58:57]

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

[michaelt, 2017-01-12 21:58:37]

The CQ bit was checked by michaelt@webrtc.org

[commit-bot: I haz the power, 2017-01-12 21:58:39]

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-01-12 22:03:18]

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

[commit-bot: I haz the power, 2017-01-12 22:03:20]

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

[minyue-webrtc, 2017-01-13 08:11:20]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2017-01-13 08:11:29]

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-01-13 08:14:37]

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

[commit-bot: I haz the power, 2017-01-13 08:14:38]

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

[minyue-webrtc, 2017-01-13 10:23:57]

The CQ bit was checked by minyue@webrtc.org

[commit-bot: I haz the power, 2017-01-13 10:24:09]

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-01-13 10:27:37]

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

[commit-bot: I haz the power, 2017-01-13 10:27:39]

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

[michaelt, 2017-01-13 10:58:44]

The CQ bit was checked by michaelt@webrtc.org

[michaelt, 2017-01-13 10:58:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, ossu@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2553413002/#ps100001 (title:
"Rebased")

[commit-bot: I haz the power, 2017-01-13 10:58:51]

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-01-13 14:02:30]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1484305124336840,
"parent_rev": "bb341975fd60bb646068be65ec35363e12903737", "commit_rev":
"bf279fc4b90d8c1c40570ddd361c69831107fbbf"}

[commit-bot: I haz the power, 2017-01-13 14:02:35]

Description was changed from

==========
Pass event log to ANA.

BUG=webrtc:6845
==========

to

==========
Pass event log to ANA.

BUG=webrtc:6845

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

[commit-bot: I haz the power, 2017-01-13 14:02:36]

Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/external/webrtc/+/bf279fc4b90d8c1c40570ddd3...