wasapi: Fix possible deadlock while downwards state change

IAudioClient::Stop() doesn't seem to wake up the event handle,
then read() or write() could be blocked forever by WaitForSingleObject.

Part-of: <https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/-/merge_requests/1329>
This commit is contained in:
Seungha Yang 2020-06-09 22:38:28 +09:00 committed by GStreamer Merge Bot
parent 3e507dc073
commit 7ab51e85ab
4 changed files with 61 additions and 6 deletions

View file

@ -175,6 +175,7 @@ gst_wasapi_sink_init (GstWasapiSink * self)
self->low_latency = DEFAULT_LOW_LATENCY; self->low_latency = DEFAULT_LOW_LATENCY;
self->try_audioclient3 = DEFAULT_AUDIOCLIENT3; self->try_audioclient3 = DEFAULT_AUDIOCLIENT3;
self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL); self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL);
self->cancellable = CreateEvent (NULL, TRUE, FALSE, NULL);
self->client_needs_restart = FALSE; self->client_needs_restart = FALSE;
CoInitializeEx (NULL, COINIT_MULTITHREADED); CoInitializeEx (NULL, COINIT_MULTITHREADED);
@ -190,6 +191,11 @@ gst_wasapi_sink_dispose (GObject * object)
self->event_handle = NULL; self->event_handle = NULL;
} }
if (self->cancellable != NULL) {
CloseHandle (self->cancellable);
self->cancellable = NULL;
}
if (self->client != NULL) { if (self->client != NULL) {
IUnknown_Release (self->client); IUnknown_Release (self->client);
self->client = NULL; self->client = NULL;
@ -564,6 +570,9 @@ gst_wasapi_sink_prepare (GstAudioSink * asink, GstAudioRingBufferSpec * spec)
res = TRUE; res = TRUE;
/* reset cancellable event handle */
ResetEvent (self->cancellable);
beach: beach:
/* unprepare() is not called if prepare() fails, but we want it to be, so call /* unprepare() is not called if prepare() fails, but we want it to be, so call
* it manually when needed */ * it manually when needed */
@ -600,6 +609,10 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
gint16 *dst = NULL; gint16 *dst = NULL;
DWORD dwWaitResult; DWORD dwWaitResult;
guint can_frames, have_frames, n_frames, write_len, written_len = 0; guint can_frames, have_frames, n_frames, write_len, written_len = 0;
HANDLE event_handle[2];
event_handle[0] = self->event_handle;
event_handle[1] = self->cancellable;
GST_OBJECT_LOCK (self); GST_OBJECT_LOCK (self);
if (self->client_needs_restart) { if (self->client_needs_restart) {
@ -607,6 +620,7 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self, HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self,
GST_OBJECT_UNLOCK (self); goto err); GST_OBJECT_UNLOCK (self); goto err);
self->client_needs_restart = FALSE; self->client_needs_restart = FALSE;
ResetEvent (self->cancellable);
} }
GST_OBJECT_UNLOCK (self); GST_OBJECT_UNLOCK (self);
@ -615,13 +629,19 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
if (self->sharemode == AUDCLNT_SHAREMODE_EXCLUSIVE) { if (self->sharemode == AUDCLNT_SHAREMODE_EXCLUSIVE) {
/* In exclusive mode we have to wait always */ /* In exclusive mode we have to wait always */
dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE); dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
if (dwWaitResult != WAIT_OBJECT_0) { if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
GST_ERROR_OBJECT (self, "Error waiting for event handle: %x", GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
(guint) dwWaitResult); (guint) dwWaitResult);
goto err; goto err;
} }
/* ::reset was requested */
if (dwWaitResult == WAIT_OBJECT_0 + 1) {
GST_DEBUG_OBJECT (self, "operation was cancelled");
return -1;
}
can_frames = gst_wasapi_sink_get_can_frames (self); can_frames = gst_wasapi_sink_get_can_frames (self);
if (can_frames < 0) { if (can_frames < 0) {
GST_ERROR_OBJECT (self, "Error getting frames to write to"); GST_ERROR_OBJECT (self, "Error getting frames to write to");
@ -645,12 +665,19 @@ gst_wasapi_sink_write (GstAudioSink * asink, gpointer data, guint length)
} }
if (can_frames == 0) { if (can_frames == 0) {
dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE); dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
if (dwWaitResult != WAIT_OBJECT_0) { if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
GST_ERROR_OBJECT (self, "Error waiting for event handle: %x", GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
(guint) dwWaitResult); (guint) dwWaitResult);
goto err; goto err;
} }
/* ::reset was requested */
if (dwWaitResult == WAIT_OBJECT_0 + 1) {
GST_DEBUG_OBJECT (self, "operation was cancelled");
return -1;
}
can_frames = gst_wasapi_sink_get_can_frames (self); can_frames = gst_wasapi_sink_get_can_frames (self);
if (can_frames < 0) { if (can_frames < 0) {
GST_ERROR_OBJECT (self, "Error getting frames to write to"); GST_ERROR_OBJECT (self, "Error getting frames to write to");
@ -713,6 +740,8 @@ gst_wasapi_sink_reset (GstAudioSink * asink)
if (!self->client) if (!self->client)
return; return;
SetEvent (self->cancellable);
GST_OBJECT_LOCK (self); GST_OBJECT_LOCK (self);
hr = IAudioClient_Stop (self->client); hr = IAudioClient_Stop (self->client);
HR_FAILED_AND (hr, IAudioClient::Stop,); HR_FAILED_AND (hr, IAudioClient::Stop,);

View file

@ -44,6 +44,7 @@ struct _GstWasapiSink
IAudioClient *client; IAudioClient *client;
IAudioRenderClient *render_client; IAudioRenderClient *render_client;
HANDLE event_handle; HANDLE event_handle;
HANDLE cancellable;
/* Client was reset, so it needs to be started again */ /* Client was reset, so it needs to be started again */
gboolean client_needs_restart; gboolean client_needs_restart;

View file

@ -189,6 +189,7 @@ gst_wasapi_src_init (GstWasapiSrc * self)
self->low_latency = DEFAULT_LOW_LATENCY; self->low_latency = DEFAULT_LOW_LATENCY;
self->try_audioclient3 = DEFAULT_AUDIOCLIENT3; self->try_audioclient3 = DEFAULT_AUDIOCLIENT3;
self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL); self->event_handle = CreateEvent (NULL, FALSE, FALSE, NULL);
self->cancellable = CreateEvent (NULL, TRUE, FALSE, NULL);
self->client_needs_restart = FALSE; self->client_needs_restart = FALSE;
self->adapter = gst_adapter_new (); self->adapter = gst_adapter_new ();
@ -205,6 +206,11 @@ gst_wasapi_src_dispose (GObject * object)
self->event_handle = NULL; self->event_handle = NULL;
} }
if (self->cancellable != NULL) {
CloseHandle (self->cancellable);
self->cancellable = NULL;
}
if (self->client_clock != NULL) { if (self->client_clock != NULL) {
IUnknown_Release (self->client_clock); IUnknown_Release (self->client_clock);
self->client_clock = NULL; self->client_clock = NULL;
@ -516,7 +522,12 @@ gst_wasapi_src_prepare (GstAudioSrc * asrc, GstAudioRingBufferSpec * spec)
(self)->ringbuffer, self->positions); (self)->ringbuffer, self->positions);
res = TRUE; res = TRUE;
/* reset cancellable event handle */
ResetEvent (self->cancellable);
beach: beach:
/* unprepare() is not called if prepare() fails, but we want it to be, so call /* unprepare() is not called if prepare() fails, but we want it to be, so call
* it manually when needed */ * it manually when needed */
if (!res) if (!res)
@ -568,6 +579,7 @@ gst_wasapi_src_read (GstAudioSrc * asrc, gpointer data, guint length,
HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self, HR_FAILED_ELEMENT_ERROR_AND (hr, IAudioClient::Start, self,
GST_OBJECT_UNLOCK (self); goto err); GST_OBJECT_UNLOCK (self); goto err);
self->client_needs_restart = FALSE; self->client_needs_restart = FALSE;
ResetEvent (self->cancellable);
gst_adapter_clear (self->adapter); gst_adapter_clear (self->adapter);
} }
@ -585,15 +597,25 @@ gst_wasapi_src_read (GstAudioSrc * asrc, gpointer data, guint length,
while (wanted > 0) { while (wanted > 0) {
DWORD dwWaitResult; DWORD dwWaitResult;
guint got_frames, avail_frames, n_frames, want_frames, read_len; guint got_frames, avail_frames, n_frames, want_frames, read_len;
HANDLE event_handle[2];
event_handle[0] = self->event_handle;
event_handle[1] = self->cancellable;
/* Wait for data to become available */ /* Wait for data to become available */
dwWaitResult = WaitForSingleObject (self->event_handle, INFINITE); dwWaitResult = WaitForMultipleObjects (2, event_handle, FALSE, INFINITE);
if (dwWaitResult != WAIT_OBJECT_0) { if (dwWaitResult != WAIT_OBJECT_0 && dwWaitResult != WAIT_OBJECT_0 + 1) {
GST_ERROR_OBJECT (self, "Error waiting for event handle: %x", GST_ERROR_OBJECT (self, "Error waiting for event handle: %x",
(guint) dwWaitResult); (guint) dwWaitResult);
goto err; goto err;
} }
/* ::reset was requested */
if (dwWaitResult == WAIT_OBJECT_0 + 1) {
GST_DEBUG_OBJECT (self, "operation was cancelled");
return -1;
}
hr = IAudioCaptureClient_GetBuffer (self->capture_client, hr = IAudioCaptureClient_GetBuffer (self->capture_client,
(BYTE **) & from, &got_frames, &flags, NULL, NULL); (BYTE **) & from, &got_frames, &flags, NULL, NULL);
if (hr != S_OK) { if (hr != S_OK) {
@ -685,6 +707,8 @@ gst_wasapi_src_reset (GstAudioSrc * asrc)
if (!self->client) if (!self->client)
return; return;
SetEvent (self->cancellable);
GST_OBJECT_LOCK (self); GST_OBJECT_LOCK (self);
hr = IAudioClient_Stop (self->client); hr = IAudioClient_Stop (self->client);
HR_FAILED_RET (hr, IAudioClock::Stop,); HR_FAILED_RET (hr, IAudioClock::Stop,);

View file

@ -46,6 +46,7 @@ struct _GstWasapiSrc
guint64 client_clock_freq; guint64 client_clock_freq;
IAudioCaptureClient *capture_client; IAudioCaptureClient *capture_client;
HANDLE event_handle; HANDLE event_handle;
HANDLE cancellable;
/* Smooth frames captured from WASAPI, which can be irregular sometimes */ /* Smooth frames captured from WASAPI, which can be irregular sometimes */
GstAdapter *adapter; GstAdapter *adapter;
/* Client was reset, so it needs to be started again */ /* Client was reset, so it needs to be started again */