989921 - Add an API to the MediaStreamGraph to register a mixer callback function

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Paul Adenot (:padenot)]

This is necessary if we want to be able to provide the audio output for a tab (for testing purposes), generally to be able to have multiple consumer of the mixed output (rather than hard-wiring it to the AEC).

Comment 1

[Dan Mosedale (:dmosedale, :dmose)]

This would very nice for doing functional testing of audio applications that do any sort of audio transmission, or even more mixing.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Add an API to the MediaStreamGraph to register a mixer callback function. r=

I figured I needed this for the msg refactor, so here it is.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8418657 [details] [diff] [review]
Add an API to the MediaStreamGraph to register a mixer callback function. r=

Review of attachment 8418657 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/AudioMixer.h
@@ +51,5 @@
>     * tracks have been mixed in. The caller should not hold onto the data. */
>    void FinishMixing() {
> +    for (MixerCallback* cb = mCallbacks.getFirst();
> +         cb != nullptr; cb = cb->getNext()) {
> +      cb->mReceiver->MixerCallback(mMixedAudio.Elements(),

will this ever call me back with frames == 0 or channels == 0?

::: content/media/MediaStreamGraph.cpp
@@ +617,2 @@
>    } else if (mMixer && !shouldMix) {
> +    mMixer->RemoveCallback(gFarendObserver);

I should note that gFarendObserver is basically a hack to avoid having to register... 

There should be an API available on MSG, and then MediaEngineWebRTCAudio should register/unregister with it.

This could be done in a follow-on bug, however.  File a followup, assign it to me, and add a comment here?

Comment 4

[Paul Adenot (:padenot)]

(In reply to Randell Jesup [:jesup] from comment #3)
> Comment on attachment 8418657 [details] [diff] [review]
> Add an API to the MediaStreamGraph to register a mixer callback function. r=
> 
> Review of attachment 8418657 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: content/media/AudioMixer.h
> @@ +51,5 @@
> >     * tracks have been mixed in. The caller should not hold onto the data. */
> >    void FinishMixing() {
> > +    for (MixerCallback* cb = mCallbacks.getFirst();
> > +         cb != nullptr; cb = cb->getNext()) {
> > +      cb->mReceiver->MixerCallback(mMixedAudio.Elements(),
> 
> will this ever call me back with frames == 0 or channels == 0?

In theory, no, but I'll MOZ_ASSERT it before landing so this stays true.

> ::: content/media/MediaStreamGraph.cpp
> @@ +617,2 @@
> >    } else if (mMixer && !shouldMix) {
> > +    mMixer->RemoveCallback(gFarendObserver);
> 
> I should note that gFarendObserver is basically a hack to avoid having to
> register... 
> 
> There should be an API available on MSG, and then MediaEngineWebRTCAudio
> should register/unregister with it.
> 
> This could be done in a follow-on bug, however.  File a followup, assign it
> to me, and add a comment here?

Sure.

Comment 5

[Paul Adenot (:padenot)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8dd41701dd92

Comment 6

[Paul Adenot (:padenot)]

Hrm, I think I pushed the wrong version or something. Backed out for now, until I investigate https://hg.mozilla.org/integration/mozilla-inbound/rev/d1b03ddc3161

Comment 7

[Paul Adenot (:padenot)]

Attachment: mixer-api

This is the right version. The only change is the added StartMixing member and call.

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8430127 [details] [diff] [review]
mixer-api

Review of attachment 8430127 [details] [diff] [review]:
-----------------------------------------------------------------

::: content/media/AudioMixer.h
@@ +89,5 @@
>      }
>    }
> +
> +  void AddCallback(MixerCallbackReceiver* aReceiver) {
> +    mCallbacks.insertBack(new MixerCallback(aReceiver));

Is there a presumption as to which thread these occur on (Add and Remove, and Access for mixing)?  If so (MSG I'm guessing) please add a comment here so we know why a lock isn't needed.

Comment 9

[Paul Adenot (:padenot)]

Attachment: Allow the MediaStreamGraph mixer to send data back to multiple consumers

Not sure if this changed, but I'm uploading it again along all the patches for
bug 848954 to make sure.

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8163a245cb74

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8163a245cb74