[PATCH 0/7] MR2640: winegstreamer: Factor some common helpers between wg_parser and wg_transform.
From: Rémi Bernon rbernon@codeweavers.com
Making sure GStreamer unix-side initialization is done early and doesn't slow down pipeline creation too much. --- dlls/winegstreamer/Makefile.in | 1 + dlls/winegstreamer/main.c | 3 ++ dlls/winegstreamer/unix_private.h | 16 ++++++- dlls/winegstreamer/unixlib.c | 76 +++++++++++++++++++++++++++++++ dlls/winegstreamer/unixlib.h | 2 + dlls/winegstreamer/wg_allocator.c | 3 -- dlls/winegstreamer/wg_format.c | 3 -- dlls/winegstreamer/wg_parser.c | 42 +---------------- dlls/winegstreamer/wg_transform.c | 6 --- 9 files changed, 99 insertions(+), 53 deletions(-) create mode 100644 dlls/winegstreamer/unixlib.c
diff --git a/dlls/winegstreamer/Makefile.in b/dlls/winegstreamer/Makefile.in index ae6420bad84..1c701bfa9f6 100644 --- a/dlls/winegstreamer/Makefile.in +++ b/dlls/winegstreamer/Makefile.in @@ -16,6 +16,7 @@ C_SRCS = \ quartz_parser.c \ quartz_transform.c \ resampler.c \ + unixlib.c \ video_decoder.c \ video_processor.c \ wg_allocator.c \ diff --git a/dlls/winegstreamer/main.c b/dlls/winegstreamer/main.c index ce59baaab3b..77c1550fbfe 100644 --- a/dlls/winegstreamer/main.c +++ b/dlls/winegstreamer/main.c @@ -616,6 +616,9 @@ static BOOL CALLBACK init_gstreamer_proc(INIT_ONCE *once, void *param, void **ct { HINSTANCE handle;
+ if (WINE_UNIX_CALL(unix_wg_process_attach, NULL)) + return FALSE; + /* Unloading glib is a bad idea.. it installs atexit handlers, * so never unload the dll after loading */ GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_FROM_ADDRESS | GET_MODULE_HANDLE_EX_FLAG_PIN, diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index ce67134af16..0838a4dec6e 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -25,13 +25,25 @@
#include <gst/gst.h>
-extern bool init_gstreamer(void) DECLSPEC_HIDDEN; +/* unixlib.c */ + +GST_DEBUG_CATEGORY_EXTERN(wine) DECLSPEC_HIDDEN; +#define GST_CAT_DEFAULT wine + +extern NTSTATUS wg_process_attach(void *args) DECLSPEC_HIDDEN; + +/* wg_parser.c */ + extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN;
+/* wg_format.c */ + extern void wg_format_from_caps(struct wg_format *format, const GstCaps *caps) DECLSPEC_HIDDEN; extern bool wg_format_compare(const struct wg_format *a, const struct wg_format *b) DECLSPEC_HIDDEN; extern GstCaps *wg_format_to_caps(const struct wg_format *format) DECLSPEC_HIDDEN;
+/* wg_transform.c */ + extern NTSTATUS wg_transform_create(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_destroy(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_set_output_format(void *args) DECLSPEC_HIDDEN; @@ -39,6 +51,8 @@ extern NTSTATUS wg_transform_push_data(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_read_data(void *args) DECLSPEC_HIDDEN; extern NTSTATUS wg_transform_get_status(void *args) DECLSPEC_HIDDEN;
+/* wg_allocator.c */ + /* wg_allocator_release_sample can be used to release any sample that was requested. */ typedef struct wg_sample *(*wg_allocator_request_sample_cb)(gsize size, void *context); extern GstAllocator *wg_allocator_create(wg_allocator_request_sample_cb request_sample, diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c new file mode 100644 index 00000000000..1570f0a2a2b --- /dev/null +++ b/dlls/winegstreamer/unixlib.c @@ -0,0 +1,76 @@ +/* + * winegstreamer Unix library interface + * + * Copyright 2020-2021 Zebediah Figura for CodeWeavers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#if 0 +#pragma makedep unix +#endif + +#include "config.h" + +#include <assert.h> +#include <stdarg.h> +#include <stdio.h> + +#include <gst/gst.h> +#include <gst/video/video.h> +#include <gst/audio/audio.h> +#include <gst/tag/tag.h> + +#include "ntstatus.h" +#define WIN32_NO_STATUS +#include "winternl.h" +#include "dshow.h" + +#include "unix_private.h" + +/* GStreamer callbacks may be called on threads not created by Wine, and + * therefore cannot access the Wine TEB. This means that we must use GStreamer + * debug logging instead of Wine debug logging. In order to be safe we forbid + * any use of Wine debug logging in this entire file. */ + +GST_DEBUG_CATEGORY(wine); + +static void init_gstreamer_once(void) +{ + char arg0[] = "wine"; + char arg1[] = "--gst-disable-registry-fork"; + char *args[] = {arg0, arg1, NULL}; + int argc = ARRAY_SIZE(args) - 1; + char **argv = args; + GError *err; + + if (!gst_init_check(&argc, &argv, &err)) + { + fprintf(stderr, "winegstreamer: failed to initialize GStreamer: %s\n", err->message); + g_error_free(err); + return; + } + + GST_DEBUG_CATEGORY_INIT(wine, "WINE", GST_DEBUG_FG_RED, "Wine GStreamer support"); + + GST_INFO("GStreamer library version %s; wine built with %d.%d.%d.", + gst_version_string(), GST_VERSION_MAJOR, GST_VERSION_MINOR, GST_VERSION_MICRO); +} + +NTSTATUS wg_process_attach(void *args) +{ + static pthread_once_t init_once = PTHREAD_ONCE_INIT; + return pthread_once(&init_once, init_gstreamer_once) ? STATUS_UNSUCCESSFUL : STATUS_SUCCESS; +} diff --git a/dlls/winegstreamer/unixlib.h b/dlls/winegstreamer/unixlib.h index d7bc7c65f0d..ebd8a01b3a1 100644 --- a/dlls/winegstreamer/unixlib.h +++ b/dlls/winegstreamer/unixlib.h @@ -335,6 +335,8 @@ struct wg_transform_get_status_params
enum unix_funcs { + unix_wg_process_attach, + unix_wg_parser_create, unix_wg_parser_destroy,
diff --git a/dlls/winegstreamer/wg_allocator.c b/dlls/winegstreamer/wg_allocator.c index 784677e2b2d..14550ad8bcc 100644 --- a/dlls/winegstreamer/wg_allocator.c +++ b/dlls/winegstreamer/wg_allocator.c @@ -35,9 +35,6 @@
#include "wine/list.h"
-GST_DEBUG_CATEGORY_EXTERN(wine); -#define GST_CAT_DEFAULT wine - typedef struct { GstMemory parent; diff --git a/dlls/winegstreamer/wg_format.c b/dlls/winegstreamer/wg_format.c index ac21b0af94f..6317a69edee 100644 --- a/dlls/winegstreamer/wg_format.c +++ b/dlls/winegstreamer/wg_format.c @@ -40,9 +40,6 @@
#include "unix_private.h"
-GST_DEBUG_CATEGORY_EXTERN(wine); -#define GST_CAT_DEFAULT wine - static enum wg_audio_format wg_audio_format_from_gst(GstAudioFormat format) { switch (format) diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index a8da149e7be..3406d022139 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -49,14 +49,6 @@ typedef enum GST_AUTOPLUG_SELECT_SKIP, } GstAutoplugSelectResult;
-/* GStreamer callbacks may be called on threads not created by Wine, and - * therefore cannot access the Wine TEB. This means that we must use GStreamer - * debug logging instead of Wine debug logging. In order to be safe we forbid - * any use of Wine debug logging in this entire file. */ - -GST_DEBUG_CATEGORY(wine); -#define GST_CAT_DEFAULT wine - typedef BOOL (*init_gst_cb)(struct wg_parser *parser);
struct wg_parser @@ -1680,35 +1672,6 @@ static BOOL wave_parser_init_gst(struct wg_parser *parser) return TRUE; }
-static void init_gstreamer_once(void) -{ - char arg0[] = "wine"; - char arg1[] = "--gst-disable-registry-fork"; - char *args[] = {arg0, arg1, NULL}; - int argc = ARRAY_SIZE(args) - 1; - char **argv = args; - GError *err; - - if (!gst_init_check(&argc, &argv, &err)) - { - fprintf(stderr, "winegstreamer: failed to initialize GStreamer: %s\n", err->message); - g_error_free(err); - return; - } - - GST_DEBUG_CATEGORY_INIT(wine, "WINE", GST_DEBUG_FG_RED, "Wine GStreamer support"); - - GST_INFO("GStreamer library version %s; wine built with %d.%d.%d.", - gst_version_string(), GST_VERSION_MAJOR, GST_VERSION_MINOR, GST_VERSION_MICRO); -} - -bool init_gstreamer(void) -{ - static pthread_once_t init_once = PTHREAD_ONCE_INIT; - - return !pthread_once(&init_once, init_gstreamer_once); -} - static NTSTATUS wg_parser_create(void *args) { static const init_gst_cb init_funcs[] = @@ -1722,9 +1685,6 @@ static NTSTATUS wg_parser_create(void *args) struct wg_parser_create_params *params = args; struct wg_parser *parser;
- if (!init_gstreamer()) - return E_FAIL; - if (!(parser = calloc(1, sizeof(*parser)))) return E_OUTOFMEMORY;
@@ -1763,6 +1723,8 @@ static NTSTATUS wg_parser_destroy(void *args) const unixlib_entry_t __wine_unix_call_funcs[] = { #define X(name) [unix_ ## name] = name + X(wg_process_attach), + X(wg_parser_create), X(wg_parser_destroy),
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index ccdd90361fc..ca89c883f62 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -39,9 +39,6 @@
#include "unix_private.h"
-GST_DEBUG_CATEGORY_EXTERN(wine); -#define GST_CAT_DEFAULT wine - #define GST_SAMPLE_FLAG_WG_CAPS_CHANGED (GST_MINI_OBJECT_FLAG_LAST << 0)
struct wg_transform @@ -360,9 +357,6 @@ NTSTATUS wg_transform_create(void *args) const gchar *media_type; GstEvent *event;
- if (!init_gstreamer()) - return STATUS_UNSUCCESSFUL; - if (!(transform = calloc(1, sizeof(*transform)))) return STATUS_NO_MEMORY; if (!(transform->container = gst_bin_new("wg_transform")))
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/unix_private.h | 3 +- dlls/winegstreamer/unixlib.c | 54 +++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 10 ------ dlls/winegstreamer/wg_transform.c | 46 +------------------------- 4 files changed, 56 insertions(+), 57 deletions(-)
diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 0838a4dec6e..40ddf1f104f 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -32,9 +32,8 @@ GST_DEBUG_CATEGORY_EXTERN(wine) DECLSPEC_HIDDEN;
extern NTSTATUS wg_process_attach(void *args) DECLSPEC_HIDDEN;
-/* wg_parser.c */ - extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN; +extern GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) DECLSPEC_HIDDEN;
/* wg_format.c */
diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 1570f0a2a2b..3307c2d6652 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -47,6 +47,60 @@
GST_DEBUG_CATEGORY(wine);
+GstElement *create_element(const char *name, const char *plugin_set) +{ + GstElement *element; + + if (!(element = gst_element_factory_make(name, NULL))) + fprintf(stderr, "winegstreamer: failed to create %s, are %u-bit GStreamer "%s" plugins installed?\n", + name, 8 * (unsigned int)sizeof(void *), plugin_set); + return element; +} + +GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) +{ + GstElement *element = NULL; + GList *tmp, *transforms; + const gchar *name; + + if (!(transforms = gst_element_factory_list_get_elements(type, GST_RANK_MARGINAL))) + goto done; + + tmp = gst_element_factory_list_filter(transforms, src_caps, GST_PAD_SINK, FALSE); + gst_plugin_feature_list_free(transforms); + if (!(transforms = tmp)) + goto done; + + tmp = gst_element_factory_list_filter(transforms, sink_caps, GST_PAD_SRC, FALSE); + gst_plugin_feature_list_free(transforms); + if (!(transforms = tmp)) + goto done; + + transforms = g_list_sort(transforms, gst_plugin_feature_rank_compare_func); + for (tmp = transforms; tmp != NULL && element == NULL; tmp = tmp->next) + { + name = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(tmp->data)); + if (!(element = gst_element_factory_create(GST_ELEMENT_FACTORY(tmp->data), NULL))) + GST_WARNING("Failed to create %s element.", name); + } + gst_plugin_feature_list_free(transforms); + +done: + if (element) + { + GST_DEBUG("Created %s element %p.", name, element); + } + else + { + gchar *src_str = gst_caps_to_string(src_caps), *sink_str = gst_caps_to_string(sink_caps); + GST_WARNING("Failed to create element matching caps %s / %s.", src_str, sink_str); + g_free(sink_str); + g_free(src_str); + } + + return element; +} + static void init_gstreamer_once(void) { char arg0[] = "wine"; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 3406d022139..1f6aed4713f 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -727,16 +727,6 @@ static gboolean sink_query_cb(GstPad *pad, GstObject *parent, GstQuery *query) } }
-GstElement *create_element(const char *name, const char *plugin_set) -{ - GstElement *element; - - if (!(element = gst_element_factory_make(name, NULL))) - fprintf(stderr, "winegstreamer: failed to create %s, are %u-bit GStreamer "%s" plugins installed?\n", - name, 8 * (unsigned int)sizeof(void *), plugin_set); - return element; -} - static struct wg_parser_stream *create_stream(struct wg_parser *parser) { struct wg_parser_stream *stream, **new_array; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index ca89c883f62..cfc04f2093e 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -267,50 +267,6 @@ NTSTATUS wg_transform_destroy(void *args) return STATUS_SUCCESS; }
-static GstElement *transform_find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) -{ - GstElement *element = NULL; - GList *tmp, *transforms; - const gchar *name; - - if (!(transforms = gst_element_factory_list_get_elements(type, GST_RANK_MARGINAL))) - goto done; - - tmp = gst_element_factory_list_filter(transforms, src_caps, GST_PAD_SINK, FALSE); - gst_plugin_feature_list_free(transforms); - if (!(transforms = tmp)) - goto done; - - tmp = gst_element_factory_list_filter(transforms, sink_caps, GST_PAD_SRC, FALSE); - gst_plugin_feature_list_free(transforms); - if (!(transforms = tmp)) - goto done; - - transforms = g_list_sort(transforms, gst_plugin_feature_rank_compare_func); - for (tmp = transforms; tmp != NULL && element == NULL; tmp = tmp->next) - { - name = gst_plugin_feature_get_name(GST_PLUGIN_FEATURE(tmp->data)); - if (!(element = gst_element_factory_create(GST_ELEMENT_FACTORY(tmp->data), NULL))) - GST_WARNING("Failed to create %s element.", name); - } - gst_plugin_feature_list_free(transforms); - -done: - if (element) - { - GST_DEBUG("Created %s element %p.", name, element); - } - else - { - gchar *src_str = gst_caps_to_string(src_caps), *sink_str = gst_caps_to_string(sink_caps); - GST_WARNING("Failed to create transform matching caps %s / %s.", src_str, sink_str); - g_free(sink_str); - g_free(src_str); - } - - return element; -} - static bool transform_append_element(struct wg_transform *transform, GstElement *element, GstElement **first, GstElement **last) { @@ -423,7 +379,7 @@ NTSTATUS wg_transform_create(void *args) case WG_MAJOR_TYPE_VIDEO_CINEPAK: case WG_MAJOR_TYPE_VIDEO_INDEO: case WG_MAJOR_TYPE_VIDEO_WMV: - if (!(element = transform_find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) + if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) || !transform_append_element(transform, element, &first, &last)) { gst_caps_unref(raw_caps);
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/wg_transform.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index cfc04f2093e..7977c818b42 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -267,13 +267,13 @@ NTSTATUS wg_transform_destroy(void *args) return STATUS_SUCCESS; }
-static bool transform_append_element(struct wg_transform *transform, GstElement *element, - GstElement **first, GstElement **last) +static bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) { gchar *name = gst_element_get_name(element); bool success = false;
- if (!gst_bin_add(GST_BIN(transform->container), element) || + if (!gst_bin_add(GST_BIN(container), element) || + !gst_element_sync_state_with_parent(element) || (*last && !gst_element_link(*last, element))) { GST_ERROR("Failed to link %s element.", name); @@ -370,7 +370,7 @@ NTSTATUS wg_transform_create(void *args) transform->input_max_length = 16; transform->output_plane_align = 15; if (!(element = create_element("h264parse", "base")) - || !transform_append_element(transform, element, &first, &last)) + || !append_element(transform->container, element, &first, &last)) goto out; /* fallthrough */ case WG_MAJOR_TYPE_AUDIO_MPEG1: @@ -380,7 +380,7 @@ NTSTATUS wg_transform_create(void *args) case WG_MAJOR_TYPE_VIDEO_INDEO: case WG_MAJOR_TYPE_VIDEO_WMV: if (!(element = find_element(GST_ELEMENT_FACTORY_TYPE_DECODER, src_caps, raw_caps)) - || !transform_append_element(transform, element, &first, &last)) + || !append_element(transform->container, element, &first, &last)) { gst_caps_unref(raw_caps); goto out; @@ -411,17 +411,17 @@ NTSTATUS wg_transform_create(void *args) * non-interleaved format. */ if (!(element = create_element("audioconvert", "base")) - || !transform_append_element(transform, element, &first, &last)) + || !append_element(transform->container, element, &first, &last)) goto out; if (!(element = create_element("audioresample", "base")) - || !transform_append_element(transform, element, &first, &last)) + || !append_element(transform->container, element, &first, &last)) goto out; break;
case WG_MAJOR_TYPE_VIDEO: case WG_MAJOR_TYPE_VIDEO_WMV: if (!(element = create_element("videoconvert", "base")) - || !transform_append_element(transform, element, &first, &last)) + || !append_element(transform->container, element, &first, &last)) goto out; /* Let GStreamer choose a default number of threads. */ gst_util_set_object_arg(G_OBJECT(element), "n-threads", "0");
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.c | 24 ++++++++++++++++ dlls/winegstreamer/wg_parser.c | 47 +++++++++++-------------------- dlls/winegstreamer/wg_transform.c | 24 ---------------- 4 files changed, 41 insertions(+), 55 deletions(-)
diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 40ddf1f104f..57cc500a36c 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -34,6 +34,7 @@ extern NTSTATUS wg_process_attach(void *args) DECLSPEC_HIDDEN;
extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN; extern GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) DECLSPEC_HIDDEN; +extern bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) DECLSPEC_HIDDEN;
/* wg_format.c */
diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 3307c2d6652..09a5f901f8d 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -101,6 +101,30 @@ done: return element; }
+bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) +{ + gchar *name = gst_element_get_name(element); + bool success = false; + + if (!gst_bin_add(GST_BIN(container), element) || + !gst_element_sync_state_with_parent(element) || + (*last && !gst_element_link(*last, element))) + { + GST_ERROR("Failed to link %s element.", name); + } + else + { + GST_DEBUG("Linked %s element %p.", name, element); + if (!*first) + *first = element; + *last = element; + success = true; + } + + g_free(name); + return success; +} + static void init_gstreamer_once(void) { char arg0[] = "wine"; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 1f6aed4713f..479ac8ca713 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -787,6 +787,7 @@ static void free_stream(struct wg_parser_stream *stream)
static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) { + GstElement *first = NULL, *last = NULL; struct wg_parser *parser = user; struct wg_parser_stream *stream; const char *name; @@ -806,62 +807,46 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user)
if (!strcmp(name, "video/x-raw")) { - GstElement *deinterlace, *vconv, *flip, *vconv2; - /* DirectShow can express interlaced video, but downstream filters can't * necessarily consume it. In particular, the video renderer can't. */ - if (!(deinterlace = create_element("deinterlace", "good"))) + if (!(element = create_element("deinterlace", "good")) + || !append_element(parser->container, element, &first, &last)) goto out;
/* decodebin considers many YUV formats to be "raw", but some quartz * filters can't handle those. Also, videoflip can't handle all "raw" * formats either. Add a videoconvert to swap color spaces. */ - if (!(vconv = create_element("videoconvert", "base"))) + if (!(element = create_element("videoconvert", "base")) + || !append_element(parser->container, element, &first, &last)) goto out;
/* GStreamer outputs RGB video top-down, but DirectShow expects bottom-up. */ - if (!(flip = create_element("videoflip", "good"))) + if (!(element = create_element("videoflip", "good")) + || !append_element(parser->container, element, &first, &last)) goto out; + stream->flip = element;
/* videoflip does not support 15 and 16-bit RGB so add a second videoconvert * to do the final conversion. */ - if (!(vconv2 = create_element("videoconvert", "base"))) + if (!(element = create_element("videoconvert", "base")) + || !append_element(parser->container, element, &first, &last)) goto out;
- /* The bin takes ownership of these elements. */ - gst_bin_add(GST_BIN(parser->container), deinterlace); - gst_element_sync_state_with_parent(deinterlace); - gst_bin_add(GST_BIN(parser->container), vconv); - gst_element_sync_state_with_parent(vconv); - gst_bin_add(GST_BIN(parser->container), flip); - gst_element_sync_state_with_parent(flip); - gst_bin_add(GST_BIN(parser->container), vconv2); - gst_element_sync_state_with_parent(vconv2); - - gst_element_link(deinterlace, vconv); - gst_element_link(vconv, flip); - gst_element_link(flip, vconv2); - - stream->post_sink = gst_element_get_static_pad(deinterlace, "sink"); - stream->post_src = gst_element_get_static_pad(vconv2, "src"); - stream->flip = flip; + stream->post_sink = gst_element_get_static_pad(first, "sink"); + stream->post_src = gst_element_get_static_pad(last, "src"); } else if (!strcmp(name, "audio/x-raw")) { - GstElement *convert; - /* Currently our dsound can't handle 64-bit formats or all * surround-sound configurations. Native dsound can't always handle * 64-bit formats either. Add an audioconvert to allow changing bit * depth and channel count. */ - if (!(convert = create_element("audioconvert", "base"))) + if (!(element = create_element("audioconvert", "base")) + || !append_element(parser->container, element, &first, &last)) goto out;
- gst_bin_add(GST_BIN(parser->container), convert); - gst_element_sync_state_with_parent(convert); - - stream->post_sink = gst_element_get_static_pad(convert, "sink"); - stream->post_src = gst_element_get_static_pad(convert, "src"); + stream->post_sink = gst_element_get_static_pad(first, "sink"); + stream->post_src = gst_element_get_static_pad(last, "src"); }
if (stream->post_sink) diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 7977c818b42..30f191c4132 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -267,30 +267,6 @@ NTSTATUS wg_transform_destroy(void *args) return STATUS_SUCCESS; }
-static bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) -{ - gchar *name = gst_element_get_name(element); - bool success = false; - - if (!gst_bin_add(GST_BIN(container), element) || - !gst_element_sync_state_with_parent(element) || - (*last && !gst_element_link(*last, element))) - { - GST_ERROR("Failed to link %s element.", name); - } - else - { - GST_DEBUG("Linked %s element %p.", name, element); - if (!*first) - *first = element; - *last = element; - success = true; - } - - g_free(name); - return success; -} - static struct wg_sample *transform_request_sample(gsize size, void *context) { struct wg_transform *transform = context;
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/wg_parser.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 479ac8ca713..24fee5acaa3 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -898,10 +898,6 @@ static void pad_removed_cb(GstElement *element, GstPad *pad, gpointer user)
if (stream->their_src == pad) { - if (stream->post_sink) - gst_pad_unlink(stream->their_src, stream->post_sink); - else - gst_pad_unlink(stream->their_src, stream->my_sink); gst_object_unref(stream->their_src); stream->their_src = NULL; return;
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/unix_private.h | 2 ++ dlls/winegstreamer/unixlib.c | 46 +++++++++++++++++++++++++++++++ dlls/winegstreamer/wg_parser.c | 39 +++----------------------- dlls/winegstreamer/wg_transform.c | 15 ++-------- 4 files changed, 54 insertions(+), 48 deletions(-)
diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 57cc500a36c..6bfab1165a2 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -35,6 +35,8 @@ extern NTSTATUS wg_process_attach(void *args) DECLSPEC_HIDDEN; extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN; extern GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) DECLSPEC_HIDDEN; extern bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) DECLSPEC_HIDDEN; +extern bool link_src_to_element(GstPad *src_pad, GstElement *element) DECLSPEC_HIDDEN; +extern bool link_element_to_sink(GstElement *element, GstPad *sink_pad) DECLSPEC_HIDDEN;
/* wg_format.c */
diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index 09a5f901f8d..ee833ecc979 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -125,6 +125,52 @@ bool append_element(GstElement *container, GstElement *element, GstElement **fir return success; }
+bool link_src_to_element(GstPad *src_pad, GstElement *element) +{ + GstPadLinkReturn ret; + GstPad *sink_pad; + + if (!(sink_pad = gst_element_get_static_pad(element, "sink"))) + { + gchar *name = gst_element_get_name(element); + GST_ERROR("Failed to find sink pad on %s", name); + g_free(name); + return false; + } + if ((ret = gst_pad_link(src_pad, sink_pad))) + { + gchar *src_name = gst_pad_get_name(src_pad), *sink_name = gst_pad_get_name(sink_pad); + GST_ERROR("Failed to link element pad %s with pad %s", src_name, sink_name); + g_free(sink_name); + g_free(src_name); + } + gst_object_unref(sink_pad); + return !ret; +} + +bool link_element_to_sink(GstElement *element, GstPad *sink_pad) +{ + GstPadLinkReturn ret; + GstPad *src_pad; + + if (!(src_pad = gst_element_get_static_pad(element, "src"))) + { + gchar *name = gst_element_get_name(element); + GST_ERROR("Failed to find src pad on %s", name); + g_free(name); + return false; + } + if ((ret = gst_pad_link(src_pad, sink_pad))) + { + gchar *src_name = gst_pad_get_name(src_pad), *sink_name = gst_pad_get_name(sink_pad); + GST_ERROR("Failed to link pad %s with element pad %s", src_name, sink_name); + g_free(sink_name); + g_free(src_name); + } + gst_object_unref(src_pad); + return !ret; +} + static void init_gstreamer_once(void) { char arg0[] = "wine"; diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c index 24fee5acaa3..593087c5017 100644 --- a/dlls/winegstreamer/wg_parser.c +++ b/dlls/winegstreamer/wg_parser.c @@ -95,7 +95,7 @@ struct wg_parser_stream struct wg_parser *parser; uint32_t number;
- GstPad *their_src, *post_sink, *post_src, *my_sink; + GstPad *their_src, *my_sink; GstElement *flip; GstSegment segment; struct wg_format preferred_format, current_format; @@ -763,15 +763,7 @@ static void free_stream(struct wg_parser_stream *stream) unsigned int i;
if (stream->their_src) - { - if (stream->post_sink) - { - gst_object_unref(stream->post_src); - gst_object_unref(stream->post_sink); - stream->post_src = stream->post_sink = NULL; - } gst_object_unref(stream->their_src); - } gst_object_unref(stream->my_sink);
pthread_cond_destroy(&stream->event_cond); @@ -832,8 +824,8 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) || !append_element(parser->container, element, &first, &last)) goto out;
- stream->post_sink = gst_element_get_static_pad(first, "sink"); - stream->post_src = gst_element_get_static_pad(last, "src"); + if (!link_src_to_element(pad, first) || !link_element_to_sink(last, stream->my_sink)) + goto out; } else if (!strcmp(name, "audio/x-raw")) { @@ -845,31 +837,8 @@ static void pad_added_cb(GstElement *element, GstPad *pad, gpointer user) || !append_element(parser->container, element, &first, &last)) goto out;
- stream->post_sink = gst_element_get_static_pad(first, "sink"); - stream->post_src = gst_element_get_static_pad(last, "src"); - } - - if (stream->post_sink) - { - if ((ret = gst_pad_link(pad, stream->post_sink)) < 0) - { - GST_ERROR("Failed to link decodebin source pad to post-processing elements, error %s.", - gst_pad_link_get_name(ret)); - gst_object_unref(stream->post_sink); - stream->post_sink = NULL; - goto out; - } - - if ((ret = gst_pad_link(stream->post_src, stream->my_sink)) < 0) - { - GST_ERROR("Failed to link post-processing elements to our sink pad, error %s.", - gst_pad_link_get_name(ret)); - gst_object_unref(stream->post_src); - stream->post_src = NULL; - gst_object_unref(stream->post_sink); - stream->post_sink = NULL; + if (!link_src_to_element(pad, first) || !link_element_to_sink(last, stream->my_sink)) goto out; - } } else if ((ret = gst_pad_link(pad, stream->my_sink)) < 0) { diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 30f191c4132..978d790fbb6 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -46,7 +46,6 @@ struct wg_transform GstElement *container; GstAllocator *allocator; GstPad *my_src, *my_sink; - GstPad *their_sink, *their_src; GstSegment segment; GstQuery *drain_query;
@@ -254,8 +253,6 @@ NTSTATUS wg_transform_destroy(void *args) gst_sample_unref(sample);
wg_allocator_destroy(transform->allocator); - g_object_unref(transform->their_sink); - g_object_unref(transform->their_src); g_object_unref(transform->container); g_object_unref(transform->my_sink); g_object_unref(transform->my_src); @@ -414,13 +411,9 @@ NTSTATUS wg_transform_create(void *args) goto out; }
- if (!(transform->their_sink = gst_element_get_static_pad(first, "sink"))) + if (!link_src_to_element(transform->my_src, first)) goto out; - if (!(transform->their_src = gst_element_get_static_pad(last, "src"))) - goto out; - if (gst_pad_link(transform->my_src, transform->their_sink) < 0) - goto out; - if (gst_pad_link(transform->their_src, transform->my_sink) < 0) + if (!link_element_to_sink(last, transform->my_sink)) goto out; if (!gst_pad_set_active(transform->my_sink, 1)) goto out; @@ -454,10 +447,6 @@ NTSTATUS wg_transform_create(void *args) return STATUS_SUCCESS;
out: - if (transform->their_sink) - gst_object_unref(transform->their_sink); - if (transform->their_src) - gst_object_unref(transform->their_src); if (transform->my_sink) gst_object_unref(transform->my_sink); if (transform->output_caps)
From: Rémi Bernon rbernon@codeweavers.com
--- dlls/winegstreamer/unix_private.h | 1 + dlls/winegstreamer/unixlib.c | 21 +++++++++++++++++++++ dlls/winegstreamer/wg_transform.c | 17 +++-------------- 3 files changed, 25 insertions(+), 14 deletions(-)
diff --git a/dlls/winegstreamer/unix_private.h b/dlls/winegstreamer/unix_private.h index 6bfab1165a2..a7b70a06cd0 100644 --- a/dlls/winegstreamer/unix_private.h +++ b/dlls/winegstreamer/unix_private.h @@ -32,6 +32,7 @@ GST_DEBUG_CATEGORY_EXTERN(wine) DECLSPEC_HIDDEN;
extern NTSTATUS wg_process_attach(void *args) DECLSPEC_HIDDEN;
+extern GstStreamType stream_type_from_caps(GstCaps *caps) DECLSPEC_HIDDEN; extern GstElement *create_element(const char *name, const char *plugin_set) DECLSPEC_HIDDEN; extern GstElement *find_element(GstElementFactoryListType type, GstCaps *src_caps, GstCaps *sink_caps) DECLSPEC_HIDDEN; extern bool append_element(GstElement *container, GstElement *element, GstElement **first, GstElement **last) DECLSPEC_HIDDEN; diff --git a/dlls/winegstreamer/unixlib.c b/dlls/winegstreamer/unixlib.c index ee833ecc979..aa8bcc03745 100644 --- a/dlls/winegstreamer/unixlib.c +++ b/dlls/winegstreamer/unixlib.c @@ -47,6 +47,27 @@
GST_DEBUG_CATEGORY(wine);
+GstStreamType stream_type_from_caps(GstCaps *caps) +{ + const gchar *media_type; + + if (!caps || !gst_caps_get_size(caps)) + return GST_STREAM_TYPE_UNKNOWN; + + media_type = gst_structure_get_name(gst_caps_get_structure(caps, 0)); + if (g_str_has_prefix(media_type, "video/") + || g_str_has_prefix(media_type, "image/")) + return GST_STREAM_TYPE_VIDEO; + if (g_str_has_prefix(media_type, "audio/")) + return GST_STREAM_TYPE_AUDIO; + if (g_str_has_prefix(media_type, "text/") + || g_str_has_prefix(media_type, "subpicture/") + || g_str_has_prefix(media_type, "closedcaption/")) + return GST_STREAM_TYPE_TEXT; + + return GST_STREAM_TYPE_UNKNOWN; +} + GstElement *create_element(const char *name, const char *plugin_set) { GstElement *element; diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 978d790fbb6..7755dac2523 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -60,17 +60,6 @@ struct wg_transform GstCaps *output_caps; };
-static bool is_caps_video(GstCaps *caps) -{ - const gchar *media_type; - - if (!caps || !gst_caps_get_size(caps)) - return false; - - media_type = gst_structure_get_name(gst_caps_get_structure(caps, 0)); - return g_str_has_prefix(media_type, "video/"); -} - static void align_video_info_planes(gsize plane_align, GstVideoInfo *info, GstVideoAlignment *align) { gst_video_alignment_reset(align); @@ -127,7 +116,7 @@ static gboolean transform_sink_query_cb(GstPad *pad, GstObject *parent, GstQuery GstCaps *caps;
gst_query_parse_allocation(query, &caps, &needs_pool); - if (!is_caps_video(caps) || !needs_pool) + if (stream_type_from_caps(caps) != GST_STREAM_TYPE_VIDEO || !needs_pool) break;
if (!gst_video_info_from_caps(&info, caps) @@ -671,7 +660,7 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsi
if ((ret = !needs_copy)) total_size = sample->size = info.size; - else if (is_caps_video(caps)) + else if (stream_type_from_caps(caps) == GST_STREAM_TYPE_VIDEO) ret = copy_video_buffer(buffer, caps, plane_align, sample, &total_size); else ret = copy_buffer(buffer, caps, sample, &total_size); @@ -707,7 +696,7 @@ static NTSTATUS read_transform_output_data(GstBuffer *buffer, GstCaps *caps, gsi
if (needs_copy) { - if (is_caps_video(caps)) + if (stream_type_from_caps(caps) == GST_STREAM_TYPE_VIDEO) GST_WARNING("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags); else GST_INFO("Copied %u bytes, sample %p, flags %#x", sample->size, sample, sample->flags);
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=131823
Your paranoid android.
=== debian11 (32 bit report) ===
qasf: asfreader.c:487: Test failed: Got hr 0x1. Unhandled exception: page fault on execute access to 0x00000000 in 32-bit code (0x00000000).
wmvcore: wmvcore.c:3766: Test failed: Got hr 0x80004005. wmvcore.c:3767: Test failed: Got refcount 1. wmvcore.c:3786: Test failed: Got hr 0x80070057. wmvcore.c:3787: Test failed: Got allocator DEADBEEF. wmvcore.c:3790: Test failed: Got hr 0x80070057. wmvcore.c:3791: Test failed: Got allocator DEADBEEF. wmvcore.c:3795: Test failed: Got hr 0x80070057. wmvcore.c:3798: Test failed: Got hr 0x80070057. wmvcore.c:3799: Test failed: Got allocator DEADBEEF. wmvcore.c:3802: Test failed: Got hr 0x80070057. wmvcore.c:3803: Test failed: Got allocator DEADBEEF. wmvcore.c:3805: Test failed: Got hr 0x80070057. wmvcore.c:3806: Test failed: Got allocator DEADBEEF. wmvcore.c:3809: Test failed: Got hr 0x80070057. wmvcore.c:3812: Test failed: Got hr 0x80070057. wmvcore.c:3813: Test failed: Got allocator DEADBEEF. wmvcore.c:3816: Test failed: Got hr 0x80070057. wmvcore.c:3817: Test failed: Got allocator DEADBEEF. wmvcore.c:3821: Test failed: Got hr 0x80070057. wmvcore.c:3824: Test failed: Got hr 0x80070057. wmvcore.c:3825: Test failed: Got allocator DEADBEEF. wmvcore.c:3828: Test failed: Got hr 0x80070057. wmvcore.c:3829: Test failed: Got allocator DEADBEEF. wmvcore.c:3831: Test failed: Got hr 0x80070057. wmvcore.c:3832: Test failed: Got allocator DEADBEEF. wmvcore.c:3835: Test failed: Got hr 0x80070057. wmvcore.c:3838: Test failed: Got hr 0x80070057. wmvcore.c:3839: Test failed: Got allocator DEADBEEF. wmvcore.c:3842: Test failed: Got hr 0x80070057. wmvcore.c:3843: Test failed: Got allocator DEADBEEF. wmvcore.c:3849: Test failed: Got hr 0x80070057. wmvcore.c:3851: Test failed: Got hr 0x80070057. wmvcore.c:3853: Test failed: Got hr 0x80070057. wmvcore.c:3862: Test failed: Got hr 0x80070057. Unhandled exception: page fault on read access to 0x000003dc in 32-bit code (0x0040f8ba).
I'd have moved the unixlib call table too, but that would cause conflicts with out-of-tree patches which add some new entry points. I'm fine with moving it nonetheless, or it could be done later.
Patch 1/7 has the unfortunate effect that now we're loading GStreamer even when winegstreamer is just being registered. I assume this is for working around a race condition in Unreal Engine, as was described to me out of channel. I'm not thrilled about doing things like this just to work around application race conditions.
(And if we are going to do that kind of a workaround, it needs to be justified in the code, lest someone later try to simplify the code by reverting it.)
I'd have moved the unixlib call table too, but that would cause conflicts with out-of-tree patches which add some new entry points. I'm fine with moving it nonetheless, or it could be done later.
If we're going to have a unixlib.c we should probably have the call table there too, yes, but I don't care strongly about doing that immediately.
Patch 1/7 has the unfortunate effect that now we're loading GStreamer even when winegstreamer is just being registered. I assume this is for working around a race condition in Unreal Engine, as was described to me out of channel. I'm not thrilled about doing things like this just to work around application race conditions.
Actually, moreover, I'm surprised that this even helps? When is winegstreamer loaded, that adding a delay to DLL load helps but adding it to parser object creation doesn't?
This is not directly related to the race condition, and the move of gstreamer init is only something that felt right to do in the PE `init_gstreamer`, and an excuse to introduce the source where common helpers would live.
Patch 1/7 has the unfortunate effect that now we're loading GStreamer even when winegstreamer is just being registered. I assume this is for working around a race condition in Unreal Engine, as was described to me out of channel. I'm not thrilled about doing things like this just to work around application race conditions.
How is that? it's called only from `DllGetClassObject` which is when objects are instantiated, not registered no?
Patch 1/7 has the unfortunate effect that now we're loading GStreamer even when winegstreamer is just being registered. I assume this is for working around a race condition in Unreal Engine, as was described to me out of channel. I'm not thrilled about doing things like this just to work around application race conditions.
How is that? it's called only from `DllGetClassObject` which is when objects are instantiated, not registered no?
No, you're right, I'm blind, sorry. [Though, in my defense, process_attach is a slightly misleading name if it's not called from DllMain().]
I'm not sure I see the benefit of adding a separate unix call then, though. If we want to move the gst_init_check() call to a more neutral file I think that's fine, but that can be done without adding a new entry point?
No, you're right, I'm blind, sorry. [Though, in my defense, process_attach is a slightly misleading name if it's not called from DllMain().]
I'm not sure I see the benefit of adding a separate unix call then, though. If we want to move the gst_init_check() call to a more neutral file I think that's fine, but that can be done without adding a new entry point?
Well, it'd save the need for the pthread_once, as it's already guarded on the PE side.
I figure now that I've missed a call from parser_create, but I think this one could be dropped as it comes from `DllGetClassObject`, and instead probably one should added in `winegstreamer_create_wm_sync_reader` instead.
On the unix side, there would be no need for guards or calls to `init_gstreamer`.
On Wed Apr 12 19:24:00 2023 +0000, Rémi Bernon wrote:
This is not directly related to the race condition, and the move of gstreamer init is only something that felt right to do in the PE `init_gstreamer`, and an excuse to introduce the source where common helpers would live.
I see that the commit mentions startup slowdown. I think it's still kind of true as in the MF case, the class that is created is `GStreamerByteStreamHandler`, and the UE race condition only starts after `BeginCreateObject`, but I'll remove the comment nonetheless as the overhead is not great.
-
Marvin -
Rémi Bernon -
Rémi Bernon (＠rbernon)
-
Zebediah Figura (＠zfigura)