1241499 - Initialize the HRTF database lazily

Status: RESOLVED FIXED

(Core :: Web Audio, defect, P1)

Description

[Paul Adenot (:padenot)]

HRTF used to be the default, so it made sense to initialize it when the engine was created, but the situation changed, so it now makes sense to delay its initialization, because it is computationally and specially expensive.

Comment 1

[Paul Adenot (:padenot)]

Attachment: MozReview Request: Bug 1241499 - Initialize the HRTF database lazily. r?karlt

Review commit: https://reviewboard.mozilla.org/r/31775/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/31775/

Comment 2

[Maire Reavy [:mreavy]]

I'm pretty sure

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 8710463 [details]
MozReview Request: Bug 1241499 - Initialize the HRTF database lazily. r?karlt

https://reviewboard.mozilla.org/r/31775/#review30211

::: dom/media/webaudio/PannerNode.cpp:67
(Diff revision 1)
> +  void InitializeHRTFDatabase()

This function does more than just initialize the database.  How would you feel about CreateHRTFPanner()?

::: dom/media/webaudio/PannerNode.cpp:276
(Diff revision 1)
> +    // We can directly call in the engine here, because it will not touch
> +    // mHRTFPanner until it receives the parameter below via a control message.

The reason described here is what provides the thread safety, thanks, but calling into the engine is not the risk because the engine is owned by the stream for the lifetime of the stream.  Threads can be confusing, so I think it would be helpful to document this clearly.

Perhaps "We can set the engine's mHRTFPanner from the main thread here because the engine will not touch ..."

I like to document thread access to variables at their declaration.  Can you say something like "mHTRFPanner is set on the main thread before and accessed on the graph thread after mPanningModelFunction first changes." at the declaration of mHRTFPanner please?

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/865b01fa7947

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/865b01fa7947