[PATCH v3 0/1] MR105: winegstreamer: reintroduce flushing for wg_parser

[zlice (＠zlice)]

Previous commit removed flushing but this causes issues in some games (Fallout 3 mainly).

- add flushing
- move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down.
- Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in.

original commit 5144b27661fcd6705353d832e0383085f8afe842 that removed flushing

EOS set in `quartz_parser.c` appropriatly lets radio loop in Fallout 3, otherwise looped audio hits an infinite 'blip-blip-blip' and radio songs ending will similarly skip a few times.

--
  v3: winegstreamer: reintroduce flushing for wg_parser

https://gitlab.winehq.org/wine/wine/-/merge_requests/105

[PATCH v3 1/1] winegstreamer: reintroduce flushing for wg_parser

[zlice]

From: zlice

Previous commit removed flushing but this causes issues in some games.

- add flushing back
- move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down.
- Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in.
---
 dlls/winegstreamer/gst_private.h   |  3 ++
 dlls/winegstreamer/main.c          | 10 +++++++
 dlls/winegstreamer/media_source.c  | 13 +++++----
 dlls/winegstreamer/quartz_parser.c |  6 ++++
 dlls/winegstreamer/unixlib.h       |  3 ++
 dlls/winegstreamer/wg_parser.c     | 44 ++++++++++++++++++++++++++++--
 dlls/winegstreamer/wm_reader.c     | 21 +++++++++-----
 7 files changed, 85 insertions(+), 15 deletions(-)

diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h
index 159143d7e54..bacf0c8443b 100644
--- a/dlls/winegstreamer/gst_private.h
+++ b/dlls/winegstreamer/gst_private.h
@@ -70,6 +70,9 @@ void wg_parser_destroy(struct wg_parser *parser);
 HRESULT wg_parser_connect(struct wg_parser *parser, uint64_t file_size);
 void wg_parser_disconnect(struct wg_parser *parser);

+void wg_parser_begin_flush(struct wg_parser *parser);
+void wg_parser_end_flush(struct wg_parser *parser);
+
 bool wg_parser_get_next_read_offset(struct wg_parser *parser, uint64_t *offset, uint32_t *size);
 void wg_parser_push_data(struct wg_parser *parser, const void *data, uint32_t size);

diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c
index 5075b3118cd..257740401a7 100644
--- a/dlls/winegstreamer/main.c
+++ b/dlls/winegstreamer/main.c
@@ -107,6 +107,16 @@ void wg_parser_disconnect(struct wg_parser *parser)
     __wine_unix_call(unix_handle, unix_wg_parser_disconnect, parser);
 }

+void wg_parser_begin_flush(struct wg_parser *parser)
+{
+    __wine_unix_call(unix_handle, unix_wg_parser_begin_flush, parser);
+}
+
+void wg_parser_end_flush(struct wg_parser *parser)
+{
+    __wine_unix_call(unix_handle, unix_wg_parser_end_flush, parser);
+}
+
 bool wg_parser_get_next_read_offset(struct wg_parser *parser, uint64_t *offset, uint32_t *size)
 {
     struct wg_parser_get_next_read_offset_params params =
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c
index f07b83f413e..e1040df045c 100644
--- a/dlls/winegstreamer/media_source.c
+++ b/dlls/winegstreamer/media_source.c
@@ -392,6 +392,7 @@ static void start_pipeline(struct media_source *source, struct source_async_comm
     if (position->vt == VT_I8)
         wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, position->hVal.QuadPart, 0,
                 AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning);
+    wg_parser_end_flush(source->wg_parser);

for (i = 0; i < source->stream_count; i++)
         flush_token_queue(source->streams[i], position->vt == VT_EMPTY);
@@ -419,6 +420,8 @@ static void stop_pipeline(struct media_source *source)
 {
     unsigned int i;

+    wg_parser_begin_flush(source->wg_parser);
+
     for (i = 0; i < source->stream_count; i++)
     {
         struct media_stream *stream = source->streams[i];
@@ -537,12 +540,10 @@ static void wait_on_sample(struct media_stream *stream, IUnknown *token)
     {
         send_buffer(stream, &buffer, token);
     }
-    else
-    {
-        stream->eos = TRUE;
-        IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEEndOfStream, &GUID_NULL, S_OK, &empty_var);
-        dispatch_end_of_presentation(stream->parent_source);
-    }
+    /*
+      There was code here always setting EOS but previous versions
+      would return. Assuming EOS is set in wg_parser_stream_get_buffer.
+    */
 }

static HRESULT WINAPI source_async_commands_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result)
diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c
index 34848c0b503..e54ebac5e7d 100644
--- a/dlls/winegstreamer/quartz_parser.c
+++ b/dlls/winegstreamer/quartz_parser.c
@@ -973,6 +973,7 @@ static HRESULT parser_init_stream(struct strmbase_filter *iface)
         return S_OK;

