Closed Bug 1241499 Opened 6 years ago Closed 5 years ago
Initialize the HRTF database lazily
58 bytes, text/x-review-board-request
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.
Review commit: https://reviewboard.mozilla.org/r/31775/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/31775/
Attachment #8710463 - Flags: review?(karlt)
I'm pretty sure
Priority: -- → P1
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?
Attachment #8710463 - Flags: review?(karlt) → review+
You need to log in before you can comment on or make changes to this bug.