[PATCH v4 0/1] MR2390: winegstreamer: Cache wg_parser input data.

[Derek Lesho (＠dlesho)]

On my Steam Deck, this reduces the time it takes to initialize the wg_parser radically (by around 1 second).  This helps in the game WILD HEARTS, which doesn't present while loading help videos during gameplay, causing large stutters.

Because GStreamer can randomly access the file with no known pattern on our side, I opted to implement this by buffering 4 chunks so that interleaved reads to different areas of the file don't cause us to discard and reload cached data more than we need to.

--
  v4: winegstreamer: Cache wg_parser input data.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390

[PATCH v4 1/1] winegstreamer: Cache wg_parser input data.

[Derek Lesho]

From: Derek Lesho <dlesho(a)codeweavers.com>

In order to reduce wg_parser initialization time by skipping the round-trip to the PE thread.

Signed-off-by: Derek Lesho <dlesho(a)codeweavers.com>
---
 dlls/winegstreamer/wg_parser.c | 168 ++++++++++++++++++++++++++++-----
 1 file changed, 147 insertions(+), 21 deletions(-)

diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index 5bb824f4399..e0ade940716 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -59,6 +59,12 @@ GST_DEBUG_CATEGORY(wine);
 
 typedef BOOL (*init_gst_cb)(struct wg_parser *parser);
 
+struct input_cache_chunk
+{
+    guint64 position;
+    uint8_t *data;
+};
+
 struct wg_parser
 {
     init_gst_cb init_gst;
@@ -96,7 +102,10 @@ struct wg_parser
     bool unlimited_buffering;
 
     gchar *sink_caps;
+
+    struct input_cache_chunk input_cache_chunks[4];
 };
+static unsigned int input_cache_chunk_size = 512 << 10;
 
 struct wg_parser_stream
 {
@@ -967,30 +976,10 @@ static void pad_removed_cb(GstElement *element, GstPad *pad, gpointer user)
     g_free(name);
 }
 
-static GstFlowReturn src_getrange_cb(GstPad *pad, GstObject *parent,
-        guint64 offset, guint size, GstBuffer **buffer)
+static GstFlowReturn issue_read_request(struct wg_parser *parser, guint64 offset, guint size, GstBuffer **buffer)
 {
-    struct wg_parser *parser = gst_pad_get_element_private(pad);
     GstFlowReturn ret;
 
-    GST_LOG("pad %p, offset %" G_GINT64_MODIFIER "u, size %u, buffer %p.", pad, offset, size, *buffer);
-
-    if (offset == GST_BUFFER_OFFSET_NONE)
-        offset = parser->next_pull_offset;
-    parser->next_pull_offset = offset + size;
-
-    if (!size)
-    {
-        /* asfreader occasionally asks for zero bytes. gst_buffer_map() will
-         * return NULL in this case. Avoid confusing the read thread by asking
-         * it for zero bytes. */
-        if (!*buffer)
-            *buffer = gst_buffer_new_and_alloc(0);
-        gst_buffer_set_size(*buffer, 0);
-        GST_LOG("Returning empty buffer.");
-        return GST_FLOW_OK;
-    }
-
     pthread_mutex_lock(&parser->mutex);
 
     assert(!parser->read_request.size);
@@ -1017,6 +1006,134 @@ static GstFlowReturn src_getrange_cb(GstPad *pad, GstObject *parent,
     return ret;
 }
 
+static struct input_cache_chunk * get_cache_entry(struct wg_parser *parser, guint64 position)
+{
+    struct input_cache_chunk chunk;
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE(parser->input_cache_chunks); i++)
+    {
+        chunk = parser->input_cache_chunks[i];
+
+        if (chunk.data && position == chunk.position)
+        {
+            if (i != 0)
+            {
+                memmove(&parser->input_cache_chunks[1], &parser->input_cache_chunks[0], i * sizeof(chunk));
+                parser->input_cache_chunks[0] = chunk;
+            }
+
+            return &parser->input_cache_chunks[0];
+        }
+    }
+
+    return NULL;
+}
+
+static gsize read_cached_chunk(struct wg_parser *parser, guint64 chunk_position, unsigned int chunk_offset, GstBuffer *buffer, guint64 buffer_offset)
+{
+    struct input_cache_chunk *chunk;
+    GstBuffer *chunk_buffer;
+    void *chunk_data;
+    GstFlowReturn ret;
+
+    if ((chunk = get_cache_entry(parser, chunk_position)))
+    {
+        return !!gst_buffer_fill(buffer, buffer_offset, chunk->data + chunk_offset, input_cache_chunk_size - chunk_offset) ? GST_FLOW_OK : GST_FLOW_ERROR;
+    }
+
+    chunk = &parser->input_cache_chunks[ ARRAY_SIZE(parser->input_cache_chunks) - 1 ];
+
+    if (!(chunk_data = chunk->data))
+        chunk_data = malloc(input_cache_chunk_size);
+
+    chunk_buffer = gst_buffer_new_wrapped_full(0, chunk_data, input_cache_chunk_size, 0, input_cache_chunk_size, NULL, NULL);
+    ret = issue_read_request(parser, chunk_position, input_cache_chunk_size, &chunk_buffer);
+    gst_buffer_unref(chunk_buffer);
+
+    if (ret != GST_FLOW_OK)
+    {
+        if (!chunk->data)
+            free(chunk_data);
+        return ret;
+    }
+
+    memmove(&parser->input_cache_chunks[1], &parser->input_cache_chunks[0], (ARRAY_SIZE(parser->input_cache_chunks) - 1) * sizeof(*chunk));
+    parser->input_cache_chunks[0].data = chunk_data;
+    parser->input_cache_chunks[0].position = chunk_position;
+
+    chunk = &parser->input_cache_chunks[0];
+    return !!gst_buffer_fill(buffer, buffer_offset, chunk->data + chunk_offset, input_cache_chunk_size - chunk_offset) ? GST_FLOW_OK : GST_FLOW_ERROR;
+}
+
+static GstFlowReturn read_input_cache(struct wg_parser *parser, guint64 offset, guint size, GstBuffer **buffer)
+{
+    unsigned int i, chunk_count, chunk_offset, buffer_offset = 0;
+    GstBuffer *working_buffer;
+    guint64 chunk_position;
+    GstFlowReturn ret;
+
+    working_buffer = *buffer;
+    if (!working_buffer)
+        working_buffer = gst_buffer_new_and_alloc(size);
+
+    chunk_position = offset - (offset % input_cache_chunk_size);
+    chunk_count = (offset + size + input_cache_chunk_size - chunk_position - 1) / input_cache_chunk_size;
+    chunk_offset = offset - chunk_position;
+
+    for (i = 0; i < chunk_count; i++)
+    {
+        if ((ret = read_cached_chunk(parser, chunk_position, chunk_offset, working_buffer, buffer_offset)) != GST_FLOW_OK)
+        {
+            if (!*buffer)
+                gst_buffer_unref(working_buffer);
+            return ret;
+        }
+
+        chunk_position += input_cache_chunk_size;
+        buffer_offset += input_cache_chunk_size - chunk_offset;
+        chunk_offset = 0;
+    }
+
+    *buffer = working_buffer;
+    return GST_FLOW_OK;
+}
+
+static GstFlowReturn src_getrange_cb(GstPad *pad, GstObject *parent,
+        guint64 offset, guint size, GstBuffer **buffer)
+{
+    struct wg_parser *parser = gst_pad_get_element_private(pad);
+
+    GST_LOG("pad %p, offset %" G_GINT64_MODIFIER "u, size %u, buffer %p.", pad, offset, size, *buffer);
+
+    if (offset == GST_BUFFER_OFFSET_NONE)
+        offset = parser->next_pull_offset;
+    parser->next_pull_offset = offset + size;
+
+    if (!size)
+    {
+        /* asfreader occasionally asks for zero bytes. gst_buffer_map() will
+         * return NULL in this case. Avoid confusing the read thread by asking
+         * it for zero bytes. */
+        if (!*buffer)
+            *buffer = gst_buffer_new_and_alloc(0);
+        gst_buffer_set_size(*buffer, 0);
+        GST_LOG("Returning empty buffer.");
+        return GST_FLOW_OK;
+    }
+
+    if (size >= input_cache_chunk_size || sizeof(void*) == 4)
+        return issue_read_request(parser, offset, size, buffer);
+
+    if (offset >= parser->file_size)
+        return GST_FLOW_EOS;
+
+    if ((offset + size) >= parser->file_size)
+        size = parser->file_size - offset;
+
+    return read_input_cache(parser, offset, size, buffer);
+}
+
 static gboolean src_query_cb(GstPad *pad, GstObject *parent, GstQuery *query)
 {
     struct wg_parser *parser = gst_pad_get_element_private(pad);
@@ -1562,6 +1679,15 @@ static NTSTATUS wg_parser_disconnect(void *args)
     g_free(parser->sink_caps);
     parser->sink_caps = NULL;
 
+    for (i = 0; i < ARRAY_SIZE(parser->input_cache_chunks); i++)
+    {
+        if (parser->input_cache_chunks[i].data)
+        {
+            free(parser->input_cache_chunks[i].data);
+            parser->input_cache_chunks[i].data = NULL;
+        }
+    }
+
     return S_OK;
 }
 
