[PATCH v3 0/2] MR8282: winegstreamer: Don't hold lock during wg_parser_stream_get_buffer.
This fixes a looping audio issue during seek in VRChat. The issue is that on Windows the stop request in handled immediately thus the AVPro MFT (which loops the audio) can be immediately flushed. Wine currently waits for a pending sample request to be completed before handling the stop request. As a result, the AVPro MFT loops audio whilst it waits for another sample (or to be flushed).
This MR releases the source CS before calling `wg_parser_stream_get_buffer` so it does not delay the handling of the stop request. This appears to be safe as the Unix side of `wg_parser_stream_get_buffer` has its own set of primitives.
-- v3: winegstreamer: Don't hold lock during wg_parser_stream_get_buffer.
From: Brendan McGrath bmcgrath@codeweavers.com
--- dlls/winegstreamer/wg_parser.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index e806298fb57..1a29015356e 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -1792,7 +1792,9 @@ static NTSTATUS wg_parser_disconnect(void *args) for (i = 0; i < parser->stream_count; ++i) { parser->streams[i]->flushing = true; + parser->streams[i]->eos = true; pthread_cond_signal(&parser->streams[i]->event_empty_cond); + pthread_cond_signal(&parser->streams[i]->event_cond); } pthread_mutex_unlock(&parser->mutex);
From: Brendan McGrath bmcgrath@codeweavers.com
--- dlls/winegstreamer/media_source.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index ab842b3e438..b9ff1e87007 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -791,6 +791,20 @@ static HRESULT media_stream_send_eos(struct media_source *source, struct media_s return S_OK; }
+static bool stream_get_buffer(struct media_stream *stream, struct wg_parser_buffer *buffer) +{ + struct media_source *source = impl_from_IMFMediaSource(stream->media_source); + wg_parser_stream_t wg_stream = stream->wg_stream; + wg_parser_t wg_parser = source->wg_parser; + bool ret; + + LeaveCriticalSection(&source->cs); + ret = wg_parser_stream_get_buffer(wg_parser, wg_stream, buffer); + EnterCriticalSection(&source->cs); + + return ret; +} + static HRESULT wait_on_sample(struct media_stream *stream, IUnknown *token) { struct media_source *source = impl_from_IMFMediaSource(stream->media_source); @@ -798,13 +812,16 @@ static HRESULT wait_on_sample(struct media_stream *stream, IUnknown *token)
TRACE("%p, %p\n", stream, token);
- while (wg_parser_stream_get_buffer(source->wg_parser, stream->wg_stream, &buffer)) + while (stream_get_buffer(stream, &buffer)) { HRESULT hr = media_stream_send_sample(stream, &buffer, token); if (hr != S_FALSE) return hr; }
+ if (source->state == SOURCE_SHUTDOWN) + return S_OK; + return media_stream_send_eos(source, stream); }
Sorry for the slow review.
This is, generally speaking, done intentionally, and it's suspicious that it'd matter. The idea is that a sample should generally only take a few ms to decode (let alone only demux), and in order to safely synchronize it's generally easiest to wait for that thread to stop. Interrupting the wait is kind of doable for winegstreamer, because we're waiting on a separate thread, but on Windows it wouldn't be a separate thread and wouldn't be interruptible.
Is there something unusual about this situation? What kind of format is the original file? How long are we waiting for another sample?
Is there something unusual about this situation? What kind of format is the original file?
I've been testing with videos from YouTube. They're HLS files (H.264/AAC). So they are streamed from the internet, which likely slows things down. I've attached the pipeline.
Note that downloading, demuxing and decoding all take place in the media source.
[0.00.15.408422079-wg_parser_caps.dot](/uploads/4dd6e05a14964a23d2336c87a440ed9c/0.00.15.408422079-wg_parser_caps.dot)
How long are we waiting for another sample?
It's not uncommon to wait ~2 seconds. Here's an example following thread 0x072c (which has just called `wait_on_sample`):
``` 184.777:0214:072c:trace:mfplat:wait_on_sample 00000003AB0F7CB0, 0000000000000000 186.314:0214:072c:trace:mfplat:MFCreateMemoryBuffer 1382400, 00000003878CFC80. ```
The delay is from GStreamer. Here's a sample of the delays as reported by GStreamer: ``` 0:00:56.613337219 5274 0x7f5b64038a70 INFO videodecoder gstvideodecoder.c:3731:gst_video_decoder_clip_and_push_buf:<avdec_h264-1> First buffer since flush took 0:00:00.938238397 to produce 0:00:58.779292706 5274 0x7f5b64038a70 INFO videodecoder gstvideodecoder.c:3731:gst_video_decoder_clip_and_push_buf:<avdec_h264-1> First buffer since flush took 0:00:02.142121758 to produce 0:00:59.931777332 5274 0x7f5b64038a70 INFO videodecoder gstvideodecoder.c:3731:gst_video_decoder_clip_and_push_buf:<avdec_h264-1> First buffer since flush took 0:00:01.128113862 to produce 0:01:01.117394275 5274 0x7f5b64038a70 INFO videodecoder gstvideodecoder.c:3731:gst_video_decoder_clip_and_push_buf:<avdec_h264-1> First buffer since flush took 0:00:01.165114977 to produce ```
on Windows it wouldn't be a separate thread and wouldn't be interruptible
It's possible Windows solves this a different way. Maybe I can write a test to confirm.
Is there something unusual about this situation? What kind of format is the original file?
I've been testing with videos from YouTube. They're HLS files (H.264/AAC). So they are streamed from the internet, which likely slows things down. I've attached the pipeline.
Note that downloading, demuxing and decoding all take place in the media source.
Ah, yes, that would be unusual. The current code is built around the assumption that as long as control is in the parser it will never actually *sleep*—either it's actively demuxing, or it's currently in the downstream chain function. If that's not the case, then yeah, we have to explicitly unblock the get_buffer function like this.
on Windows it wouldn't be a separate thread and wouldn't be interruptible
It's possible Windows solves this a different way. Maybe I can write a test to confirm.
Maybe, but as long as the demuxer component is running in pull mode, it's going to be necessary to do something like this.
The pipeline I attached above is from Proton, the Wine pipeline is a little different:
[0.00.02.072525578-wg_parser_caps.dot](/uploads/ff3ce471d93dfe7ec0335655617dddfd/0.00.02.072525578-wg_parser_caps.dot)
The main difference is where the HLS file comes from. On Wine, the HLS comes from an `IMFByteStream` (which wraps an `IStream` created with `URLOpenBlockingStreamW`). This byte stream is then processed by a `decodebin` element.
On Proton, the URL is passed directly to a `urldecodebin` element, so no `IMFByteStream` is used. Instead the `urldecodebin` uses a `souphttpsrc` element to directly fetch the HLS file.
The HLS is a simple ASCII file though. It contains the list of TS files for the actual video. So on both Wine and Proton, the actual TS files are downloaded by a `souphttpsrc` element within the `hlsdemux` element.
as long as the demuxer component is running in pull mode, it's going to be necessary to do something like this.
So I suspect that even though the demuxer components on this occasion are running in push mode, because the TS is not being pushed by our `src` pad (within `wg_parser`), then our only option is likely to unblock before calling `wg_parser_stream_get_buffer`. Let me know if you agree.
Maybe I can write a test to confirm.
Well, I couldn't find a way to get an `IMFMediaSource` for the HLS format, but I did come up with a test using MP4:
https://gitlab.winehq.org/redmcg/wine/-/merge_requests/4
The test works by artificially delaying/preventing the read of the entire file. The MP4 file used in this test has the metadata at the start of the file (it is usually at the end). The test then allows no more than the first 512KB of the file to be read (I had to use 512KB as our caching in `wg_parser.c` expects to read that much and doesn't handle truncated reads).
The test then just requests samples from a single stream until it is exhausted (the test assumes a stream is exhausted when it doesn't receive a requested sample within 5 seconds).
With this sample request still outstanding, the test then tries other requests on the media source, such as requesting a sample from the other stream, or calling `IMFMediaSource::Stop`. On Windows, the new request is handled in less than a second. On Wine, it waits until the current sample request is complete (thus the tests fail and are marked `todo_wine`).
I found that this MR fixes all the `todo_wine`s but for `the `IMFMediaSource::Shutdown`. See:
I'm getting crashes and hangs running mf:mf with these patches.
On Fri Jul 4 00:48:27 2025 +0000, Elizabeth Figura wrote:
I'm getting crashes and hangs running mf:mf with these patches.
Hmm. I tried running locally, and I did get one freeze. But it looks like @rbernon has already raised MR !8415 to address this.
So I tried again after merging that MR, and MR !8402 for good measure. I was also hitting one test that seemed flaky, so I marked it `flaky_wine`. With all that in place, I ran the test over 100 times without a single test failure (or freeze or crash):
[test_summary.log](/uploads/42fb44af4f1897945cf84d3022731c05/test_summary.log)
Could you please try again with this branch:
https://gitlab.winehq.org/redmcg/wine/-/commits/bm_cw_25439_wait_on_sample_w...
I'm curious if this fixes it for you, or if maybe it's a timing issue which just doesn't occur on my PC.
This merge request was approved by Elizabeth Figura.
Not sure if I messed up the test, or if it's fixed upstream, by 8402 or 8415, but it seems to work now.
-
Brendan McGrath -
Brendan McGrath (＠redmcg)
-
Elizabeth Figura (＠zfigura)