зеркало из https://github.com/mozilla/gecko-dev.git
Bug 1127213 - Fix various issues with the device change notification in the WASAPI cubeb backend. r=kinetik
This patch does the following: - Introduces an owned_critical_section object to be able to assert that a current thread owns a critical section - Change the auto_lock to use the above, add auto_unlock (basically like the Gecko AutoEnter/AutoExit things) - Fix an issue during audio output device switch where the clock would use the old sample rate. Apparently I did not notice this because my headset and the sound card on this laptop have the same rate - Check that we could acquire a device_enumerator in the ctor before deallocating in the dtor, as that can happen if a ton of streams are running at once (I had this issue running the full mochitest suite) - Stop getting another device_enumator in unregister_notification_client, fixing a leak - Assert that setup_wasapi_stream and close_wasapi_stream are called with the lock held, this was the cause of the crash for this bug - Make close_wasapi_stream clear out its state to make sure setup_wasapi_stream and close_wasapi_stream are called in the right order (especially, not two setup_wasapi_stream without close in between, since that would leak stuff) - In wasapi_stream_destroy, unregister the notification client before destroying the CRITICAL_SECTION (this was the cause of a crash I duped against this bug)
This commit is contained in:
Paul Adenot
Родитель
5386f3a04e
Коммит
8b005f3fb1
|
|
@ -4,7 +4,12 @@
|
|||
* This program is made available under an ISC-style license. See the
|
||||
* accompanying file LICENSE for details.
|
||||
*/
|
||||
// This enables assert in release, and lets us have debug-only code
|
||||
#ifdef NDEBUG
|
||||
#define DEBUG
|
||||
#undef NDEBUG
|
||||
#endif // #ifdef NDEBUG
|
||||
|
||||
#if defined(HAVE_CONFIG_H)
|
||||
#include "config.h"
|
||||
#endif
|
||||
|
|
@ -75,18 +80,86 @@ void SafeRelease(T * ptr)
|
|||
}
|
||||
}
|
||||
|
||||
/* This wraps a critical section to track the owner in debug mode, adapted from
|
||||
* NSPR and http://blogs.msdn.com/b/oldnewthing/archive/2013/07/12/10433554.aspx */
|
||||
class owned_critical_section
|
||||
{
|
||||
public:
|
||||
|
||||
owned_critical_section()
|
||||
#ifdef DEBUG
|
||||
: owner(0)
|
||||
#endif
|
||||
{
|
||||
InitializeCriticalSection(&critical_section);
|
||||
}
|
||||
|
||||
~owned_critical_section()
|
||||
{
|
||||
DeleteCriticalSection(&critical_section);
|
||||
}
|
||||
|
||||
void enter()
|
||||
{
|
||||
EnterCriticalSection(&critical_section);
|
||||
#ifdef DEBUG
|
||||
assert(owner != GetCurrentThreadId(), "recursive locking");
|
||||
owner = GetCurrentThreadId();
|
||||
#endif
|
||||
}
|
||||
|
||||
void leave()
|
||||
{
|
||||
#ifdef DEBUG
|
||||
/* GetCurrentThreadId cannot return 0: it is not a the valid thread id */
|
||||
owner = 0;
|
||||
#endif
|
||||
LeaveCriticalSection(&critical_section);
|
||||
}
|
||||
|
||||
#ifdef DEBUG
|
||||
/* This is guaranteed to have the good behaviour if it succeeds. The behaviour
|
||||
* is undefined otherwise. */
|
||||
void assert_current_thread_owns()
|
||||
{
|
||||
/* This implies owner != 0, because GetCurrentThreadId cannot return 0. */
|
||||
assert(owner == GetCurrentThreadId());
|
||||
}
|
||||
#endif
|
||||
|
||||
private:
|
||||
CRITICAL_SECTION critical_section;
|
||||
#ifdef DEBUG
|
||||
DWORD owner;
|
||||
#endif
|
||||
};
|
||||
|
||||
struct auto_lock {
|
||||
auto_lock(CRITICAL_SECTION * lock)
|
||||
auto_lock(owned_critical_section * lock)
|
||||
:lock(lock)
|
||||
{
|
||||
EnterCriticalSection(lock);
|
||||
lock->enter();
|
||||
}
|
||||
~auto_lock()
|
||||
{
|
||||
LeaveCriticalSection(lock);
|
||||
lock->leave();
|
||||
}
|
||||
private:
|
||||
CRITICAL_SECTION * lock;
|
||||
owned_critical_section * lock;
|
||||
};
|
||||
|
||||
struct auto_unlock {
|
||||
auto_unlock(owned_critical_section * lock)
|
||||
:lock(lock)
|
||||
{
|
||||
lock->leave();
|
||||
}
|
||||
~auto_unlock()
|
||||
{
|
||||
lock->enter();
|
||||
}
|
||||
private:
|
||||
owned_critical_section * lock;
|
||||
};
|
||||
|
||||
struct auto_com {
|
||||
|
|
@ -186,7 +259,7 @@ struct cubeb_stream
|
|||
HANDLE thread;
|
||||
/* We synthetize our clock from the callbacks. */
|
||||
LONG64 clock;
|
||||
CRITICAL_SECTION stream_reset_lock;
|
||||
owned_critical_section * stream_reset_lock;
|
||||
/* Maximum number of frames we can be requested in a callback. */
|
||||
uint32_t buffer_frame_count;
|
||||
/* Resampler instance. Resampling will only happen if necessary. */
|
||||
|
|
@ -257,7 +330,7 @@ public:
|
|||
/* Close the stream */
|
||||
wasapi_stream_stop(stm);
|
||||
{
|
||||
auto_lock lock(&stm->stream_reset_lock);
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
close_wasapi_stream(stm);
|
||||
/* Reopen a stream and start it immediately. This will automatically pick the
|
||||
* new default device for this role. */
|
||||
|
|
@ -314,6 +387,12 @@ bool should_downmix(cubeb_stream * stream)
|
|||
return stream->mix_params.channels < stream->stream_params.channels;
|
||||
}
|
||||
|
||||
float stream_to_mix_samplerate_ratio(cubeb_stream * stream)
|
||||
{
|
||||
auto_lock lock(stream->stream_reset_lock);
|
||||
return float(stream->stream_params.rate) / stream->mix_params.rate;
|
||||
}
|
||||
|
||||
/* Upmix function, copies a mono channel in two interleaved
|
||||
* stereo channel. |out| has to be twice as long as |in| */
|
||||
template<typename T>
|
||||
|
|
@ -387,10 +466,10 @@ refill(cubeb_stream * stm, float * data, long frames_needed)
|
|||
dest = data;
|
||||
}
|
||||
|
||||
stm->clock = InterlockedAdd64(&stm->clock, frames_needed);
|
||||
|
||||
long out_frames = cubeb_resampler_fill(stm->resampler, dest, frames_needed);
|
||||
|
||||
stm->clock = InterlockedAdd64(&stm->clock, frames_needed * stream_to_mix_samplerate_ratio(stm));
|
||||
|
||||
/* XXX: Handle this error. */
|
||||
if (out_frames < 0) {
|
||||
assert(false);
|
||||
|
|
@ -545,13 +624,10 @@ HRESULT register_notification_client(cubeb_stream * stm)
|
|||
|
||||
HRESULT unregister_notification_client(cubeb_stream * stm)
|
||||
{
|
||||
HRESULT hr = CoCreateInstance(__uuidof(MMDeviceEnumerator),
|
||||
NULL, CLSCTX_INPROC_SERVER,
|
||||
IID_PPV_ARGS(&stm->device_enumerator));
|
||||
assert(stm);
|
||||
|
||||
if (FAILED(hr)) {
|
||||
LOG("Could not get device enumerator: %x", hr);
|
||||
return hr;
|
||||
if (!stm->device_enumerator) {
|
||||
return S_OK;
|
||||
}
|
||||
|
||||
stm->device_enumerator->UnregisterEndpointNotificationCallback(stm->notification_client);
|
||||
|
|
@ -847,8 +923,12 @@ int setup_wasapi_stream(cubeb_stream * stm)
|
|||
IMMDevice * device;
|
||||
WAVEFORMATEX * mix_format;
|
||||
|
||||
stm->stream_reset_lock->assert_current_thread_owns();
|
||||
|
||||
auto_com com;
|
||||
|
||||
assert(!stm->client && "WASAPI stream already setup, close it first.");
|
||||
|
||||
hr = get_default_endpoint(&device);
|
||||
if (FAILED(hr)) {
|
||||
LOG("Could not get default endpoint, error: %x", hr);
|
||||
|
|
@ -978,7 +1058,16 @@ wasapi_stream_init(cubeb * context, cubeb_stream ** stream,
|
|||
stm->draining = false;
|
||||
stm->latency = latency;
|
||||
stm->clock = 0;
|
||||
InitializeCriticalSection(&stm->stream_reset_lock);
|
||||
|
||||
/* Null out WASAPI-specific state */
|
||||
stm->resampler = nullptr;
|
||||
stm->client = nullptr;
|
||||
stm->render_client = nullptr;
|
||||
stm->audio_stream_volume = nullptr;
|
||||
stm->device_enumerator = nullptr;
|
||||
stm->notification_client = nullptr;
|
||||
|
||||
stm->stream_reset_lock = new owned_critical_section();
|
||||
|
||||
stm->shutdown_event = CreateEvent(NULL, 0, 0, NULL);
|
||||
stm->refill_event = CreateEvent(NULL, 0, 0, NULL);
|
||||
|
|
@ -996,9 +1085,15 @@ wasapi_stream_init(cubeb * context, cubeb_stream ** stream,
|
|||
return CUBEB_ERROR;
|
||||
}
|
||||
|
||||
rv = setup_wasapi_stream(stm);
|
||||
if (rv != CUBEB_OK) {
|
||||
return rv;
|
||||
{
|
||||
/* Locking here is not stricly necessary, because we don't have a
|
||||
notification client that can reset the stream yet, but it lets us
|
||||
assert that the lock is held in the function. */
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
rv = setup_wasapi_stream(stm);
|
||||
if (rv != CUBEB_OK) {
|
||||
return rv;
|
||||
}
|
||||
}
|
||||
|
||||
hr = register_notification_client(stm);
|
||||
|
|
@ -1017,18 +1112,29 @@ void close_wasapi_stream(cubeb_stream * stm)
|
|||
{
|
||||
assert(stm);
|
||||
|
||||
SafeRelease(stm->client);
|
||||
SafeRelease(stm->render_client);
|
||||
stm->stream_reset_lock->assert_current_thread_owns();
|
||||
|
||||
cubeb_resampler_destroy(stm->resampler);
|
||||
SafeRelease(stm->client);
|
||||
stm->client = nullptr;
|
||||
|
||||
SafeRelease(stm->render_client);
|
||||
stm->client = nullptr;
|
||||
|
||||
if (stm->resampler) {
|
||||
cubeb_resampler_destroy(stm->resampler);
|
||||
stm->resampler = nullptr;
|
||||
}
|
||||
|
||||
free(stm->mix_buffer);
|
||||
stm->mix_buffer = nullptr;
|
||||
}
|
||||
|
||||
void wasapi_stream_destroy(cubeb_stream * stm)
|
||||
{
|
||||
assert(stm);
|
||||
|
||||
unregister_notification_client(stm);
|
||||
|
||||
if (stm->thread) {
|
||||
SetEvent(stm->shutdown_event);
|
||||
WaitForSingleObject(stm->thread, INFINITE);
|
||||
|
|
@ -1038,11 +1144,13 @@ void wasapi_stream_destroy(cubeb_stream * stm)
|
|||
|
||||
SafeRelease(stm->shutdown_event);
|
||||
SafeRelease(stm->refill_event);
|
||||
DeleteCriticalSection(&stm->stream_reset_lock);
|
||||
|
||||
close_wasapi_stream(stm);
|
||||
{
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
close_wasapi_stream(stm);
|
||||
}
|
||||
|
||||
unregister_notification_client(stm);
|
||||
delete stm->stream_reset_lock;
|
||||
|
||||
free(stm);
|
||||
}
|
||||
|
|
@ -1051,7 +1159,7 @@ int wasapi_stream_start(cubeb_stream * stm)
|
|||
{
|
||||
HRESULT hr;
|
||||
|
||||
auto_lock lock(&stm->stream_reset_lock);
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
|
||||
assert(stm);
|
||||
|
||||
|
|
@ -1076,7 +1184,7 @@ int wasapi_stream_stop(cubeb_stream * stm)
|
|||
{
|
||||
assert(stm && stm->shutdown_event);
|
||||
|
||||
auto_lock lock(&stm->stream_reset_lock);
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
|
||||
SetEvent(stm->shutdown_event);
|
||||
|
||||
|
|
@ -1086,6 +1194,7 @@ int wasapi_stream_stop(cubeb_stream * stm)
|
|||
}
|
||||
|
||||
if (stm->thread) {
|
||||
auto_unlock lock(stm->stream_reset_lock);
|
||||
WaitForSingleObject(stm->thread, INFINITE);
|
||||
CloseHandle(stm->thread);
|
||||
stm->thread = NULL;
|
||||
|
|
@ -1109,7 +1218,7 @@ int wasapi_stream_get_latency(cubeb_stream * stm, uint32_t * latency)
|
|||
{
|
||||
assert(stm && latency);
|
||||
|
||||
auto_lock lock(&stm->stream_reset_lock);
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
|
||||
/* The GetStreamLatency method only works if the
|
||||
* AudioClient has been initialized. */
|
||||
|
|
@ -1132,7 +1241,7 @@ int wasapi_stream_set_volume(cubeb_stream * stm, float volume)
|
|||
/* up to 9.1 for now */
|
||||
float volumes[10];
|
||||
|
||||
auto_lock lock(&stm->stream_reset_lock);
|
||||
auto_lock lock(stm->stream_reset_lock);
|
||||
|
||||
hr = stm->audio_stream_volume->GetChannelCount(&channels);
|
||||
if (hr != S_OK) {
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче