Closed
Bug 1045018
Opened 10 years ago
Closed 10 years ago
Add a way to set the kAudioHardwarePropertyRunLoop property on OSX only once per process
Categories
(Core :: Audio/Video, defect)
Tracking
()
RESOLVED
FIXED
mozilla34
People
(Reporter: padenot, Assigned: padenot)
References
Details
Attachments
(3 files)
3.58 KB,
patch
|
gcp
:
review+
|
Details | Diff | Splinter Review |
4.33 KB,
patch
|
kinetik
:
review+
|
Details | Diff | Splinter Review |
2.37 KB,
patch
|
gcp
:
review+
|
Details | Diff | Splinter Review |
I got backed out in bug 1027713 because we are setting kAudioHardwarePropertyRunLoop to NULL in cubeb (that forces device plug/unplug notifications to run on their own thread instead of the main thread) and the webrtc.org code base sets it to NULL as well, and messing with an event loop while it's running is not a good idea.
The plan is as follow:
- Use gcp's new content/media/systemservices directory to be able to share a single function between cubeb and webrtc.org code
- implement a thread safe singleton that ensure the function is called only once
- add a header to cubeb so we can have two implementation: a mozilla-central implementation (when Firefox is running), that uses the singleton, and cubeb's standalone implementation (for tests in m-c, or when building standalone)
- #ifdef our way to victory in the webrtc.org codebase, using MOZILLA_INTERNAL_API
Assignee | ||
Updated•10 years ago
|
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8463348 -
Flags: review?(gpascutto)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → paul
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8463349 -
Flags: review?(kinetik)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8463351 -
Flags: review?(gpascutto)
Assignee | ||
Comment 4•10 years ago
|
||
This is all green on try.
Assignee | ||
Comment 5•10 years ago
|
||
And also, this needs patches from bug 1015932.
Comment 6•10 years ago
|
||
Comment on attachment 8463348 [details] [diff] [review]
Implement a thread safe singleton to allow setting kAudioHardwarePropertyRunLoop property to NULL only once. r=
Review of attachment 8463348 [details] [diff] [review]:
-----------------------------------------------------------------
::: content/media/systemservices/OSXRunLoopSingleton.cpp
@@ +13,5 @@
> +
> +static bool gRunLoopSet = false;
> +static mozilla::StaticMutex gMutex;
> +
> +void mozilla_set_coreaudio_notification_runloop_if_needed()
nit: I could argue the _if_needed is an implementation detail the API naming doesn't need to expose.
@@ +20,5 @@
> + if (gRunLoopSet) {
> + return;
> + }
> +
> + OSStatus r;
nit: Move this down to where it's used.
@@ +30,5 @@
> + kAudioObjectPropertyScopeGlobal,
> + kAudioObjectPropertyElementMaster
> + };
> +
> + CFRunLoopRef run_loop = NULL;
nit: Maybe nullptr and #include <mozilla/NullPtr.h>
::: content/media/systemservices/OSXRunLoopSingleton.h
@@ +3,5 @@
> + * License, v. 2.0. If a copy of the MPL was not distributed with this file,
> + * You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> +#ifndef OSXRUNLOOPSINGLETON_H
> +#define OSXRUNLOOPSINGLETON_H
nit: We're inconsistent about the leading_, but all other code uses a trailing _, so BLAH_H_
::: content/media/systemservices/moz.build
@@ +33,3 @@
> include('/ipc/chromium/chromium-config.mozbuild')
>
> FINAL_LIBRARY = 'gklayout'
Needs updating now.
Attachment #8463348 -
Flags: review?(gpascutto) → review+
Updated•10 years ago
|
Attachment #8463351 -
Flags: review?(gpascutto) → review+
Updated•10 years ago
|
Attachment #8463349 -
Flags: review?(kinetik) → review+
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/9e7128b769a7
https://hg.mozilla.org/mozilla-central/rev/fd4a70e6d514
https://hg.mozilla.org/mozilla-central/rev/3e28cd6f2ab8
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
You need to log in
before you can comment on or make changes to this bug.
Description
•