[PATCH v2 0/1] MR3426: winegstreamer: Keep default decodebin queue limits for buffers and time.
Only make size unlimited, which fixes problems with Persona 4 while keeping the memory usage lower. -- v2: winegstreamer: Keep default decodebin queue limits. https://gitlab.winehq.org/wine/wine/-/merge_requests/3426
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/gst_private.h | 2 +- dlls/winegstreamer/main.c | 5 ++--- dlls/winegstreamer/media_source.c | 8 +------- dlls/winegstreamer/quartz_parser.c | 2 +- dlls/winegstreamer/unixlib.h | 1 - dlls/winegstreamer/wg_parser.c | 10 ---------- dlls/winegstreamer/wm_reader.c | 2 +- 7 files changed, 6 insertions(+), 24 deletions(-) diff --git a/dlls/winegstreamer/gst_private.h b/dlls/winegstreamer/gst_private.h index e32fa46a117..920ffc9d252 100644 --- a/dlls/winegstreamer/gst_private.h +++ b/dlls/winegstreamer/gst_private.h @@ -69,7 +69,7 @@ HRESULT wg_sample_queue_create(struct wg_sample_queue **out); void wg_sample_queue_destroy(struct wg_sample_queue *queue); void wg_sample_queue_flush(struct wg_sample_queue *queue, bool all); -wg_parser_t wg_parser_create(enum wg_parser_type type, bool unlimited_buffering); +wg_parser_t wg_parser_create(enum wg_parser_type type); void wg_parser_destroy(wg_parser_t parser); HRESULT wg_parser_connect(wg_parser_t parser, uint64_t file_size); diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index b651b45be69..a92d440c42f 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -66,17 +66,16 @@ bool array_reserve(void **elements, size_t *capacity, size_t count, size_t size) return TRUE; } -wg_parser_t wg_parser_create(enum wg_parser_type type, bool unlimited_buffering) +wg_parser_t wg_parser_create(enum wg_parser_type type) { struct wg_parser_create_params params = { .type = type, - .unlimited_buffering = unlimited_buffering, .err_on = ERR_ON(quartz), .warn_on = WARN_ON(quartz), }; - TRACE("type %#x, unlimited_buffering %d.\n", type, unlimited_buffering); + TRACE("type %#x.\n", type); if (WINE_UNIX_CALL(unix_wg_parser_create, ¶ms)) return 0; diff --git a/dlls/winegstreamer/media_source.c b/dlls/winegstreamer/media_source.c index e1e64fc7f98..7b0857158ae 100644 --- a/dlls/winegstreamer/media_source.c +++ b/dlls/winegstreamer/media_source.c @@ -1643,13 +1643,7 @@ static HRESULT media_source_create(struct object_context *context, IMFMediaSourc if (FAILED(hr = MFAllocateWorkQueue(&object->async_commands_queue))) goto fail; - /* In Media Foundation, sources may read from any media source stream - * without fear of blocking due to buffering limits on another. Trailmakers, - * a Unity3D Engine game, only reads one sample from the audio stream (and - * never deselects it). Remove buffering limits from decodebin in order to - * account for this. Note that this does leak memory, but the same memory - * leak occurs with native. */ - if (!(parser = wg_parser_create(WG_PARSER_DECODEBIN, true))) + if (!(parser = wg_parser_create(WG_PARSER_DECODEBIN))) { hr = E_OUTOFMEMORY; goto fail; diff --git a/dlls/winegstreamer/quartz_parser.c b/dlls/winegstreamer/quartz_parser.c index 734e2e48e1f..f9d8d79b259 100644 --- a/dlls/winegstreamer/quartz_parser.c +++ b/dlls/winegstreamer/quartz_parser.c @@ -1451,7 +1451,7 @@ static HRESULT parser_create(enum wg_parser_type type, struct parser **parser) if (!(object = calloc(1, sizeof(*object)))) return E_OUTOFMEMORY; - if (!(object->wg_parser = wg_parser_create(type, false))) + if (!(object->wg_parser = wg_parser_create(type))) { free(object); return E_OUTOFMEMORY; diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 66c363b5b6e..702bd7aa69b 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -212,7 +212,6 @@ struct wg_parser_create_params { wg_parser_t parser; wg_parser_type type; - UINT8 unlimited_buffering; UINT8 err_on; UINT8 warn_on; }; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 9c020cbf055..298c05c6b88 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -93,8 +93,6 @@ struct wg_parser bool sink_connected; - bool unlimited_buffering; - gchar *sink_caps; struct input_cache_chunk input_cache_chunks[4]; @@ -1771,13 +1769,6 @@ static BOOL decodebin_parser_init_gst(struct wg_parser *parser) gst_bin_add(GST_BIN(parser->container), element); parser->decodebin = element; - if (parser->unlimited_buffering) - { - g_object_set(parser->decodebin, "max-size-buffers", G_MAXUINT, NULL); - g_object_set(parser->decodebin, "max-size-time", G_MAXUINT64, NULL); - g_object_set(parser->decodebin, "max-size-bytes", G_MAXUINT, NULL); - } - g_signal_connect(element, "pad-added", G_CALLBACK(pad_added_cb), parser); g_signal_connect(element, "pad-removed", G_CALLBACK(pad_removed_cb), parser); g_signal_connect(element, "autoplug-continue", G_CALLBACK(autoplug_continue_cb), parser); @@ -1888,7 +1879,6 @@ static NTSTATUS wg_parser_create(void *args) pthread_cond_init(&parser->read_cond, NULL); pthread_cond_init(&parser->read_done_cond, NULL); parser->init_gst = init_funcs[params->type]; - parser->unlimited_buffering = params->unlimited_buffering; parser->err_on = params->err_on; parser->warn_on = params->warn_on; GST_DEBUG("Created winegstreamer parser %p.", parser); diff --git a/dlls/winegstreamer/wm_reader.c b/dlls/winegstreamer/wm_reader.c index 0dfe765c759..6440f8dbb60 100644 --- a/dlls/winegstreamer/wm_reader.c +++ b/dlls/winegstreamer/wm_reader.c @@ -1459,7 +1459,7 @@ static HRESULT init_stream(struct wm_reader *reader, QWORD file_size) HRESULT hr; WORD i; - if (!(wg_parser = wg_parser_create(WG_PARSER_DECODEBIN, false))) + if (!(wg_parser = wg_parser_create(WG_PARSER_DECODEBIN))) return E_OUTOFMEMORY; reader->wg_parser = wg_parser; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/3426
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=135377 Your paranoid android. === debian11b (64 bit WoW report) === win32u: win32u.c:1056: Test failed: win32u:win32u:0b98 done (0) in 0s 1337B
v2: Keep all default queue limits. I tried again with Trailmakers and P4G (which once required the size limit to be kept, but the game then changed their movie format and backend) and the unlimited buffering doesn't seem to make any difference anymore except using much more RAM. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3426#note_40634
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3426
v2: Keep all default queue limits.
I tried again with Trailmakers and P4G (which once required the size limit to be kept, but the game then changed their movie format and backend) and the unlimited buffering doesn't seem to make any difference anymore except using much more RAM.
That makes a lot of sense, thanks for testing. This has been on my list of things to reëxamine for some time, actually. It was originally added because mfplat sources don't get starved, but I later found that multiqueue (and hence decodebin) has logic in place specifically to prevent this kind of starvation, so yeah, we really shouldn't have needed that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/3426#note_40650
participants (3)
-
Marvin -
Rémi Bernon -
Zebediah Figura (＠zfigura)