[PATCH v2 0/1] MR7824: winegstreamer: Handle the Stream Group Done event.

[Brendan McGrath (＠redmcg)]

This event indicates that there will be no more data. Handling it like
an EOS fixes a dead lock that occurs when a stream completes.

--
  v2: winegstreamer: Handle the Stream Group Done event.

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

[PATCH v2 1/1] winegstreamer: Handle the Stream Group Done event.

[Brendan McGrath]

From: Brendan McGrath

This event indicates that there will be no more data. Handling it like
an EOS fixes a dead lock that occurs when a stream completes.
---
 dlls/winegstreamer/wg_parser.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index 710cfe6a0a5..fc14eaf3a98 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -598,6 +598,13 @@ static gboolean sink_event_cb(GstPad *pad, GstObject *parent, GstEvent *event)
             break;
         }

+        /* decodebin collects EOS and sends them only when all streams are EOS.
+         * In place it sends stream-group-done notifications for individual
+         * streams. This is mainly meant to accommodate chained OGGs. However,
+         * Windows is generally capable of reading from arbitrary streams while
+         * ignoring others, and should still send EOS in that case.
+         * Therefore translate stream-group-done back to EOS. */
+        case GST_EVENT_STREAM_GROUP_DONE:
         case GST_EVENT_EOS:
             pthread_mutex_lock(&parser->mutex);
             stream->eos = true;

[Brendan McGrath (＠redmcg)]

> I guess native is supposed to not block in that case

That's a good point. If we have two streams call `IMFMediaStream::RequestSample`, then we will process them in sequence rather than in parallel. That is, one stream will have to wait for the other to receive data (as we block all other streams once the source cs is acquired in `source_async_commands_Invoke`).

This MR will at least unblock a stream when there is no more data, but Windows probably delivers data for the other stream immediately (i.e. it can deliver data for one stream, even if the other was already waiting). And I believe I recall seeing audio latency in Wine as a result of this. From memory, it was a 4K video, so delivery of the video sample took a while and as a result the audio fell behind. We should probably look to lock the individual streams rather than the entire source when possible.
> It's probably worth adding a comment here to that effect.

Good point again. I've pushed your comment verbatim.

[Elizabeth Figura (＠zfigura)]

> > I guess native is supposed to not block in that case
>
> That's a good point. If we have two streams call `IMFMediaStream::RequestSample`, then we will process them in sequence rather than in parallel. That is, one stream will have to wait for the other to receive data (as we block all other streams once the source cs is acquired in `source_async_commands_Invoke`).
>
> This MR will at least unblock a stream when there is no more data, but Windows probably delivers data for the other stream immediately (i.e. it can deliver data for one stream, even if the other was already waiting). And I believe I recall seeing audio latency in Wine as a result of this. From memory, it was a 4K video, so delivery of the video sample took a while and as a result the audio fell behind. We should probably look to lock the individual streams rather than the entire source when possible.

Hrm, I didn't think we had a locking problem, but if we do that should be fixed, yeah.

[PATCH v2 0/1] MR7824: winegstreamer: Handle the Stream Group Done event. - approved

[Elizabeth Figura (＠zfigura)]

This merge request was approved by Elizabeth Figura.