Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FFmpeg: Only load versioned libraries on Linux #6154

Merged
merged 4 commits into from Mar 26, 2024

Conversation

FreezyLemon
Copy link
Contributor

@FreezyLemon FreezyLemon commented Jan 28, 2024

Prerequisite:

macOS and Windows already use versioned files, e.g. libavutil.56.dylib, avutil-56.dll. Linux uses unversioned binaries at the moment, presumably because the NativeLibrary.Load methods don't handle the Linux versioning conventions (libavutil.so.56) very well.

This change makes it so only compatible FFmpeg libraries are loaded1, which is the first step towards implementing #6025.

Footnotes

  1. As determined by the file name, which has some obvious caveats. Properly checking the library version seems like an overkill.

This reduces the likelihood of loading incompatible FFmpeg binaries
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jan 29, 2024
Copy link
Contributor

@sr229 sr229 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small nits, I think you may need to make a new CI pipeline to test the switching.

osu.Framework/Graphics/Video/VideoDecoder.cs Show resolved Hide resolved
@bdach
Copy link
Collaborator

bdach commented Feb 12, 2024

"you may need to make a new CI pipeline to test the switching"

@sr229 what switching?

@sr229
Copy link
Contributor

sr229 commented Feb 12, 2024

"you may need to make a new CI pipeline to test the switching"

@sr229 what switching?

Apologies, it seems this code is also meaning to set the stone for #6025 so I may have misread the intent of the PR as asking for a precise version especially in the context of the aforementioned issue this would break on a minor version upgrade. Please disregard and dismiss.

Copy link
Contributor

@smoogipoo smoogipoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm! Thanks!

@smoogipoo smoogipoo merged commit 51d8215 into ppy:master Mar 26, 2024
20 checks passed
@FreezyLemon FreezyLemon deleted the linux-load-versioned-ffmpeg-libs branch March 26, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants