Module: wine
Branch: master
Commit: fcdc76388f6b9d5abfe17bdee1d2577b62866a96
URL: https://gitlab.winehq.org/wine/wine/-/commit/fcdc76388f6b9d5abfe17bdee1d257…
Author: Zebediah Figura <zfigura(a)codeweavers.com>
Date: Tue Jan 3 13:58:40 2023 -0600
winegstreamer: Explicitly sleep in the streaming thread when flushing.
The code originally intended to achieve this by simply waiting on flushing_cs,
and having the application thread hold flushing_cs while seeking. Unfortunately,
this can result in starvation of the application thread, since the streaming
thread always reacquires flushing_cs immediately after releasing it. Avoid this
by using a separate condition variable.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53403
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
dlls/winegstreamer/quartz_parser.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index 36ca84debba..5561b106327 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -54,20 +54,23 @@ struct parser
struct wg_parser *wg_parser;
- /* This protects the "streaming" field, accessed by both the application
- * and streaming threads.
+ /* This protects the "streaming" and "flushing" fields, accessed by both
+ * the application and streaming threads.
* We cannot use the filter lock for this, since that is held while waiting
* for the streaming thread, and hence the streaming thread cannot take the
* filter lock.
* This lock must not be acquired before acquiring the filter lock or
* flushing_cs. */
CRITICAL_SECTION streaming_cs;
+ CONDITION_VARIABLE flushing_cv;
/* FIXME: It would be nice to avoid duplicating these with strmbase.
* However, synchronization is tricky; we need access to be protected by a
* separate lock. */
bool streaming, sink_connected;
+ bool flushing;
+
HANDLE read_thread;
BOOL (*init_gst)(struct parser *filter);
@@ -986,11 +989,16 @@ static DWORD CALLBACK stream_thread(void *arg)
struct wg_parser_buffer buffer;
EnterCriticalSection(&filter->streaming_cs);
+
+ while (filter->flushing)
+ SleepConditionVariableCS(&filter->flushing_cv, &filter->streaming_cs, INFINITE);
+
if (!filter->streaming)
{
LeaveCriticalSection(&filter->streaming_cs);
break;
}
+
LeaveCriticalSection(&filter->streaming_cs);
EnterCriticalSection(&pin->flushing_cs);
@@ -1384,6 +1392,8 @@ static HRESULT parser_create(enum wg_parser_type type, struct parser **parser)
InitializeCriticalSection(&object->streaming_cs);
object->streaming_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": parser.streaming_cs");
+ InitializeConditionVariable(&object->flushing_cv);
+
*parser = object;
return S_OK;
}
@@ -1532,8 +1542,13 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface,
IAsyncReader_BeginFlush(filter->reader);
}
- /* Acquire the flushing locks. This blocks the streaming threads, and
- * ensures the seek is serialized between flushes. */
+ /* Signal the streaming threads to "pause". */
+ EnterCriticalSection(&filter->streaming_cs);
+ filter->flushing = true;
+ LeaveCriticalSection(&filter->streaming_cs);
+
+ /* Acquire the flushing locks, to make sure the streaming threads really
+ * are paused. This ensures the seek is serialized between flushes. */
for (i = 0; i < filter->source_count; ++i)
{
struct parser_source *flush_pin = filter->sources[i];
@@ -1576,6 +1591,12 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface,
}
}
+ /* Signal the streaming threads to resume. */
+ EnterCriticalSection(&filter->streaming_cs);
+ filter->flushing = false;
+ LeaveCriticalSection(&filter->streaming_cs);
+ WakeConditionVariable(&filter->flushing_cv);
+
return S_OK;
}
Module: wine
Branch: master
Commit: 9c531232779e70a7704b648375cf97c8fcab5f80
URL: https://gitlab.winehq.org/wine/wine/-/commit/9c531232779e70a7704b648375cf97…
Author: Zebediah Figura <zfigura(a)codeweavers.com>
Date: Tue Jan 3 13:47:06 2023 -0600
winegstreamer: Protect the "streaming" member of struct parser with a separate lock.
The code previously relied on inherent atomicity of atomic types, but atomicity
doesn't imply the right memory ordering. Be explicit about the threading model
we want.
Signed-off-by: Zebediah Figura <zfigura(a)codeweavers.com>
---
dlls/winegstreamer/quartz_parser.c | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index 8a227145d78..36ca84debba 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -54,6 +54,15 @@ struct parser
struct wg_parser *wg_parser;
+ /* This protects the "streaming" field, accessed by both the application
+ * and streaming threads.
+ * We cannot use the filter lock for this, since that is held while waiting
+ * for the streaming thread, and hence the streaming thread cannot take the
+ * filter lock.
+ * This lock must not be acquired before acquiring the filter lock or
+ * flushing_cs. */
+ CRITICAL_SECTION streaming_cs;
+
/* FIXME: It would be nice to avoid duplicating these with strmbase.
* However, synchronization is tricky; we need access to be protected by a
* separate lock. */
@@ -972,10 +981,18 @@ static DWORD CALLBACK stream_thread(void *arg)
TRACE("Starting streaming thread for pin %p.\n", pin);
- while (filter->streaming)
+ for (;;)
{
struct wg_parser_buffer buffer;
+ EnterCriticalSection(&filter->streaming_cs);
+ if (!filter->streaming)
+ {
+ LeaveCriticalSection(&filter->streaming_cs);
+ break;
+ }
+ LeaveCriticalSection(&filter->streaming_cs);
+
EnterCriticalSection(&pin->flushing_cs);
if (pin->eos)
@@ -1095,6 +1112,9 @@ static void parser_destroy(struct strmbase_filter *iface)
wg_parser_destroy(filter->wg_parser);
+ filter->streaming_cs.DebugInfo->Spare[0] = 0;
+ DeleteCriticalSection(&filter->streaming_cs);
+
strmbase_sink_cleanup(&filter->sink);
strmbase_filter_cleanup(&filter->filter);
free(filter);
@@ -1167,7 +1187,9 @@ static HRESULT parser_cleanup_stream(struct strmbase_filter *iface)
if (!filter->sink_connected)
return S_OK;
+ EnterCriticalSection(&filter->streaming_cs);
filter->streaming = false;
+ LeaveCriticalSection(&filter->streaming_cs);
for (i = 0; i < filter->source_count; ++i)
{
@@ -1359,6 +1381,9 @@ static HRESULT parser_create(enum wg_parser_type type, struct parser **parser)
return E_OUTOFMEMORY;
}
+ InitializeCriticalSection(&object->streaming_cs);
+ object->streaming_cs.DebugInfo->Spare[0] = (DWORD_PTR)(__FILE__ ": parser.streaming_cs");
+
*parser = object;
return S_OK;
}