Update nativelibs to latest version #6013
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Looking into the linux failures. |
|
I dunno. After some extremely crude attempts to try and force github runners to output The relevant portion being: tl;dr: Apparently the game is not even trying to load the new binaries. For whatever reason. It's just not seeing them, at all. I give up for now. @FreezyLemon If you have any brilliant ideas, they'd be appreciated right about now, because otherwise we just revert everything I guess? |
|
I guess telling the dynamic linker to forcibly use the new libraries, as in 1be8f82, appears to work (new-log.txt). But it doesn't answer why this used to do the right thing automatically before and now doesn't. |
|
Not sure how urgent this is, but I'll take a look later today. Might be related to prefix/libdir changes in the build script. |
|
It's not very urgent, so it can wait a few hours, but generally things have moved forward considerably what with the merging and deploying, and as such we're in a bit of a limbo. Probably don't want to remain in this half-broken state for too long. |
|
Since I can't repro this locally at all (not even in a Ubuntu VM), I tried getting some info from CI: Linux has logic to load libraries via dlopen, so I added some logging to it: diff --git a/osu.Framework/Platform/Linux/Native/Library.cs b/osu.Framework/Platform/Linux/Native/Library.cs
index 169125fd0b..dd9135e2f5 100644
--- a/osu.Framework/Platform/Linux/Native/Library.cs
+++ b/osu.Framework/Platform/Linux/Native/Library.cs
@@ -3,6 +3,8 @@
#nullable disable
+using osu.Framework.Logging;
+
using System;
using System.Diagnostics;
using System.IO;
@@ -15,6 +17,9 @@ public static class Library
[DllImport("libdl.so.2", EntryPoint = "dlopen")]
private static extern IntPtr dlopen(string library, LoadFlags flags);
+ [DllImport("libdl.so.2", EntryPoint = "dlerror")]
+ private static extern IntPtr dlerror();
+
/// <summary>
/// Loads a library with flags to use with dlopen. Uses <see cref="LoadFlags"/> for the flags
///
@@ -26,11 +31,26 @@ public static void Load(string library, LoadFlags flags)
{
string paths = (string)AppContext.GetData("NATIVE_DLL_SEARCH_DIRECTORIES");
Debug.Assert(paths != null);
+ Logger.Log($"Search paths: {paths}", "linux libs");
foreach (string path in paths.Split(':'))
{
- if (dlopen(Path.Combine(path, library), flags) != IntPtr.Zero)
+ Logger.Log("Trying " + Path.Combine(path, library), "linux libs");
+ if (dlopen(Path.Combine(path, library), flags) == IntPtr.Zero)
+ {
+ string err = string.Empty;
+ var errPtr = dlerror();
+ if (errPtr != IntPtr.Zero)
+ {
+ err = Marshal.PtrToStringAnsi(errPtr);
+ }
+ Logger.Log($"Failed loading {library}: {err}", "linux libs");
+ }
+ else
+ {
+ Logger.Log($"Successfully loaded {library}", "linux libs");
break;
+ }
}
}This is what it logs (for The output you get from I sadly can't get any information on the error since |
|
I set What is it even doing here? |
|
You know sometimes you just go down the depths of madness and decide to yourself "yes, this problem is so cursed, that it's time to write a C program". And you do, and get this back: Which isn't still a solution as much as a vague hint at a direction of one, but at least it's progress? @FreezyLemon does this tell you anything at all? |
|
Found the problem. Enabling VAAPI/VDPAU for Linux has made So, assuming the framework does not want to impose external dependencies, these need to be included for linux targets. I also assume these should be included in the build scripts, which means this is not a quick fix... |
Given we ship half the world already outside of glibc to have functional appimages, we most definitely do not. However,
Does that mean they were... disabled until now? Maybe we can just disable them again for the time being as a stopgap? |
I'm almost 100% sure that they are disabled on master, yes. I don't have a simple way to confirm, but some things point towards it. FFmpeg normally auto-detects the external libraries installed on the build host, and adds those to the build config (this is not the case in the updated build scripts, where it uses a static list of features). On my desktop, this includes VAAPI/VDPAU. But since the GitHub Actions runner image doesn't seem to include these, it probably wasn't enabled when they were built. I also cannot get HW decoding to work on master whatsoever, there's no mention in the runtime logs (even though there should be, even when only attempting to setup HW decoding). As an aside & confirmation for my theory, |
|
Hmm. I think we should either disable video hardware accel on linux again for now (safe option) or probably work around it on CI for the time being and see what happens when we roll that out to users. @peppy @smoogipoo any opinions on how you'd wanna see this played? |
|
I spent some more time on this and have a branch that includes libva and libvdpau in the build scripts. Validating and testing this will take a bit of time though. If you just want to get this done now, you can disable & remove the Linux hwaccel libraries like this: diff --git a/osu.Framework.NativeLibs/scripts/ffmpeg/build-linux.sh b/osu.Framework.NativeLibs/scripts/ffmpeg/build-linux.sh
index 66891d58b..763058f01 100755
--- a/osu.Framework.NativeLibs/scripts/ffmpeg/build-linux.sh
+++ b/osu.Framework.NativeLibs/scripts/ffmpeg/build-linux.sh
@@ -7,13 +7,6 @@ popd > /dev/null
source "$SCRIPT_PATH/common.sh"
FFMPEG_FLAGS+=(
- --enable-vaapi
- --enable-vdpau
- --enable-hwaccel='h264_vaapi,h264_vdpau'
- --enable-hwaccel='hevc_vaapi,hevc_vdpau'
- --enable-hwaccel='vp8_vaapi,vp8_vdpau'
- --enable-hwaccel='vp9_vaapi,vp9_vdpau'
-
--target-os=linux
)
Then rebuild, publish a new NativeLibs version and use that. |
peppy
added a commit
to peppy/osu-framework
that referenced
this pull request
Oct 4, 2023
peppy
force-pushed
the
update-native-libs
branch
from
6f237fd
to
614dd0b
Compare
peppy
force-pushed
the
update-native-libs
branch
from
614dd0b
to
c24103f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.