[PATCH v3 0/1] MR105: winegstreamer: reintroduce flushing for wg_parser
Previous commit removed flushing but this causes issues in some games (Fallout 3 mainly). - add flushing - move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down. - Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in. original commit 5144b27661fcd6705353d832e0383085f8afe842 that removed flushing EOS set in `quartz_parser.c` appropriatly lets radio loop in Fallout 3, otherwise looped audio hits an infinite 'blip-blip-blip' and radio songs ending will similarly skip a few times. -- v3: winegstreamer: reintroduce flushing for wg_parser https://gitlab.winehq.org/wine/wine/-/merge_requests/105
From: zlice <zlice555(a)gmail.com> Previous commit removed flushing but this causes issues in some games. - add flushing back - move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down. - Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in. --- dlls/winegstreamer/gst_private.h | 3 ++ dlls/winegstreamer/main.c | 10 +++++++ dlls/winegstreamer/media_source.c | 13 +++++---- dlls/winegstreamer/quartz_parser.c | 6 ++++ dlls/winegstreamer/unixlib.h | 3 ++ dlls/winegstreamer/wg_parser.c | 44 ++++++++++++++++++++++++++++-- dlls/winegstreamer/wm_reader.c | 21 +++++++++----- 7 files changed, 85 insertions(+), 15 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index 159143d7e54..bacf0c8443b 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -70,6 +70,9 @@ void wg_parser_destroy(struct wg_parser *parser); HRESULT wg_parser_connect(struct wg_parser *parser, uint64_t file_size); void wg_parser_disconnect(struct wg_parser *parser); +void wg_parser_begin_flush(struct wg_parser *parser); +void wg_parser_end_flush(struct wg_parser *parser); + bool wg_parser_get_next_read_offset(struct wg_parser *parser, uint64_t *offset, uint32_t *size); void wg_parser_push_data(struct wg_parser *parser, const void *data, uint32_t size); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index 5075b3118cd..257740401a7 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -107,6 +107,16 @@ void wg_parser_disconnect(struct wg_parser *parser) __wine_unix_call(unix_handle, unix_wg_parser_disconnect, parser); } +void wg_parser_begin_flush(struct wg_parser *parser) +{ + __wine_unix_call(unix_handle, unix_wg_parser_begin_flush, parser); +} + +void wg_parser_end_flush(struct wg_parser *parser) +{ + __wine_unix_call(unix_handle, unix_wg_parser_end_flush, parser); +} + bool wg_parser_get_next_read_offset(struct wg_parser *parser, uint64_t *offset, uint32_t *size) { struct wg_parser_get_next_read_offset_params params = diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index f07b83f413e..e1040df045c 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -392,6 +392,7 @@ static void start_pipeline(struct media_source *source, struct source_async_comm if (position->vt == VT_I8) wg_parser_stream_seek(source->streams[0]->wg_stream, 1.0, position->hVal.QuadPart, 0, AM_SEEKING_AbsolutePositioning, AM_SEEKING_NoPositioning); + wg_parser_end_flush(source->wg_parser); for (i = 0; i < source->stream_count; i++) flush_token_queue(source->streams[i], position->vt == VT_EMPTY); @@ -419,6 +420,8 @@ static void stop_pipeline(struct media_source *source) { unsigned int i; + wg_parser_begin_flush(source->wg_parser); + for (i = 0; i < source->stream_count; i++) { struct media_stream *stream = source->streams[i]; @@ -537,12 +540,10 @@ static void wait_on_sample(struct media_stream *stream, IUnknown *token) { send_buffer(stream, &buffer, token); } - else - { - stream->eos = TRUE; - IMFMediaEventQueue_QueueEventParamVar(stream->event_queue, MEEndOfStream, &GUID_NULL, S_OK, &empty_var); - dispatch_end_of_presentation(stream->parent_source); - } + /* + There was code here always setting EOS but previous versions + would return. Assuming EOS is set in wg_parser_stream_get_buffer. + */ } static HRESULT WINAPI source_async_commands_Invoke(IMFAsyncCallback *iface, IMFAsyncResult *result) diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 34848c0b503..e54ebac5e7d 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -973,6 +973,7 @@ static HRESULT parser_init_stream(struct strmbase_filter *iface) return S_OK; filter->streaming = true; + wg_parser_end_flush(filter->wg_parser); for (i = 0; i < filter->source_count; ++i) { @@ -1030,6 +1031,7 @@ static HRESULT parser_cleanup_stream(struct strmbase_filter *iface) return S_OK; filter->streaming = false; + wg_parser_begin_flush(filter->wg_parser); for (i = 0; i < filter->source_count; ++i) { @@ -1354,6 +1356,8 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface, if (!(current_flags & AM_SEEKING_NoFlush)) { + wg_parser_begin_flush(filter->wg_parser); + for (i = 0; i < filter->source_count; ++i) { if (filter->sources[i]->pin.pin.peer) @@ -1381,6 +1385,8 @@ static HRESULT WINAPI GST_Seeking_SetPositions(IMediaSeeking *iface, if (!(current_flags & AM_SEEKING_NoFlush)) { + wg_parser_end_flush(filter->wg_parser); + for (i = 0; i < filter->source_count; ++i) { struct parser_source *flush_pin = filter->sources[i]; diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index e8cdfaf7217..817f7ac351c 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -269,6 +269,9 @@ enum unix_funcs unix_wg_parser_connect, unix_wg_parser_disconnect, + unix_wg_parser_begin_flush, + unix_wg_parser_end_flush, + unix_wg_parser_get_next_read_offset, unix_wg_parser_push_data, diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 7d55897aa0a..7eb5b6674e5 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -87,7 +87,7 @@ struct wg_parser GstFlowReturn ret; } read_request; - bool sink_connected; + bool flushing, sink_connected; bool unlimited_buffering; }; @@ -126,6 +126,35 @@ static NTSTATUS wg_parser_get_stream(void *args) return S_OK; } +static NTSTATUS wg_parser_begin_flush(void *args) +{ + struct wg_parser *parser = args; + unsigned int i; + + pthread_mutex_lock(&parser->mutex); + parser->flushing = true; + pthread_mutex_unlock(&parser->mutex); + + for (i = 0; i < parser->stream_count; ++i) + { + if (parser->streams[i]->enabled) + pthread_cond_signal(&parser->streams[i]->event_cond); + } + + return S_OK; +} + +static NTSTATUS wg_parser_end_flush(void *args) +{ + struct wg_parser *parser = args; + + pthread_mutex_lock(&parser->mutex); + parser->flushing = false; + pthread_mutex_unlock(&parser->mutex); + + return S_OK; +} + static NTSTATUS wg_parser_get_next_read_offset(void *args) { struct wg_parser_get_next_read_offset_params *params = args; @@ -270,9 +299,16 @@ static NTSTATUS wg_parser_stream_get_buffer(void *args) pthread_mutex_lock(&parser->mutex); - while (!stream->eos && !stream->buffer) + while (!parser->flushing && !stream->eos && !stream->buffer) pthread_cond_wait(&stream->event_cond, &parser->mutex); + if (parser->flushing) + { + pthread_mutex_unlock(&parser->mutex); + GST_DEBUG("Filter is flushing.\n"); + return VFW_E_WRONG_STATE; + } + /* Note that we can both have a buffer and stream->eos, in which case we * must return the buffer. */ if ((buffer = stream->buffer)) @@ -1569,6 +1605,7 @@ static NTSTATUS wg_parser_create(void *args) pthread_cond_init(&parser->init_cond, NULL); pthread_cond_init(&parser->read_cond, NULL); pthread_cond_init(&parser->read_done_cond, NULL); + parser->flushing = true; parser->init_gst = init_funcs[params->type]; parser->unlimited_buffering = params->unlimited_buffering; @@ -1605,6 +1642,9 @@ const unixlib_entry_t __wine_unix_call_funcs[] = X(wg_parser_connect), X(wg_parser_disconnect), + X(wg_parser_begin_flush), + X(wg_parser_end_flush), + X(wg_parser_get_next_read_offset), X(wg_parser_push_data), diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 03adea8a318..ae109c79be5 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -1512,6 +1512,7 @@ static HRESULT init_stream(struct wm_reader *reader, QWORD file_size) wg_parser_stream_enable(stream->wg_stream, &stream->format); } + wg_parser_end_flush(reader->wg_parser); /* We probably discarded events because streams weren't enabled yet. * Now that they're all enabled seek back to the start again. */ wg_parser_stream_seek(reader->streams[0].wg_stream, 1.0, 0, 0, @@ -1878,10 +1879,6 @@ HRESULT wm_reader_get_stream_sample(struct wm_reader *reader, WORD stream_number struct wg_parser_stream *wg_stream; struct wg_parser_buffer wg_buffer; struct wm_stream *stream; - DWORD size, capacity; - INSSBuffer *sample; - HRESULT hr; - BYTE *data; for (;;) { @@ -1914,16 +1911,26 @@ HRESULT wm_reader_get_stream_sample(struct wm_reader *reader, WORD stream_number if (stream->eos) return NS_E_NO_MORE_SAMPLES; + /* + There was code here always setting EOS but previous versions + would return. Assuming EOS is set in wg_parser_stream_get_buffer. + EOS was set at the end of this func before. + Could use has_duration below? + */ if (!wg_parser_stream_get_buffer(wg_stream, &wg_buffer)) { - stream->eos = true; - TRACE("End of stream.\n"); - return NS_E_NO_MORE_SAMPLES; + FIXME("Stream is flushing.\n"); + return E_NOTIMPL; } } TRACE("Got buffer for '%s' stream %p.\n", get_major_type_string(stream->format.major_type), stream); + DWORD size, capacity; + INSSBuffer *sample; + HRESULT hr; + BYTE *data; + if (callback_advanced && stream->read_compressed && stream->allocate_stream) { if (FAILED(hr = IWMReaderCallbackAdvanced_AllocateForStream(callback_advanced, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/105
On 6/1/22 20:58, zlice wrote:
From: zlice <zlice555(a)gmail.com>
Previous commit removed flushing but this causes issues in some games.
- add flushing back - move variable creation after initial condition returns in wm_reader_get_stream_sample. This caused some slow down. - Remove EOS sets. Some are not what flushing did before. Had some seg faults on exiting Fallout 3 with them in.
As stated in my original reply to this patch [1]: I don't think "reintroduce flushing" is the right answer here, not without understanding why it matters. The point of 5144b2766 is that flushing should not make a difference. It allows wg_parser_stream_get_event() to return more quickly, but that same cost is added to the subsequent seek or stop request, so GST_Seeking_SetPositions() or parser_cleanup_stream() will end up taking just as long. If flushing does make a difference, I think we need to understand why, and quite likely solve this a different way. [1] https://www.winehq.org/pipermail/wine-devel/2022-May/217461.html
participants (3)
-
Zebediah Figura -
zlice -
zlice (＠zlice)