Frogging-Family / wine-tkg-git

The wine-tkg build systems, to create custom Wine and Proton builds
856 stars 155 forks source link

[valve-exp-bleeding] mfplat broken again, crashes every game I try #1072

Closed adolfintel closed 11 months ago

adolfintel commented 11 months ago

I'm using tkg with the valve-exp-bleeding profile and I noticed that since a couple of weeks every game I try that uses mfplat crashes as soon as it tries to play a video.

Older builds work so I suspect it is related to the mfplat patches that were committed to experimental_8.0 and bleeding-edge by @ivyl but I'm still trying to bisect the exact commit.

I'm using Arch Linux with kernel 6.5.4 and a 6900xt with Mesa 23.3 built from source.

gabriele2000 commented 11 months ago

I'm using tkg with the valve-exp-bleeding profile and I noticed that since a couple of weeks every game I try that uses mfplat crashes as soon as it tries to play a video.

Older builds work so I suspect it is related to the mfplat patches that were committed to experimental_8.0 and bleeding-edge by @ivyl but I'm still trying to bisect the exact commit.

I'm using Arch Linux with kernel 6.5.4 and a 6900xt with Mesa 23.3 built from source.

are you getting an assertion failed?

adolfintel commented 11 months ago

No, it just crashes with no error message. I'm almost done bisecting it, it's a pain in the ass when every attempt takes 15 minutes to build.

adolfintel commented 11 months ago

Bisected to commit 182dd174431c8cf669ae69d40dfb1819a214a8c2

gabriele2000 commented 11 months ago

No, it just crashes with no error message. I'm almost done bisecting it, it's a pain in the ass when every attempt takes 15 minutes to build.

I was asking because with WINE-GE-8.15 I'm having this weird problem with, for example, battlefield 4. If I load ReShade there's this "Assertion Failed" error, with a code that points to an error that happens if you try to execute 64-bit stuff with 32-bit libs or something like that.

Of course it's fake because everything's right and doesn't happen in version 7. Plus, it just crashes without errors, but inspecting the log makes me see the error

ivyl commented 11 months ago

@adolfintel huh, I wonder how it causes a crash for you. It works when using gst that's shipped with Proton.

Can you try running with this patch applied:

diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index a7784b923bb..ee4f4483d59 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -571,6 +571,7 @@ static void deep_element_added_cb(GstBin *self, GstBin *sub_bin, GstElement *ele
 {
     GstElementFactory *factory = gst_element_get_factory(element);
     const char *name = gst_element_factory_get_longname(factory);
+    GST_DEBUG("element %p, factory %p, name %p, name %s.", element, factory, name, name);

     if (strstr(name, "Dav1d"))
     {

And with GST_DEBUG_NO_COLOR=1 GST_DEBUG=4,WINE:9 ?

adolfintel commented 11 months ago

Thanks @ivyl, I'll try your patch and let you know.

The crash seems to be related to vaapi, if I remove gstreamer-vaapi from the system it works even after that commit. Weird since other apps can use vaapi, maybe it doesn't like that n-threads option?

ivyl commented 11 months ago

It should only add that option when dav1d is used - note the name check. It's a software AV1 decoder that we use via gst-plugins-rs. The patch is meant to limit the # of threads because it likes to just go for the number of cores and explode memory usage.

My guess is that there may be something weird going on with the vaapi elements and one of the values we depend on is NULL. Hopefully a NULL check will do.

adolfintel commented 11 months ago

@ivyl here's the log when the game crashes with vaapi:

0:00:03.136394547 4053012 0x7fcb68000b70 DEBUG                   WINE wg_parser.c:574:deep_element_added_cb: element 0x7fcb68042ba0, factory 0x7f133090, name 0x7f134ae0, name VA-API Decode Bin.
0:00:03.136399507 4053012 0x7fcb68000b70 DEBUG                   WINE wg_parser.c:574:deep_element_added_cb: element 0x7fcb680336c0, factory 0x7f20d1b0, name 0x7f20d2e0, name Queue.

(wine:4053012): GStreamer-CRITICAL **: 21:22:53.944: gst_element_factory_get_metadata: assertion 'GST_IS_ELEMENT_FACTORY (factory)' failed
0:00:03.136412907 4053012 0x7fcb68000b70 DEBUG                   WINE wg_parser.c:574:deep_element_added_cb: element 0x7fcb680441f0, factory (nil), name (nil), name (null).
ivyl commented 11 months ago

Weird, an element without a factory... Not sure how vaapi elements work.

diff --git a/dlls/winegstreamer/wg_parser.c b/dlls/winegstreamer/wg_parser.c
index a7784b923bb..3e5afe49bf0 100644
--- a/dlls/winegstreamer/wg_parser.c
+++ b/dlls/winegstreamer/wg_parser.c
@@ -569,10 +569,16 @@ static void no_more_pads_cb(GstElement *element, gpointer user)

 static void deep_element_added_cb(GstBin *self, GstBin *sub_bin, GstElement *element, gpointer user)
 {
-    GstElementFactory *factory = gst_element_get_factory(element);
-    const char *name = gst_element_factory_get_longname(factory);
+    GstElementFactory *factory = NULL;
+    const char *name = NULL;

-    if (strstr(name, "Dav1d"))
+    if (element)
+        factory = gst_element_get_factory(element);
+
+    if (factory)
+        name = gst_element_factory_get_longname(factory);
+
+    if (name && strstr(name, "Dav1d"))
     {
 #if defined(__x86_64__)
         GST_DEBUG("%s found, setting n-threads to 4.", name);

If this fixes it I'll merge it.

adolfintel commented 11 months ago

@ivyl Yes, that seems to work both with and without vaapi :)

ivyl commented 11 months ago

Merged as a fixup: https://github.com/ValveSoftware/wine/commit/6fcbc203c5a8cf04e6963f7c07ebf81378d0b9c0

it's already in bleeding-edge and will be a part of the next experimental release