Have AudioRendererMixerManager use separate mixers for separate render views. This is a follow-up …

Have AudioRendererMixerManager use separate mixers for separate render views.  This is a follow-up on SVN r170919; and adds support for associating RenderViews with audio streams mixed together in the render process.

BUG=3541
TEST=AudioRendererMixerManagerTest.* in content_unittests.  Also, ran a local Chrome build with --vmodule=audio_renderer_host=1,render_audiosourceprovider=1 and used window.open() to create multiple RenderViews in one render process, and confirmed desired behaviors.


Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=173721

[miu, 2012-12-16 01:12:45]

Hi Dale,

It's a small one this time.  ;-)  I'm aware you're doing a lot of these for me
lately, but I'm pretty sure a change to render-side mixing is something you'd
want to take a look at.

-Yuri

[DaleCurtis, 2012-12-17 19:06:19]

Hmm, this works, but brings RenderViewIds into ARMM. The concept would work just
as well if ARMMs were per-RenderViewId. Was this something you considered? I.e.
maybe RenderThreadImpl (or a tab mirroring specific object) owns a <int, ARMM>
map?

I ask because that internal map will probably already becoming more complicated
when we make it an <input_AudioParameters, output_AudioParameters> map.

[miu, 2012-12-17 22:50:26]

On 2012/12/17 19:06:19, DaleCurtis wrote:
> Hmm, this works, but brings RenderViewIds into ARMM. The concept would work
just
> as well if ARMMs were per-RenderViewId. Was this something you considered?
I.e.
> maybe RenderThreadImpl (or a tab mirroring specific object) owns a <int, ARMM>
> map?

I hadn't considered this.

I took a look, and think the current approach is more ideal.  The main reason is
that we'd have to put in a lot of extra code to manage the lifetime of ARMMs. 
For example, when a RenderView is shut down, we'd need to remove ARMM from a map
in RenderThreadImpl and then delete it only after we know there are no mixers
in-use.  Resolving this would be difficult because shutdown of audio is
asynchronous (i.e., across threads).  ARMM would have to become a ref-counted
object, and we'd have to plumb-in something to deref() it once
AudioRendererMixerInput calls the "RemoveMixer" callback.

> I ask because that internal map will probably already becoming more
complicated
> when we make it an <input_AudioParameters, output_AudioParameters> map.

FWICT, you'll be needing to create a "struct MixerKey" to use as a key for the
mixer map in ARMM anyway.  Something like:

struct MixerKey {
  AudioParameters input_params;
  AudioParameters output_params;

  bool operator<(...);
};

Would you prefer I do that in this change, instead of using an std::pair<...>?

[DaleCurtis, 2012-12-17 23:48:20]

I think it's too early for that change, so this is fine for now. lgtm.

[miu, 2012-12-18 05:17:57]

On 2012/12/17 23:48:20, DaleCurtis wrote:
> I think it's too early for that change, so this is fine for now. lgtm.

Thanks for the quick turnaround.  :)

[commit-bot: I haz the power, 2012-12-18 06:43:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/miu@chromium.org/11598002/1

[commit-bot: I haz the power, 2012-12-18 12:19:02]

Change committed as 173721