Plugin Power Saver: Mute throttled plugins.

Plugin Power Saver: Mute throttled plugins.

BUG=452215, 403800
Committed: https://crrev.com/43d8cc4ccbb9dacf346bc6d6fe0f407b3a1e9c59
Cr-Commit-Position: refs/heads/master@{#321593}

[tommycli, 2015-01-28 20:41:23]

tommycli@chromium.org changed reviewers:
+ dmichael@chromium.org, piman@chromium.org

[tommycli, 2015-01-28 20:41:24]

piman/dmichael:

I've been trying to mute plugin audio by zeroing out the audio buffers.

However, this doesn't work, as it seems that there are actually two
PPB_Audio_Shared objects, one of which may live in a different process?

I thought zeroing out the buffers would be a 'trivial' way of muting the audio,
but this doesn't seem to be the case.

Let me know if you guys have any advice.

https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
File ppapi/shared_impl/ppb_audio_shared.cc (right):

https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
ppapi/shared_impl/ppb_audio_shared.cc:236: if (IsMutedForPowerSaver()) {
This doesn't actually work!

As far as I can tell, there are actually two instances of PPB_Audio_Shared, and
the muted_for_power_saver_ variable is not shared between them.

Is the other one the "proxy" object that lives in a whole different process?

[piman, 2015-01-28 21:53:18]

I'm not super familiar with the audio path....

[piman, 2015-01-28 21:58:50]

Generally, PPAPI resources exist in 2 forms, one in the plugin and one in the
renderer or browser.

I have a vague understanding that for audio, things go directly between the
plugin and the browser, so doing anything in the renderer probably has no
effect. You'd need to send a message to the plugin (or deal with it in the
browser, but that may be harder).

https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
File ppapi/shared_impl/ppb_audio_shared.h (right):

https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
ppapi/shared_impl/ppb_audio_shared.h:117: volatile bool muted_for_power_saver_;
Why volatile?

[tommycli, 2015-01-28 23:04:52]

On 2015/01/28 21:58:50, piman (Very slow to review) wrote:
> Generally, PPAPI resources exist in 2 forms, one in the plugin and one in the
> renderer or browser.
> 
> I have a vague understanding that for audio, things go directly between the
> plugin and the browser, so doing anything in the renderer probably has no
> effect. You'd need to send a message to the plugin (or deal with it in the
> browser, but that may be harder).
> 
>
https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
> File ppapi/shared_impl/ppb_audio_shared.h (right):
> 
>
https://codereview.chromium.org/879403002/diff/1/ppapi/shared_impl/ppb_audio_...
> ppapi/shared_impl/ppb_audio_shared.h:117: volatile bool
muted_for_power_saver_;
> Why volatile?

piman: Okay thank you, that is helpful. It confirms my suspicion that there's
some multi-process trickery going on.

Maybe the callback-starvation technique we discussed in email is will ultimately
be cleaner than trying to zero out the buffers. I'm going to table this for now.

[dmichael (off chromium), 2015-01-29 17:21:03]

You might be able to just take advantage of the StopPlayback() path? IIUC, it
will result in sending a message to the browser to pause output. It's likely to
have the right effect, though I'm not sure it will stop the callbacks in the
plugin.

[tommycli, 2015-02-13 23:08:10]

Patchset #7 (id:120001) has been deleted

[tommycli, 2015-02-19 00:30:30]

Patchset #7 (id:140001) has been deleted

[tommycli, 2015-03-19 22:38:40]

dmichael/piman: PTAL. Patchset 11 has been tested with our Adobe collaborator as
'works fine for me'.

Do you guys mind taking another look?

Thanks!

[piman, 2015-03-19 23:06:24]

lgtm

[dmichael (off chromium), 2015-03-20 16:40:15]

lgtm

[tommycli, 2015-03-20 16:56:28]

The CQ bit was checked by tommycli@chromium.org

[commit-bot: I haz the power, 2015-03-20 16:57:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/879403002/260001

[commit-bot: I haz the power, 2015-03-20 17:48:27]

Committed patchset #12 (id:260001)

[commit-bot: I haz the power, 2015-03-20 17:49:29]

Patchset 12 (id:??) landed as
https://crrev.com/43d8cc4ccbb9dacf346bc6d6fe0f407b3a1e9c59
Cr-Commit-Position: refs/heads/master@{#321593}