Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916
Committed: https://crrev.com/adbc993c30d6c1582b142db137ddedb57d648228
Cr-Commit-Position: refs/heads/master@{#402428}

[Henrik Grunell, 2016-06-21 09:51:13]

Description was changed from

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

BUG=607916
==========

to

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

BUG=607916
==========

[Henrik Grunell, 2016-06-21 09:51:13]

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

[Henrik Grunell, 2016-06-21 09:51:49]

Description was changed from

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

BUG=607916
==========

to

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916
==========

[o1ka, 2016-06-21 12:43:09]

lgtm % comment clarification

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_input.cc:130: return mixer_ &&
mixer_->CurrentThreadIsRenderingThread();
I would say something like:
This method is expected to be called in callback_->Render().
|mixer_| access on rendering thread is safe:
AudioRendererMixer::AddInput()/RemoveInput()/Render() take lock for
AudioConverter operations, which guarantees that there are no
ARMI::ProvideInput()(=>callback_->Render()) calls coming before/during
AddInput() and during/after RemoveInput().

[DaleCurtis, 2016-06-21 19:50:04]

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_input.cc:130: return mixer_ &&
mixer_->CurrentThreadIsRenderingThread();
On 2016/06/21 at 12:43:09, o1ka wrote:
> I would say something like:
> This method is expected to be called in callback_->Render().
> |mixer_| access on rendering thread is safe:
AudioRendererMixer::AddInput()/RemoveInput()/Render() take lock for
AudioConverter operations, which guarantees that there are no
ARMI::ProvideInput()(=>callback_->Render()) calls coming before/during
AddInput() and during/after RemoveInput().

Can we just DCHECK(mixer_)?

[Henrik Grunell, 2016-06-23 06:19:55]

CR fixes. Moved a comment to header file.

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
File media/base/audio_renderer_mixer_input.cc (right):

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_input.cc:130: return mixer_ &&
mixer_->CurrentThreadIsRenderingThread();
On 2016/06/21 12:43:09, o1ka wrote:
> I would say something like:
> This method is expected to be called in callback_->Render().
> |mixer_| access on rendering thread is safe:
> AudioRendererMixer::AddInput()/RemoveInput()/Render() take lock for
> AudioConverter operations, which guarantees that there are no
> ARMI::ProvideInput()(=>callback_->Render()) calls coming before/during
> AddInput() and during/after RemoveInput().

I'd prefer a more general and shorter comment. :) Since the details are about
something outside of this class, which can change. What about describing the
contract in the header?

https://codereview.chromium.org/2083933003/diff/1/media/base/audio_renderer_m...
media/base/audio_renderer_mixer_input.cc:130: return mixer_ &&
mixer_->CurrentThreadIsRenderingThread();
On 2016/06/21 19:50:03, DaleCurtis wrote:
> On 2016/06/21 at 12:43:09, o1ka wrote:
> > I would say something like:
> > This method is expected to be called in callback_->Render().
> > |mixer_| access on rendering thread is safe:
> AudioRendererMixer::AddInput()/RemoveInput()/Render() take lock for
> AudioConverter operations, which guarantees that there are no
> ARMI::ProvideInput()(=>callback_->Render()) calls coming before/during
> AddInput() and during/after RemoveInput().
> 
> Can we just DCHECK(mixer_)?

No need to DCHECK since it'll crash when dereferencing. And as a means to
document the code, it doesn't document why it's safe throughout the execution of
the function. So I think a a comment is better, but "mixer_ &&" can be removed.

[Henrik Grunell, 2016-06-23 13:58:18]

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

[commit-bot: I haz the power, 2016-06-23 13:58:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2083933003/20001

[commit-bot: I haz the power, 2016-06-23 15:04:19]

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

[commit-bot: I haz the power, 2016-06-23 15:04:20]

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

[Henrik Grunell, 2016-06-27 08:24:23]

Ping Dale.

[DaleCurtis, 2016-06-27 17:45:13]

lgtm

[Henrik Grunell, 2016-06-28 06:54:44]

The CQ bit was checked by grunell@chromium.org

[Henrik Grunell, 2016-06-28 06:54:45]

The patchset sent to the CQ was uploaded after l-g-t-m from olka@chromium.org
Link to the patchset: https://codereview.chromium.org/2083933003/#ps20001
(title: "Code review. Rebase.")

[commit-bot: I haz the power, 2016-06-28 06:55:01]

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

[commit-bot: I haz the power, 2016-06-28 09:06:52]

Description was changed from

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916
==========

to

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916
==========

[commit-bot: I haz the power, 2016-06-28 09:06:53]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-06-28 09:08:19]

Description was changed from

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916
==========

to

==========
Implement AudioRendererMixerInput::CurrentThreadIsRenderingThread().

This is needed for switching WebRTC rendering to go through the mixer.

BUG=607916

Committed: https://crrev.com/adbc993c30d6c1582b142db137ddedb57d648228
Cr-Commit-Position: refs/heads/master@{#402428}
==========

[commit-bot: I haz the power, 2016-06-28 09:08:20]

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