-
Notifications
You must be signed in to change notification settings - Fork 419
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
Add low latency initialisation support on windows by directly interacting with WASAPI #6088
Changes from all commits
23fe527
fb86f55
5807165
82663ab
f03169d
c5db3e7
37e5f8b
a7a254a
f3c39e8
6813e3b
64360a8
a0b2071
7aafe27
fd52277
71c9fa3
7798dca
28eb229
b91b484
b328590
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,10 @@ | |
using System.Diagnostics; | ||
using System.Linq; | ||
using ManagedBass; | ||
using ManagedBass.Mix; | ||
using ManagedBass.Wasapi; | ||
using osu.Framework.Audio; | ||
using osu.Framework.Bindables; | ||
using osu.Framework.Development; | ||
using osu.Framework.Platform.Linux.Native; | ||
|
||
|
@@ -73,12 +76,16 @@ internal void RegisterManager(AudioManager manager) | |
|
||
managers.Add(manager); | ||
} | ||
|
||
manager.GlobalMixerHandle.BindTo(globalMixerHandle); | ||
} | ||
|
||
internal void UnregisterManager(AudioManager manager) | ||
{ | ||
lock (managers) | ||
managers.Remove(manager); | ||
|
||
manager.GlobalMixerHandle.UnbindFrom(globalMixerHandle); | ||
} | ||
|
||
protected override void OnExit() | ||
|
@@ -109,22 +116,35 @@ protected override void OnExit() | |
FreeDevice(d); | ||
} | ||
|
||
internal static bool InitDevice(int deviceId) | ||
#region BASS Initialisation | ||
|
||
// TODO: All this bass init stuff should probably not be in this class. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure whether this is something to be actioned at this time or left for later but I most definitely agree. Should be in its own class, preferably behind a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could move it now, but was planning to leave for a follow-up effort, because I guarantee that will increase review overhead ten-fold. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I'll look away for now then. |
||
|
||
private WasapiProcedure? wasapiProcedure; | ||
private WasapiNotifyProcedure? wasapiNotifyProcedure; | ||
|
||
/// <summary> | ||
/// If a global mixer is being used, this will be the BASS handle for it. | ||
/// If non-null, all game mixers should be added to this mixer. | ||
/// </summary> | ||
private readonly Bindable<int?> globalMixerHandle = new Bindable<int?>(); | ||
|
||
internal bool InitDevice(int deviceId) | ||
{ | ||
Debug.Assert(ThreadSafety.IsAudioThread); | ||
Trace.Assert(deviceId != -1); // The real device ID should always be used, as the -1 device has special cases which are hard to work with. | ||
|
||
// Try to initialise the device, or request a re-initialise. | ||
if (Bass.Init(deviceId, Flags: (DeviceInitFlags)128)) // 128 == BASS_DEVICE_REINIT | ||
{ | ||
initialised_devices.Add(deviceId); | ||
return true; | ||
} | ||
if (!Bass.Init(deviceId, Flags: (DeviceInitFlags)128)) // 128 == BASS_DEVICE_REINIT | ||
return false; | ||
|
||
attemptWasapiInitialisation(); | ||
|
||
return false; | ||
initialised_devices.Add(deviceId); | ||
return true; | ||
} | ||
|
||
internal static void FreeDevice(int deviceId) | ||
internal void FreeDevice(int deviceId) | ||
{ | ||
Debug.Assert(ThreadSafety.IsAudioThread); | ||
|
||
|
@@ -136,6 +156,8 @@ internal static void FreeDevice(int deviceId) | |
Bass.Free(); | ||
} | ||
|
||
freeWasapi(); | ||
|
||
if (selectedDevice != deviceId && canSelectDevice(selectedDevice)) | ||
Bass.CurrentDevice = selectedDevice; | ||
|
||
|
@@ -155,5 +177,88 @@ internal static void PreloadBass() | |
Library.Load("libbass.so", Library.LoadFlags.RTLD_LAZY | Library.LoadFlags.RTLD_GLOBAL); | ||
} | ||
} | ||
|
||
private void attemptWasapiInitialisation() | ||
{ | ||
if (RuntimeInfo.OS != RuntimeInfo.Platform.Windows) | ||
return; | ||
|
||
int wasapiDevice = -1; | ||
|
||
// WASAPI device indices don't match normal BASS devices. | ||
// Each device is listed multiple times with each supported channel/frequency pair. | ||
// | ||
// Working backwards to find the correct device is how bass does things internally (see BassWasapi.GetBassDevice). | ||
if (Bass.CurrentDevice > 0) | ||
{ | ||
string driver = Bass.GetDeviceInfo(Bass.CurrentDevice).Driver; | ||
|
||
if (!string.IsNullOrEmpty(driver)) | ||
{ | ||
// In the normal execution case, BassWasapi.GetDeviceInfo will return false as soon as we reach the end of devices. | ||
// This while condition is just a safety to avoid looping forever. | ||
// It's intentionally quite high because if a user has many audio devices, this list can get long. | ||
// | ||
// Retrieving device info here isn't free. In the future we may want to investigate a better method. | ||
while (wasapiDevice < 16384) | ||
{ | ||
if (!BassWasapi.GetDeviceInfo(++wasapiDevice, out WasapiDeviceInfo info)) | ||
break; | ||
|
||
if (info.ID == driver) | ||
break; | ||
} | ||
} | ||
} | ||
|
||
// To keep things in a sane state let's only keep one device initialised via wasapi. | ||
freeWasapi(); | ||
initWasapi(wasapiDevice); | ||
} | ||
|
||
private void initWasapi(int wasapiDevice) | ||
{ | ||
// This is intentionally initialised inline and stored to a field. | ||
// If we don't do this, it gets GC'd away. | ||
wasapiProcedure = (buffer, length, _) => | ||
{ | ||
if (globalMixerHandle.Value == null) | ||
return 0; | ||
|
||
return Bass.ChannelGetData(globalMixerHandle.Value!.Value, buffer, length); | ||
}; | ||
wasapiNotifyProcedure = (notify, device, _) => Scheduler.Add(() => | ||
{ | ||
if (notify == WasapiNotificationType.DefaultOutput) | ||
{ | ||
freeWasapi(); | ||
initWasapi(device); | ||
} | ||
}); | ||
|
||
bool initialised = BassWasapi.Init(wasapiDevice, Procedure: wasapiProcedure, Buffer: 0.001f, Period: 0.001f); | ||
|
||
if (!initialised) | ||
return; | ||
|
||
BassWasapi.GetInfo(out var wasapiInfo); | ||
globalMixerHandle.Value = BassMix.CreateMixerStream(wasapiInfo.Frequency, wasapiInfo.Channels, BassFlags.MixerNonStop | BassFlags.Decode | BassFlags.Float); | ||
BassWasapi.Start(); | ||
|
||
BassWasapi.SetNotify(wasapiNotifyProcedure); | ||
} | ||
|
||
private void freeWasapi() | ||
{ | ||
if (globalMixerHandle.Value == null) return; | ||
|
||
// The mixer probably doesn't need to be recycled. Just keeping things sane for now. | ||
Bass.StreamFree(globalMixerHandle.Value.Value); | ||
BassWasapi.Stop(); | ||
BassWasapi.Free(); | ||
globalMixerHandle.Value = null; | ||
} | ||
|
||
#endregion | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get this may be a bit messy, but I don't want to over-complicate things. In the future we probably still want to revisit having a global mixer in all cases (#4915) at which point this will become the norm, not the "exception" for windows-only.
I could have added the global mixer in this PR but I felt that's probably a bad move to limit potential breakage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a global parent mixer is probably fine by itself, but what I worry about more is the changes in flags that the presence of this global mixer forces (
BassFlags.Decode
,BassFlags.MixerChanBuffer | BassFlags.MixerChanNoRampin
).I'd probably expect more commentary in xmldoc here, such that if the global mixer is present, then it basically takes over the duties of playback and all child mixers do not play on their own but rather expose audio data which the global pulls from as required. It seems like a major paradigm shift that isn't really documented anywhere (in fact I'm attempting to back-reason this currently using bass docs, but I'm not really completely sure if I read the intent of setting the aforementioned flags correctly).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the flags are required and it's all just bass nuances. i can document it but at the end of the day all i'll be doing is saying something like "this is done this way because it works". ie. without the decode flag it just breaks everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought there was some more nuance to it other than "make bass work in this configuration". If that is the case then I guess I can just not care.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of this stuff was trial and error in an isolated project until shit just worked 😅 💦