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.
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.