849916 - Do the plumbing necessary to make it possible to hook up PannerNode to the MSG backend

Status: RESOLVED FIXED

(Core :: Web Audio, defect)

Comment 1

[(no longer active)]

Sorry I did not get to write a patch for this tonight.  I will do that tomorrow morning.

Comment 2

[(no longer active)]

Attachment: Part 1: Make it possible to send ThreeDPoint objects as commands to the MSG thread

Comment 3

[(no longer active)]

Attachment: Part 2: Add helper functions for sending different argument types to the AudioNodeStream

Comment 4

[(no longer active)]

Attachment: Part 3: Create an engine for PannerNodes and make sure that we deliver the panner and listener parameters to it

Comment 5

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/afad1c17015e
https://hg.mozilla.org/integration/mozilla-inbound/rev/2becc603eeb8
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ee69256f8f2

Comment 6

[Ryan VanderMeulen [:RyanVM]]

Backed out for build bustage.
https://hg.mozilla.org/integration/mozilla-inbound/rev/03275a0cc91f

https://tbpl.mozilla.org/php/getParsedLog.php?id=20575311&tree=Mozilla-Inbound

../../../../content/media/webaudio/PannerNode.cpp:126:45: error: use of undeclared identifier 'pannerNode'
  Context()->Listener()->RegisterPannerNode(pannerNode);
                                            ^
1 error generated.
make[7]: *** [PannerNode.o] Error 1

Comment 7

[(no longer active)]

Let's try again:

https://hg.mozilla.org/integration/mozilla-inbound/rev/e2f71314b177
https://hg.mozilla.org/integration/mozilla-inbound/rev/9481f3f04df9
https://hg.mozilla.org/integration/mozilla-inbound/rev/10d17cb9cd1d

Hopefully this time I committed something that is, well, C++.  ;-)

Comment 8

[(no longer active)]

Attachment: Part 4: Don't rely on things still being around when we run PannerNode's dtor, use a weak pointer instead

So it turns out that if the cycle collector unlinks us, mContext will be null in ~PannerNode, which means that this cleanup code will crash.  I could modify the CC macros to do some magic before nulling out mContext but that's aweful since mContext lives in AudioNode, so let's just use a weak pointer.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 724192 [details] [diff] [review]
Part 4: Don't rely on things still being around when we run PannerNode's dtor, use a weak pointer instead

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

Why don't we make mPanners strong, and traverse it and unlink it in CC? Seems like that would be better.

Comment 10

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9)
> Why don't we make mPanners strong, and traverse it and unlink it in CC?
> Seems like that would be better.

Because that would make implementing the node ownership model of making sure nodes will go away after playback is finished if script is not holding references to them more complicated than necessary.  I think we should try to avoid holding references to nodes outside of the audio graph.

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/e2f71314b177
https://hg.mozilla.org/mozilla-central/rev/9481f3f04df9
https://hg.mozilla.org/mozilla-central/rev/10d17cb9cd1d

Comment 12

[(no longer active)]

Comment on attachment 724192 [details] [diff] [review]
Part 4: Don't rely on things still being around when we run PannerNode's dtor, use a weak pointer instead

https://hg.mozilla.org/integration/mozilla-inbound/rev/0cb671b3125b

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0cb671b3125b

Comment 14

[(no longer active)]

Mass moving Web Audio bugs to the Web Audio component.  Filter on duckityduck.