filter->streaming = true;
+    wg_parser_end_flush(filter->wg_parser);

for (i = 0; i < filter->source_count; ++i)
     {
@@ -1030,6 +1031,7 @@ static HRESULT parser_cleanup_stream(struct strmbase_filter *iface)
         return S_OK;

filter->streaming = false;
+    wg_parser_begin_flush(filter->wg_parser);

for (i = 0; i < filter->source_count; ++i)
     {
@@ -1354,6 +1356,8 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface,

if (!(current_flags & AM_SEEKING_NoFlush))
     {
+        wg_parser_begin_flush(filter->wg_parser);
+
         for (i = 0; i < filter->source_count; ++i)
         {
             if (filter->sources[i]->pin.pin.peer)
@@ -1381,6 +1385,8 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface,

if (!(current_flags & AM_SEEKING_NoFlush))
     {
+        wg_parser_end_flush(filter->wg_parser);
+
         for (i = 0; i < filter->source_count; ++i)
         {
             struct parser_source *flush_pin = filter->sources[i];
diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h
index e8cdfaf7217..817f7ac351c 100644
--- a/dlls/winegstreamer/unixlib.h
+++ b/dlls/winegstreamer/unixlib.h
@@ -269,6 +269,9 @@ enum unix_funcs
     unix_wg_parser_connect,
     unix_wg_parser_disconnect,

+    unix_wg_parser_begin_flush,
+    unix_wg_parser_end_flush,
+
     unix_wg_parser_get_next_read_offset,
     unix_wg_parser_push_data,

diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index 7d55897aa0a..7eb5b6674e5 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -87,7 +87,7 @@ struct wg_parser
         GstFlowReturn ret;
     } read_request;

-    bool sink_connected;
+    bool flushing, sink_connected;

bool unlimited_buffering;
 };
@@ -126,6 +126,35 @@ static NTSTATUS wg_parser_get_stream(void *args)
     return S_OK;
 }

+static NTSTATUS wg_parser_begin_flush(void *args)
+{
+    struct wg_parser *parser = args;
+    unsigned int i;
+
+    pthread_mutex_lock(&parser->mutex);
+    parser->flushing = true;
+    pthread_mutex_unlock(&parser->mutex);
+
+    for (i = 0; i < parser->stream_count; ++i)
+    {
+        if (parser->streams[i]->enabled)
+            pthread_cond_signal(&parser->streams[i]->event_cond);
+    }
+
+    return S_OK;
+}
+
+static NTSTATUS wg_parser_end_flush(void *args)
+{
+    struct wg_parser *parser = args;
+
+    pthread_mutex_lock(&parser->mutex);
+    parser->flushing = false;
+    pthread_mutex_unlock(&parser->mutex);
+
+    return S_OK;
+}
+
 static NTSTATUS wg_parser_get_next_read_offset(void *args)
 {
     struct wg_parser_get_next_read_offset_params *params = args;
@@ -270,9 +299,16 @@ static NTSTATUS wg_parser_stream_get_buffer(void *args)

pthread_mutex_lock(&parser->mutex);

-    while (!stream->eos && !stream->buffer)
+    while (!parser->flushing && !stream->eos && !stream->buffer)
         pthread_cond_wait(&stream->event_cond, &parser->mutex);

+    if (parser->flushing)
+    {
+        pthread_mutex_unlock(&parser->mutex);
+        GST_DEBUG("Filter is flushing.\n");
+        return VFW_E_WRONG_STATE;
+    }
+
     /* Note that we can both have a buffer and stream->eos, in which case we
      * must return the buffer. */
     if ((buffer = stream->buffer))
@@ -1569,6 +1605,7 @@ static NTSTATUS wg_parser_create(void *args)
     pthread_cond_init(&parser->init_cond, NULL);
     pthread_cond_init(&parser->read_cond, NULL);
     pthread_cond_init(&parser->read_done_cond, NULL);
+    parser->flushing = true;
     parser->init_gst = init_funcs[params->type];
     parser->unlimited_buffering = params->unlimited_buffering;

@@ -1605,6 +1642,9 @@ const unixlib_entry_t __wine_unix_call_funcs[] =
     X(wg_parser_connect),
     X(wg_parser_disconnect),

+    X(wg_parser_begin_flush),
+    X(wg_parser_end_flush),
+
     X(wg_parser_get_next_read_offset),
     X(wg_parser_push_data),

diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c
index 03adea8a318..ae109c79be5 100644
--- a/dlls/winegstreamer/wm_reader.c
+++ b/dlls/winegstreamer/wm_reader.c
@@ -1512,6 +1512,7 @@ static HRESULT init_stream(struct wm_reader *reader, QWORD file_size)
         wg_parser_stream_enable(stream->wg_stream, &stream->format);
     }

+    wg_parser_end_flush(reader->wg_parser);
     /* We probably discarded events because streams weren't enabled yet.
      * Now that they're all enabled seek back to the start again. */
     wg_parser_stream_seek(reader->streams[0].wg_stream, 1.0, 0, 0,
@@ -1878,10 +1879,6 @@ HRESULT wm_reader_get_stream_sample(struct wm_reader *reader, WORD stream_number
     struct wg_parser_stream *wg_stream;
     struct wg_parser_buffer wg_buffer;
     struct wm_stream *stream;
-    DWORD size, capacity;
-    INSSBuffer *sample;
-    HRESULT hr;
-    BYTE *data;

for (;;)
     {
@@ -1914,16 +1911,26 @@ HRESULT wm_reader_get_stream_sample(struct wm_reader *reader, WORD stream_number
             if (stream->eos)
                 return NS_E_NO_MORE_SAMPLES;

+            /*
+              There was code here always setting EOS but previous versions
+              would return. Assuming EOS is set in wg_parser_stream_get_buffer.
+              EOS was set at the end of this func before.
+              Could use has_duration below?
+            */
             if (!wg_parser_stream_get_buffer(wg_stream, &wg_buffer))
             {
-                stream->eos = true;
-                TRACE("End of stream.\n");
-                return NS_E_NO_MORE_SAMPLES;
+                FIXME("Stream is flushing.\n");
+                return E_NOTIMPL;
             }
         }

TRACE("Got buffer for '%s' stream %p.\n", get_major_type_string(stream->format.major_type), stream);

+        DWORD size, capacity;
+        INSSBuffer *sample;
+        HRESULT hr;
+        BYTE *data;
+
         if (callback_advanced && stream->read_compressed && stream->allocate_stream)
         {
             if (FAILED(hr = IWMReaderCallbackAdvanced_AllocateForStream(callback_advanced,

[PATCH v3 1/1] winegstreamer: reintroduce flushing for wg_parser

[Zebediah Figura]

On 6/1/22 20:58, zlice wrote:
> From: zlice
>
> Previous commit removed flushing but this causes issues in some games.
> - add flushing back
> - move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down.
> - Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in.

As stated in my original reply to this patch [1]:

I don't think "reintroduce flushing" is the right answer here, not
without understanding why it matters.

The point of 5144b2766 is that flushing should not make a difference. It
allows wg_parser_stream_get_event() to return more quickly, but that
same cost is added to the subsequent seek or stop request, so
GST_Seeking_SetPositions() or parser_cleanup_stream() will end up taking
just as long.

If flushing does make a difference, I think we need to understand why,
and quite likely solve this a different way.

[1] https://www.winehq.org/pipermail/wine-devel/2022-May/217461.html