-- 
GitLab

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

[Derek Lesho (＠dlesho)]

bump

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390#note_29920

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
> bool unlimited_buffering;
>
> gchar *sink_caps;
> +
> +    struct input_cache_chunk input_cache_chunks[4];
>  };
> +static unsigned int input_cache_chunk_size = 512 << 10;

"static const"

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390#note_29969

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static gsize read_cached_chunk(struct wg_parser *parser, guint64 chunk_position, unsigned int chunk_offset, GstBuffer *buffer, guint64 buffer_offset)
> +{
> +    struct input_cache_chunk *chunk;
> +    GstBuffer *chunk_buffer;
> +    void *chunk_data;
> +    GstFlowReturn ret;
> +
> +    if ((chunk = get_cache_entry(parser, chunk_position)))
> +    {
> +        return !!gst_buffer_fill(buffer, buffer_offset, chunk->data + chunk_offset, input_cache_chunk_size - chunk_offset) ? GST_FLOW_OK : GST_FLOW_ERROR;
> This is an awfully long line; I'd just make this a normal if/else. There's another below.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390#note_29970

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/winegstreamer/wg_parser.c:
> +        if (chunk.data && position == chunk.position)
> +        {
> +            if (i != 0)
> +            {
> +                memmove(&parser->input_cache_chunks[1], &parser->input_cache_chunks[0], i * sizeof(chunk));
> +                parser->input_cache_chunks[0] = chunk;
> +            }
> +
> +            return &parser->input_cache_chunks[0];
> +        }
> +    }
> +
> +    return NULL;
> +}
> +
> +static gsize read_cached_chunk(struct wg_parser *parser, guint64 chunk_position, unsigned int chunk_offset, GstBuffer *buffer, guint64 buffer_offset)
> The return type should be GstFlowReturn.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390#note_29971

[Zebediah Figura (＠zfigura)]

Sorry for missing this, it seems that v4 didn't make it to the wine-gitlab list, so I didn't notice it had been pushed.

Besides a few nitpicks this looks mostly good.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/2390#note_29972