Forward output glitch information from stream WebRTC log

Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function) as callback when creating an input stream. In this function, the message is passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
Committed: https://crrev.com/2b2114872f08a1fcfe2a6fb284b645a8152da85c
Cr-Commit-Position: refs/heads/master@{#396722}

[Henrik Grunell, 2016-04-05 12:35:39]

Description was changed from

==========
WIP

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

to

==========
WIP

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

[Henrik Grunell, 2016-04-05 15:11:38]

Description was changed from

==========
WIP

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

to

==========
Write OS glitches info to WebRTC log

*Work in progress*

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

[Henrik Grunell, 2016-04-05 15:11:38]

grunell@chromium.org changed reviewers:
+ dalecurtis@chromium.org, olka@chromium.org

[Henrik Grunell, 2016-04-05 15:13:57]

Dale: can you have a look at the idea here: are you OK with using a function
pointer set by the embedder?

Olga: general comments.

[o1ka, 2016-04-05 15:28:42]

I think it looks fine.

https://codereview.chromium.org/1864483002/diff/20001/content/browser/rendere...
File content/browser/renderer_host/media/media_stream_manager.cc (right):

https://codereview.chromium.org/1864483002/diff/20001/content/browser/rendere...
content/browser/renderer_host/media/media_stream_manager.cc:417:
media::InitWebRtcLoggingCallback(&MediaStreamManager::SendMessageToNativeLog);
I would do it the first thing in the constructor.

https://codereview.chromium.org/1864483002/diff/20001/media/audio/audio_loggi...
File media/audio/audio_logging.cc (right):

https://codereview.chromium.org/1864483002/diff/20001/media/audio/audio_loggi...
media/audio/audio_logging.cc:12: // TODO: Make sure it's thread safe.
You can use atomic flag to guard initialization.

[DaleCurtis, 2016-04-05 21:56:52]

https://codereview.chromium.org/1864483002/diff/20001/media/audio/mac/audio_l...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/1864483002/diff/20001/media/audio/mac/audio_l...
media/audio/mac/audio_low_latency_input_mac.cc:1331: WebRtcLogMessage("(Example)
Here the stats will be logged.");
No, this is a layering violation. media/ shouldn't know anything about WebRTC.
This should be some generic logging infrastructure (likely attached to the
existing AudioLog).

[Henrik Grunell, 2016-04-06 12:19:55]

Description was changed from

==========
Write OS glitches info to WebRTC log

*Work in progress*

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

to

==========
Write OS glitches info to WebRTC log

*DRAFT*

* Add OnStatistics() function to AudioLog interface.
* Hand stream id and pointer to the audio log to the stream. (Safe since
AudioInputRendererHost who owns the audio log outlives all streams.)
* The stream logs as appropriate.

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

[Henrik Grunell, 2016-04-06 12:25:31]

I draft implemented a new approach. It compiles on Linux, works, and also spits
out some debug printouts.

Have a look if this seems OK.

I discovered a problem further up the chain that makes these particular logs (at
stream destruction) to be ignored anyway. That has to be fixed before it makes
sense to continue on this, so after feedback I'll pause this CL.

https://codereview.chromium.org/1864483002/diff/20001/media/audio/mac/audio_l...
File media/audio/mac/audio_low_latency_input_mac.cc (right):

https://codereview.chromium.org/1864483002/diff/20001/media/audio/mac/audio_l...
media/audio/mac/audio_low_latency_input_mac.cc:1331: WebRtcLogMessage("(Example)
Here the stats will be logged.");
On 2016/04/05 21:56:52, DaleCurtis wrote:
> No, this is a layering violation. media/ shouldn't know anything about WebRTC.
> This should be some generic logging infrastructure (likely attached to the
> existing AudioLog).

What about adding something like
AudioLog::OnStatistic(int component_id, const std::string& name, int value);
?
And use like:
audio_log->OnStatistic(stream_id, "Media.Audio.Capture.Glitches",
glitches_detected_);

Then media internals can format and forward to the WebRTC log (maybe filtered on
stat name), and in the future we could add presentation of component specific
stats in media-interals (I think that would be useful).

[DaleCurtis, 2016-04-06 21:18:10]

Seems okay, instead of passing the AudioLog all the way down into the streams,
we might want to let the dispatcher (which logs things for streams today) handle
the logging by giving each created stream a log callback or something similar.

[DaleCurtis, 2016-04-06 21:18:55]

Whoops forgot the why in that statement: It allows the streams to not have to
worry about their IDs, which is kind of an external-to-streams concept.

[Henrik Grunell, 2016-05-17 12:36:41]

Patchset #5 (id:80001) has been deleted

[Henrik Grunell, 2016-05-17 12:49:28]

Description was changed from

==========
Write OS glitches info to WebRTC log

*DRAFT*

* Add OnStatistics() function to AudioLog interface.
* Hand stream id and pointer to the audio log to the stream. (Safe since
AudioInputRendererHost who owns the audio log outlives all streams.)
* The stream logs as appropriate.

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

to

==========
Forward output glitch information from stream WebRTC log

* OnStatistics() function is added to AudioLog interface.
* The AudioOutputDispatcherImpl gives a "statistics callback" to the stream, and
forwards those calls to the AudioLog.
* The Mac output stream informs about OS glitches.

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

[Henrik Grunell, 2016-05-17 12:51:08]

Patchset #4 (id:60001) has been deleted

[Henrik Grunell, 2016-05-17 12:51:19]

Patchset #4 (id:100001) has been deleted

[Henrik Grunell, 2016-05-17 13:03:35]

Ptal.

I plan to create a CL for input tomorrow. I couldn't access bugs.chromium.org so
no BUG specified yet, will add that.

[Henrik Grunell, 2016-05-18 13:02:11]

grunell@chromium.org changed reviewers:
+ sievers@chromium.org

[Henrik Grunell, 2016-05-18 13:02:34]

Description was changed from

==========
Forward output glitch information from stream WebRTC log

* OnStatistics() function is added to AudioLog interface.
* The AudioOutputDispatcherImpl gives a "statistics callback" to the stream, and
forwards those calls to the AudioLog.
* The Mac output stream informs about OS glitches.

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

to

==========
Forward output glitch information from stream WebRTC log

* OnStatistics() function is added to AudioLog interface.
* The AudioOutputDispatcherImpl gives a "statistics callback" to the stream, and
forwards those calls to the AudioLog.
* The Mac output stream informs about OS glitches.

BUG=610967
==========

[Henrik Grunell, 2016-05-18 13:03:55]

Added sievers for trivial content/* changes, please review.

Dale: please review as well.

[o1ka, 2016-05-18 14:00:17]

Confused about input/output and declaration/usage mismatches. Could you clarify?

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
File media/audio/android/audio_manager_android.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
media/audio/android/audio_manager_android.cc:222: const StatisticsCallback&
statistics_callback) {
Does it compile? It looks like there is a mismatch in the header and here.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
File media/audio/android/audio_manager_android.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
media/audio/android/audio_manager_android.h:57: const StatisticsCallback&
statistics_callback) override;
Why is it for input stream here?

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager_base.cc:136: stream = MakeLinearOutputStream(params);
Why there is no logging for linear?

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager_base.cc:157: AudioInputStream*
AudioManagerBase::MakeAudioInputStream(
Do you want to log for input, output, or both?

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
File media/audio/audio_output_dispatcher_impl.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:193: auto it =
audio_stream_ids_.find(physical_stream);
Is there a guarantee that access to |audio_stream_ids_| is thread safe?

https://codereview.chromium.org/1864483002/diff/140001/media/audio/mac/audio_...
File media/audio/mac/audio_manager_mac.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/mac/audio_...
media/audio/mac/audio_manager_mac.cc:696: stream = new AUAudioInputStream(this,
params, audio_device_id);
This does not seem to match the modified AUAudioInputStream declaration?

[DaleCurtis, 2016-05-19 00:44:02]

Instead of passing a statistics cb should we just pass the audio log into the
output stream?

[Henrik Grunell, 2016-05-19 08:26:13]

On 2016/05/19 00:44:02, DaleCurtis wrote:
> Instead of passing a statistics cb should we just pass the audio log into the
> output stream?

Dale, you're arguing with yourself. :D Quoting your previous comment:

"Seems okay, instead of passing the AudioLog all the way down into the streams,
we might want to let the dispatcher (which logs things for streams today) handle
the logging by giving each created stream a log callback or something similar."

and

"It allows the streams to not have to worry about their IDs, which is kind of an
external-to-streams concept."

Which I think is a good argument for having a callback instead of AudioLog in
the stream.

[DaleCurtis, 2016-05-19 18:53:28]

Haha sorry, I blame vacation :) StatsCB is fine with me, I'll take a look
shortly!

[DaleCurtis, 2016-05-19 20:34:37]

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_logg...
File media/audio/audio_logging.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_logg...
media/audio/audio_logging.h:54: virtual void OnStatistics(int component_id,
It seems this should just be OnStatistics(component_id, string)? Otherwise
you're enforcing all statistics to be of integer type.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager.h:138: using StatisticsCallback =
Again, this should just be a base::Callback(string).

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
File media/audio/audio_output_dispatcher_impl.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:149: AudioOutputStream* stream =
audio_manager_->MakeAudioOutputStream(
Just bind this directly to the audio log, since it owns the stream.

[Henrik Grunell, 2016-05-23 17:13:55]

Code review fixes. I've added for input streams and "linear" streams as well in
this CL.

Note: I'm only using the callback for Mac output. I'll add Mac low latency input
tomorrow.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
File media/audio/android/audio_manager_android.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
media/audio/android/audio_manager_android.cc:222: const StatisticsCallback&
statistics_callback) {
On 2016/05/18 14:00:16, o1ka wrote:
> Does it compile? It looks like there is a mismatch in the header and here.

Done.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
File media/audio/android/audio_manager_android.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/android/au...
media/audio/android/audio_manager_android.h:57: const StatisticsCallback&
statistics_callback) override;
On 2016/05/18 14:00:16, o1ka wrote:
> Why is it for input stream here?

Sorry, wrong one. Now I've added for both input and output, and for linear and
low latency.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_logg...
File media/audio/audio_logging.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_logg...
media/audio/audio_logging.h:54: virtual void OnStatistics(int component_id,
On 2016/05/19 20:34:37, DaleCurtis wrote:
> It seems this should just be OnStatistics(component_id, string)? Otherwise
> you're enforcing all statistics to be of integer type.

I did consider just using a string (and call it OnMessage() or so). My idea with
string + value was to allow media-internals to track the stats in the future and
int would do for most cases. We could of course change that if we want to add
stats to media-internals later.

If we only pass a string we're not bound to use the AudioLog, so for input we
can just use the existing AudioInputCOntroller::EventHandler::OnLog() which
would simplify a bit there.

I'm fine with either, actually I lean towards just a string and change the name
to AudioLog::OnMessage(), for using existing path on input and avoid adding
extra code.

I've changed to string only.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
File media/audio/audio_manager.h (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager.h:138: using StatisticsCallback =
On 2016/05/19 20:34:37, DaleCurtis wrote:
> Again, this should just be a base::Callback(string).

Done.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
File media/audio/audio_manager_base.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager_base.cc:136: stream = MakeLinearOutputStream(params);
On 2016/05/18 14:00:16, o1ka wrote:
> Why there is no logging for linear?

I've added linear as well now.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_mana...
media/audio/audio_manager_base.cc:157: AudioInputStream*
AudioManagerBase::MakeAudioInputStream(
On 2016/05/18 14:00:16, o1ka wrote:
> Do you want to log for input, output, or both?

Output was intended first. But now I've added both.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
File media/audio/audio_output_dispatcher_impl.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:149: AudioOutputStream* stream =
audio_manager_->MakeAudioOutputStream(
On 2016/05/19 20:34:37, DaleCurtis wrote:
> Just bind this directly to the audio log, since it owns the stream.

Good point, done.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:193: auto it =
audio_stream_ids_.find(physical_stream);
On 2016/05/18 14:00:17, o1ka wrote:
> Is there a guarantee that access to |audio_stream_ids_| is thread safe?

This function has been removed.

https://codereview.chromium.org/1864483002/diff/140001/media/audio/mac/audio_...
File media/audio/mac/audio_manager_mac.cc (right):

https://codereview.chromium.org/1864483002/diff/140001/media/audio/mac/audio_...
media/audio/mac/audio_manager_mac.cc:696: stream = new AUAudioInputStream(this,
params, audio_device_id);
On 2016/05/18 14:00:17, o1ka wrote:
> This does not seem to match the modified AUAudioInputStream declaration?

Fixed.

[DaleCurtis, 2016-05-23 18:39:47]

lgtm

https://codereview.chromium.org/1864483002/diff/160001/media/audio/audio_outp...
File media/audio/audio_output_dispatcher_impl.cc (right):

https://codereview.chromium.org/1864483002/diff/160001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:150: const int stream_id =
audio_stream_id_;
audio_stream_id_++; they're free and lets not bother reusing a failed one.

[Henrik Grunell, 2016-05-24 10:23:32]

Description was changed from

==========
Forward output glitch information from stream WebRTC log

* OnStatistics() function is added to AudioLog interface.
* The AudioOutputDispatcherImpl gives a "statistics callback" to the stream, and
forwards those calls to the AudioLog.
* The Mac output stream informs about OS glitches.

BUG=610967
==========

to

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

[Henrik Grunell, 2016-05-24 10:24:52]

Description was changed from

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

to

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

[Henrik Grunell, 2016-05-25 14:23:01]

grunell@chromium.org changed reviewers:
+ slan@chromium.org

[Henrik Grunell, 2016-05-25 14:23:49]

grunell@chromium.org changed reviewers:
+ xiyuan@chromium.org

[Henrik Grunell, 2016-05-25 14:27:36]

sievers: content/browser/*
slan: chromecast/*
xiyuan: components/audio_modem/*

The above are trivial changes.

Changes since patch set 6, which Dale lgtm'd:
* Fixed one review comment.
* Fixed build errors (trivial).
* Added log callback to Mac low latency input audio.
* Added tests for log callbacks in unit tests.

Olga and/or Dale, ptal at the last two additions.

https://codereview.chromium.org/1864483002/diff/160001/media/audio/audio_outp...
File media/audio/audio_output_dispatcher_impl.cc (right):

https://codereview.chromium.org/1864483002/diff/160001/media/audio/audio_outp...
media/audio/audio_output_dispatcher_impl.cc:150: const int stream_id =
audio_stream_id_;
On 2016/05/23 18:39:47, DaleCurtis wrote:
> audio_stream_id_++; they're free and lets not bother reusing a failed one.

Done.

[Henrik Grunell, 2016-05-25 14:28:35]

The CQ bit was checked by grunell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-25 14:28:55]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864483002/220001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864483002/220001

[commit-bot: I haz the power, 2016-05-25 14:44:19]

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

[commit-bot: I haz the power, 2016-05-25 14:44:21]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[DaleCurtis, 2016-05-25 19:58:55]

still lgtm

[xiyuan, 2016-05-25 23:41:42]

components/audio_modem LGTM

[slan, 2016-05-26 00:17:23]

lgtm % cast_shell_linux errors.

[Henrik Grunell, 2016-05-26 10:20:32]

The CQ bit was checked by grunell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-26 10:20:47]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864483002/260001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864483002/260001

[commit-bot: I haz the power, 2016-05-26 10:35:28]

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

[commit-bot: I haz the power, 2016-05-26 10:35:30]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Henrik Grunell, 2016-05-26 11:05:16]

The CQ bit was checked by grunell@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-26 11:05:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864483002/280001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864483002/280001

[commit-bot: I haz the power, 2016-05-26 11:22:16]

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

[commit-bot: I haz the power, 2016-05-26 11:22:17]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[no sievers, 2016-05-26 19:54:30]

content lgtm

[Henrik Grunell, 2016-05-26 20:33:27]

The CQ bit was checked by grunell@chromium.org

[Henrik Grunell, 2016-05-26 20:33:28]

The patchset sent to the CQ was uploaded after l-g-t-m from slan@chromium.org,
dalecurtis@chromium.org, xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/1864483002/#ps300001
(title: "Now really fixing the cast compile error. I hope. Really.")

[commit-bot: I haz the power, 2016-05-26 20:34:05]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864483002/300001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864483002/300001

[commit-bot: I haz the power, 2016-05-26 20:37:18]

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

[commit-bot: I haz the power, 2016-05-26 20:37:20]

Try jobs failed on following builders:
  ios-device on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-gn/bui...)
  ios-simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-gn on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-gn/...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_gn_rel on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_gn_r...)
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Henrik Grunell, 2016-05-30 11:22:50]

The CQ bit was checked by grunell@chromium.org

[Henrik Grunell, 2016-05-30 11:22:50]

The patchset sent to the CQ was uploaded after l-g-t-m from slan@chromium.org,
sievers@chromium.org, dalecurtis@chromium.org, xiyuan@chromium.org
Link to the patchset: https://codereview.chromium.org/1864483002/#ps320001
(title: "Rebase.")

[commit-bot: I haz the power, 2016-05-30 11:23:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1864483002/320001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1864483002/320001

[commit-bot: I haz the power, 2016-05-30 13:51:01]

Description was changed from

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

to

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

[commit-bot: I haz the power, 2016-05-30 13:51:03]

Committed patchset #14 (id:320001)

[commit-bot: I haz the power, 2016-05-30 13:52:21]

Description was changed from

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967
==========

to

==========
Forward output glitch information from stream WebRTC log

* A callback is added as input parameter on AudioManager create stream
functions.
* OnLogMessage() function is added to the AudioLog interface.
* The AudioOutputDispatcherImpl gives AudioLog::OnLogMessage() as callback when
creating an output stream.
* AudioInputController gives AudioInputController::LogMessage() (new function)
as callback when creating an input stream. In this function, the message is
passed to AudioInputRendererHost::OnLog().
* In both input and output cases, the message is then passed to the WebRTC log.
* The Mac input and output streams inform about OS glitches via the callback.

BUG=610967

Committed: https://crrev.com/2b2114872f08a1fcfe2a6fb284b645a8152da85c
Cr-Commit-Position: refs/heads/master@{#396722}
==========

[commit-bot: I haz the power, 2016-05-30 13:52:23]

Patchset 14 (id:??) landed as
https://crrev.com/2b2114872f08a1fcfe2a6fb284b645a8152da85c
Cr-Commit-Position: refs/heads/master@{#396722}

[ericrk, 2016-05-30 18:13:22]

A revert of this CL (patchset #14 id:320001) has been created in
https://codereview.chromium.org/2023943002/ by ericrk@chromium.org.

The reason for reverting is: This change broke AudioInputTest.Record on the GPU
FYI bots. For example, see
https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20Retina%20Debug/b...


Confirmed locally that the revert fixes the test failure..

[Guido Urdaneta, 2016-05-31 16:05:58]

A revert of this CL (patchset #14 id:320001) has been created in
https://codereview.chromium.org/2025733003/ by guidou@chromium.org.

The reason for reverting is: Suspect of causing WebRTC Linux Tester bot to fail.


See
https://build.chromium.org/p/chromium.webrtc/builders/Linux%20Tester/builds/1...

Will reland if the revert does not fix the bot..

[Henrik Grunell, 2016-06-01 07:15:21]

On 2016/05/31 16:05:58, Guido Urdaneta wrote:
> A revert of this CL (patchset #14 id:320001) has been created in
> https://codereview.chromium.org/2025733003/ by mailto:guidou@chromium.org.
> 
> The reason for reverting is: Suspect of causing WebRTC Linux Tester bot to
fail.
> 
> 
> See
>
https://build.chromium.org/p/chromium.webrtc/builders/Linux%20Tester/builds/1...
> 
> Will reland if the revert does not fix the bot..

For the record, this last revert didn't land. It was supposed to be a revert of
the re-landed CL, so reverting this CL failed. Anyway, the re-landed CL was not
the cause of the bot failure.