On 5/12/22 03:15, Rémi Bernon wrote:
From: Rémi Bernon rbernon@codeweavers.com
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45988 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=47084 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49715 Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=52183 Signed-off-by: Rémi Bernon rbernon@codeweavers.com
dlls/winegstreamer/wg_transform.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/dlls/winegstreamer/wg_transform.c b/dlls/winegstreamer/wg_transform.c index 21392a82509..dcc334caf7a 100644 --- a/dlls/winegstreamer/wg_transform.c +++ b/dlls/winegstreamer/wg_transform.c @@ -114,7 +114,18 @@ static GstElement *transform_find_element(GstElementFactoryListType type, GstCap 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)))
/* VA-API plugins are not supported at the moment, they do not support* buffer pool negociation and will consistently ignore our pool, as* it is not VA-API compatible, but also our plane alignment requirements* from video meta. They also are asynchronous by design, which isn't* going to let us implement zero-copy in the transforms reliably, and* vaapidecodebin includes a videoconvert element which does more than* we would like.*/
I'll admit, none of these sound like good reasons to ignore vaapi.
Regarding allocation: the source is not actually required to use an allocator proposed by the sink in an ALLOCATION query. The documentation isn't really clear about this, but I asked for a clarification from the GStreamer developers. vaapi's behaviour here is not a bug.
Regarding asynchronicity: It's not obvious from the comment why asynchronicity implies that zero-copy can't be done. Regardless, though, there's no API guarantee that filters are synchronous, including decoders. We shouldn't hardcode vaapi on those grounds.
Regarding videoconvert: why is this a problem?
To summarize, it's not clear from the comment why any of these things are a problem, and, even assuming they are, none of them inhere to vaapi specifically.
While zero-copy is desirable, I don't think it should be an absolute requirement if we can't easily guarantee it, and the details regarding ALLOCATION queries suggests that we indeed can't.
It may be that hardware plugins provide consistently worse performance than anything else, and should be disabled on those grounds. (Perhaps even for the above reasons—although I'd find it surprising that a CPU copy is the bottleneck and not the download itself—and in any case the comment doesn't make that clear). It's not immediately clear that hardware plugins are *always* going to provide worse performance, regardless of CPU setup, but it also wouldn't surprise me if so.
Ideally this should be a decision made on a lower level than Wine, but I also recognize that it's going to be hard to do so, since it depends on a lot of context, and I think it's fine to put code in Wine for those reasons. If we do, though:
* I think we should match elements based on GST_ELEMENT_FACTORY_KLASS_HARDWARE than string-matching the element name;
* ideally we should disprefer hardware decoders rather than disabling them completely.
if (!strncmp(name, "vaapi", 5))GST_FIXME("Ignoring unsupported %s plugin", name);else 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);