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

Workaround for AL interop on Linux #421

Open
wants to merge 1 commit into
base: netstandard
Choose a base branch
from

Conversation

sk-zk
Copy link

@sk-zk sk-zk commented Aug 15, 2020

This fix makes ALSoundOut work on both Linux and Windows.

Admittedly, it's not an ideal solution, but since the path for DllImport has to be constant, I couldn't really find a better way to do it.

@filoe
Copy link
Owner

filoe commented Aug 15, 2020

Thanks for contributing.
Maybe we could add support for MacOS as well?

@sk-zk
Copy link
Author

sk-zk commented Aug 15, 2020

Probably. I don't know anything about Mac though so someone else is going to have to check that

@filoe
Copy link
Owner

filoe commented Aug 15, 2020

Tried to look it up here https://github.com/opentk/opentk/blob/master/src/OpenAL/OpenToolkit.OpenAL/OpenALLibraryNameContainer.cs

Maybe we could load it dynamically anyways. I've already did something like that for xaudio and seems like opentk is doing something similar for all openal apis.

@mnadareski
Copy link

I had to end up modifying this slightly so that it worked, also putting it after the configuration PropertyGroups:

  <PropertyGroup Condition=" '$(OS)' == 'Windows_NT' ">
    <DefineConstants>$(DefineConstants);WINDOWS</DefineConstants>
  </PropertyGroup>

@Silverdimond
Copy link

Silverdimond commented Jun 30, 2023

I think a more viable solution that wouldn't require for it to be built for one or other platform would be following the microsoft example on cross platform P/Invoke
https://learn.microsoft.com/en-us/dotnet/standard/native-interop/cross-platform
I am not sure where to put the NativeLibrary.SetDllImportResolver instruction but it does seem to work if i put it in the constructor for ALSoundOut never mind you can only set it once which complicates things further.

//ALSoundOut line 89
public ALSoundOut(int latency, ThreadPriority playbackThreadPriority, SynchronizationContext eventSyncContext)
{
   if (latency <= 0)
     throw new ArgumentOutOfRangeException("latency");
   _latency = latency;
   _playbackPriority = playbackThreadPriority;
   _syncContext = eventSyncContext;
   if(!ResolverIsSet)
   {
      ResolverIsSet = true;
      NativeLibrary.SetDllImportResolver(Assembly.GetExecutingAssembly(), ALInteropsNativeMethods.DllImportResolver);
   }
   if (!ALInteropsNativeMethods.IsSupported())
   {
       throw new PlatformNotSupportedException("openAL is not supported by the current platform. Consider installing openAL on the current platform.");
   }
}
/// <summary>
/// Has ALSoundOut been called previously, if so skip setting the DLL import resolver
/// </summary>
 private static bool ResolverIsSet = false;
        
 //rename ALInterops to ALInteropsNativeMethods https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1060
internal static class ALInteropsNativeMethods 
{
  private const string libLocation = "libopenal";
  public static IntPtr DllImportResolver(string libraryName, Assembly assembly, DllImportSearchPath? searchPath)
  {
     if (libraryName == "libopenal")
     {
         if (OperatingSystem.IsWindows())
         {
             return NativeLibrary.Load("openal32.dll", assembly, searchPath);
         }
      }
     return IntPtr.Zero;
 }

I've just realised that OperatingSystem.IsWindows() might require net6, you could probably just change it over to a check for Environment.OSVersion or better yet RuntimeInformation.IsOSPlatform(OSPlatform.Windows)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants