[PATCH 0/1] MR6432: winegstreamer: Allow application to drain queue.

[Brendan McGrath (＠redmcg)]

Pushing all queued input immediately causes gstreamer to process all
frames in advance and queue them in the output_queue of the transform.

This results in a large amount of memory usage that can cause an OOM
issue.

This patch allows the queue to be drained at the rate required by the
application.

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

[PATCH 1/1] winegstreamer: Allow application to drain queue.

[Brendan McGrath]

From: Brendan McGrath <bmcgrath(a)codeweavers.com>

Pushing all queued input immediately causes gstreamer to process all
frames in advance and queue them in the output_queue of the transform.

This results in a large amount of memory usage that can cause an OOM
issue.

This patch allows the queue to be drained at the rate required by the
application.
---
 dlls/winegstreamer/wg_transform.c | 68 +++++++++++++++++++------------
 1 file changed, 41 insertions(+), 27 deletions(-)

diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c
index deb99b78529..558f6572a1b 100644
--- a/dlls/winegstreamer/wg_transform.c
+++ b/dlls/winegstreamer/wg_transform.c
@@ -62,6 +62,8 @@ struct wg_transform
     GstCaps *desired_caps;
     GstCaps *output_caps;
     GstCaps *input_caps;
+
+    bool draining;
 };
 
 static struct wg_transform *get_transform(wg_transform_t trans)
@@ -783,6 +785,13 @@ NTSTATUS wg_transform_push_data(void *args)
     GstBuffer *buffer;
     guint length;
 
+    if (transform->draining)
+    {
+        GST_INFO("Refusing %u bytes, transform is draining", sample->size);
+        params->result = MF_E_NOTACCEPTING;
+        return STATUS_SUCCESS;
+    }
+
     length = gst_atomic_queue_length(transform->input_queue);
     if (length >= transform->attrs.input_queue_length + 1)
     {
@@ -997,6 +1006,33 @@ static NTSTATUS read_transform_output(struct wg_sample *sample, GstBuffer *buffe
     return STATUS_SUCCESS;
 }
 
+static NTSTATUS complete_drain(struct wg_transform *transform)
+{
+    if (transform->draining && gst_atomic_queue_length(transform->input_queue) == 0)
+    {
+        GstEvent *event;
+        transform->draining = false;
+        if (!(event = gst_event_new_segment_done(GST_FORMAT_TIME, -1))
+                || !push_event(transform->my_src, event))
+            goto error;
+        if (!(event = gst_event_new_eos())
+                || !push_event(transform->my_src, event))
+            goto error;
+        if (!(event = gst_event_new_stream_start("stream"))
+                || !push_event(transform->my_src, event))
+            goto error;
+        if (!(event = gst_event_new_segment(&transform->segment))
+                || !push_event(transform->my_src, event))
+            goto error;
+    }
+
+    return STATUS_SUCCESS;
+
+error:
+    GST_ERROR("Failed to drain transform %p.", transform);
+    return STATUS_UNSUCCESSFUL;
+}
+
 static bool get_transform_output(struct wg_transform *transform, struct wg_sample *sample)
 {
     GstBuffer *input_buffer;
@@ -1009,6 +1045,8 @@ static bool get_transform_output(struct wg_transform *transform, struct wg_sampl
     {
         if ((ret = gst_pad_push(transform->my_src, input_buffer)))
             GST_WARNING("Failed to push transform input, error %d", ret);
+
+        complete_drain(transform);
     }
 
     /* Remove the sample so the allocator cannot use it */
@@ -1124,36 +1162,12 @@ NTSTATUS wg_transform_get_status(void *args)
 NTSTATUS wg_transform_drain(void *args)
 {
     struct wg_transform *transform = get_transform(*(wg_transform_t *)args);
-    GstBuffer *input_buffer;
-    GstFlowReturn ret;
-    GstEvent *event;
 
-    GST_LOG("transform %p", transform);
+    GST_LOG("transform %p, draining %d buffers", transform, gst_atomic_queue_length(transform->input_queue));
 
-    while ((input_buffer = gst_atomic_queue_pop(transform->input_queue)))
-    {
-        if ((ret = gst_pad_push(transform->my_src, input_buffer)))
-            GST_WARNING("Failed to push transform input, error %d", ret);
-    }
+    transform->draining = true;
 
-    if (!(event = gst_event_new_segment_done(GST_FORMAT_TIME, -1))
-            || !push_event(transform->my_src, event))
-        goto error;
-    if (!(event = gst_event_new_eos())
-            || !push_event(transform->my_src, event))
-        goto error;
-    if (!(event = gst_event_new_stream_start("stream"))
-            || !push_event(transform->my_src, event))
-        goto error;
-    if (!(event = gst_event_new_segment(&transform->segment))
-            || !push_event(transform->my_src, event))
-        goto error;
-
-    return STATUS_SUCCESS;
-
-error:
-    GST_ERROR("Failed to drain transform %p.", transform);
-    return STATUS_UNSUCCESSFUL;
+    return complete_drain(transform);
 }
 
 NTSTATUS wg_transform_flush(void *args)
-- 
GitLab

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

[PATCH 1/1] winegstreamer: Allow application to drain queue.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=148164

Your paranoid android.


=== debian11b (64 bit WoW report) ===

user32:
sysparams.c:4717: Test failed: CreateProcessA failed, error 731

[PATCH 0/1] MR6432: winegstreamer: Allow application to drain queue. - approved

[Rémi Bernon]

This merge request was approved by Rémi Bernon.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6432

[Elizabeth Figura (＠zfigura)]

This is causing test failures for me:

```
0:00:00.544661099 16144 0x707ff2d0 ERROR                   WINE wg_transform.c:743:wg_transform_set_output_type: Failed to drain transform 0x7dd14930.
mfplat.c:2277: Test failed: vp 0 adv 0: Unexpected hr 0xd0000001.
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6432#note_80961

[Brendan McGrath (＠redmcg)]

On Wed Sep  4 21:40:41 2024 +0000, Elizabeth Figura wrote:
> This is causing test failures for me:
> ```
> 0:00:00.544661099 16144 0x707ff2d0 ERROR                   WINE
> wg_transform.c:743:wg_transform_set_output_type: Failed to drain
> transform 0x7dd14930.
> mfplat.c:2277: Test failed: vp 0 adv 0: Unexpected hr 0xd0000001.
> ```
> Thanks Zeb. I get this failure with the `master` branch. Does the test pass for you without this commit?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6432#note_81140

[Elizabeth Figura (＠zfigura)]

On Wed Sep  4 21:40:41 2024 +0000, Brendan McGrath wrote:
> Thanks Zeb. I get this failure with the `master` branch. Does the test
> pass for you without this commit?
> You're right, sorry, this is preexisting. I saw the drain-related error and assumed it was caused by this commit; I should have checked. Not sure when that was introduced...

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6432#note_81422

[PATCH 0/1] MR6432: winegstreamer: Allow application to drain queue. - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/6432