[PATCH v3 0/2] MR5255: winegstreamer: Append an optional parser before decoders in wg_transform.
Supersedes https://gitlab.winehq.org/wine/wine/-/merge_requests/5090 -- v3: winegstreamer: Append an optional parser before decoders. winegstreamer: Release sink caps in the error path. https://gitlab.winehq.org/wine/wine/-/merge_requests/5255
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/wg_transform.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 516b28e82e2..c252142d13d 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -338,7 +338,7 @@ NTSTATUS wg_transform_create(void *args) struct wg_format output_format = *params->output_format; struct wg_format input_format = *params->input_format; GstElement *first = NULL, *last = NULL, *element; - GstCaps *raw_caps = NULL, *src_caps = NULL; + GstCaps *sink_caps = NULL, *src_caps = NULL; NTSTATUS status = STATUS_UNSUCCESSFUL; GstPadTemplate *template = NULL; struct wg_transform *transform; @@ -395,7 +395,7 @@ NTSTATUS wg_transform_create(void *args) * raw output media type should be enough. */ media_type = gst_structure_get_name(gst_caps_get_structure(transform->output_caps, 0)); - if (!(raw_caps = gst_caps_new_empty_simple(media_type))) + if (!(sink_caps = gst_caps_new_empty_simple(media_type))) goto out; switch (input_format.major_type) @@ -408,12 +408,9 @@ NTSTATUS wg_transform_create(void *args) case WG_MAJOR_TYPE_VIDEO_INDEO: case WG_MAJOR_TYPE_VIDEO_WMV: case WG_MAJOR_TYPE_VIDEO_MPEG1: - if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) + if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, sink_caps)) || !append_element(transform->container, element, &first, &last)) - { - gst_caps_unref(raw_caps); goto out; - } break; case WG_MAJOR_TYPE_AUDIO: @@ -421,12 +418,9 @@ NTSTATUS wg_transform_create(void *args) break; case WG_MAJOR_TYPE_UNKNOWN: GST_FIXME("Format %u not implemented!", input_format.major_type); - gst_caps_unref(raw_caps); goto out; } - gst_caps_unref(raw_caps); - switch (output_format.major_type) { case WG_MAJOR_TYPE_AUDIO: @@ -507,6 +501,7 @@ NTSTATUS wg_transform_create(void *args) goto out; gst_caps_unref(src_caps); + gst_caps_unref(sink_caps); GST_INFO("Created winegstreamer transform %p.", transform); params->transform = (wg_transform_t)(ULONG_PTR)transform; @@ -521,6 +516,8 @@ out: gst_object_unref(transform->my_src); if (src_caps) gst_caps_unref(src_caps); + if (sink_caps) + gst_caps_unref(sink_caps); if (transform->allocator) wg_allocator_destroy(transform->allocator); if (transform->drain_query) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5255
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/winegstreamer/quartz_transform.c | 3 ++- dlls/winegstreamer/unixlib.h | 1 + dlls/winegstreamer/wg_transform.c | 21 +++++++++++++++++++-- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/dlls/winegstreamer/quartz_transform.c b/dlls/winegstreamer/quartz_transform.c index 5189c0b22d3..760b46950bc 100644 --- a/dlls/winegstreamer/quartz_transform.c +++ b/dlls/winegstreamer/quartz_transform.c @@ -99,7 +99,8 @@ static HRESULT transform_init_stream(struct strmbase_filter *iface) { struct transform *filter = impl_from_strmbase_filter(iface); struct wg_format input_format, output_format; - struct wg_transform_attrs attrs = {0}; + /* FIXME: GStreamer mpegvideoparse causes some test failures when inserted */ + struct wg_transform_attrs attrs = {.no_parser = TRUE}; HRESULT hr; if (filter->source.pin.peer) diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index 4ec9fce515e..b1975bae182 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -338,6 +338,7 @@ struct wg_transform_attrs UINT32 input_queue_length; BOOL allow_size_change; BOOL low_latency; + BOOL no_parser; }; struct wg_transform_create_params diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index c252142d13d..b2669907068 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -338,7 +338,7 @@ NTSTATUS wg_transform_create(void *args) struct wg_format output_format = *params->output_format; struct wg_format input_format = *params->input_format; GstElement *first = NULL, *last = NULL, *element; - GstCaps *sink_caps = NULL, *src_caps = NULL; + GstCaps *sink_caps = NULL, *src_caps = NULL, *parsed_caps = NULL; NTSTATUS status = STATUS_UNSUCCESSFUL; GstPadTemplate *template = NULL; struct wg_transform *transform; @@ -390,6 +390,11 @@ NTSTATUS wg_transform_create(void *args) gst_pad_set_query_function(transform->my_sink, transform_sink_query_cb); gst_pad_set_chain_function(transform->my_sink, transform_sink_chain_cb); + media_type = gst_structure_get_name(gst_caps_get_structure(src_caps, 0)); + if (!(parsed_caps = gst_caps_new_empty_simple(media_type))) + goto out; + gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, NULL); + /* Since we append conversion elements, we don't want to filter decoders * based on the actual output caps now. Matching decoders with the * raw output media type should be enough. @@ -408,7 +413,16 @@ NTSTATUS wg_transform_create(void *args) case WG_MAJOR_TYPE_VIDEO_INDEO: case WG_MAJOR_TYPE_VIDEO_WMV: case WG_MAJOR_TYPE_VIDEO_MPEG1: - if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, sink_caps)) + if (!transform->attrs.no_parser && (element = find_element(GST_ELEMENT_FACTORY_TYPE_PARSER, src_caps, parsed_caps)) + && !append_element(transform->container, element, &first, &last)) + goto out; + else + { + gst_caps_unref(parsed_caps); + parsed_caps = gst_caps_ref(src_caps); + } + + if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, parsed_caps, sink_caps)) || !append_element(transform->container, element, &first, &last)) goto out; break; @@ -500,6 +514,7 @@ NTSTATUS wg_transform_create(void *args) || !push_event(transform->my_src, event)) goto out; + gst_caps_unref(parsed_caps); gst_caps_unref(src_caps); gst_caps_unref(sink_caps); @@ -516,6 +531,8 @@ out: gst_object_unref(transform->my_src); if (src_caps) gst_caps_unref(src_caps); + if (parsed_caps) + gst_caps_unref(parsed_caps); if (sink_caps) gst_caps_unref(sink_caps); if (transform->allocator) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/5255
v3: Fix some test failures introduced by the previous change, though it wasn't completely right in the first place: use the parsed caps only to find a decoder a leave src_caps alone as it's later used when feeding the pipeline input caps. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5255#note_63907
```
+ media_type = gst_structure_get_name(gst_caps_get_structure(src_caps, 0)); + if (!(parsed_caps = gst_caps_new_empty_simple(media_type))) + goto out; + gst_caps_set_simple(parsed_caps, "parsed", G_TYPE_BOOLEAN, true, NULL);
This doesn't work, because not all caps use "parsed" for this. For example, AAC uses "framed" instead.
And, like I said in 5255, I don't see how restricting the caps is necessary. What parser are you imagining would parse to the wrong format?
Working around the mpegaudioparse problem does not seem unreasonable to me, but I don't think doing it from the frontend makes sense at all. I'd also really rather not have specific workarounds in the code until the actual problem is understood.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/5255#note_64119
This doesn't work, because not all caps use "parsed" for this. For example, AAC uses "framed" instead.
At best it does nothing, it does not "not work". Anyway I've removed that.
Working around the mpegaudioparse problem does not seem unreasonable to me, but I don't think doing it from the frontend makes sense at all. I'd also really rather not have specific workarounds in the code until the actual problem is understood.
The problem resides in mpegvideoparse and the quartz tests, so having the workaround in the quartz mpeg decoder sounds resaonable. A bit more specifically, mpegvideoparse holds on some buffer we pass from the tests because it decides that it needs more data to tell whether they really are on frame boundaries or not, when it gets more and passes more data to ffmpeg the decoder in turn gets confused because it does not find the boundaries it expects. Splitting the frame data differently in the test makes the test pass. I think this is a non-issue anyway, the test comment suggests that native outputs buffer when EOS is seen anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/5255#note_64123
participants (2)
-
Rémi Bernon -
Zebediah Figura (＠zfigura)