Closed Bug 1045018 Opened 5 years ago Closed 5 years ago

Add a way to set the kAudioHardwarePropertyRunLoop property on OSX only once per process

Categories

(Core :: Audio/Video, defect)

x86_64
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla34

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(3 files)

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
Blocks: 1027713
Depends on: 1015932
Assignee: nobody → paul
This is all green on try.
And also, this needs patches from bug 1015932.
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+
Attachment #8463351 - Flags: review?(gpascutto) → review+
Attachment #8463349 - Flags: review?(kinetik) → review+
You need to log in before you can comment on or make changes to this bug.