dolphin
dolphin copied to clipboard
Update cubeb to mozilla/cubeb@773f16b7ea308392c05be3e290163d1f636e6024
Our current revision is from October 2017, so it's reasonable to update it. This fixes a long-standing issue where Windows would not remember Dolphin's volume as set in the Volume Mixer between sessions.
What complicates this is that cubeb at some point changed so it requires the user to initialize COM on Windows rather than doing it by itself, and also (at least via documentation, although it seems in practice this isn't a hard rule -- still, one should abide by the documentation...) requires that all calls to the cubeb API be only done on threads where COM has been initialized in MTA (CoInitializeEx(nullptr, COINIT_MULTITHREADED)
) mode.
I've done this by adding a RunInCubebContext()
function that tries to initialize in MTA mode, and if that fails creates a temporary thread to do it there instead. This is inspired by a long comment thread over in #8920, the last attempt at this, though with my own spin on it. That attempt never got merged, but maybe this one will be good enough?
Some extra notes:
- The cubeb sources are 1:1 copied from the linked commit, except for the
CMakeLists.txt
, which was slightly modified to work with our CMake build scripts. - Two extra libraries are required to be linked on Windows compared to the October 2017 version, which have been added to the relevant
.csproj
files.
Worth noting that it seems Mozilla deprecated (some parts of) the library in favor of the rust version: https://github.com/mozilla/cubeb/wiki/Backend-Support Not that it means we shouldn't use it, but just good to be aware.
I would recommend using wil for the COM management , since it nicely solves the problem(s) and we already include it (maybe time to update that External, as well :))
Otherwise, lgtm
Hey this actually builds now with the build server upgrade, great!
nice.
in addition to using WIL, what do you think about using submodule instead of copying the sources?
Yeah I think we can use a submodule.
Regarding WIL, I'm not quite sure how to design this as-is with WIL? I guess it would in general be better to always use a separate thread here since we don't want to randomly enable MTA mode permanently on whatever thread happens to call a Cubeb function... let's see...
hrm, starting a thread to handle a single function call seems a bit smelly :p but yea, maybe it doesn't fit.
We could have a cubeb worker thread that gets function calls to execute via a queue, if that's better.
7c8dd95bf6ab0abc43c6baddb29e9d341cff7f5a is a variant using a worker thread. It's a bit ridiculous, but maybe actually the better option? Avoids the CoInitialize on the main thread and never opens any temporary secondary threads.
i was looking at c++/winrt stuff recently and noticed it has functionality to forward coroutines/callbacks onto a threadpool (for cross-apartment invocations). I wouldn't be surprised if WIL has a similar thing (or maybe it's better to just use the code in winrt header, anyway - it's <winrt/base.h>
). Generally it just provides c++ syntactic sugar on com.
I'll be honest, I'm not sure how either WIL or WinRT help me here. If you have an idea how to do this better please go ahead, because this is the best I can come up with.
I’m not going to get to that and the pr has been around forever so if you think it’s good go for it
Well, don't mind me then...
Depending on how the deprecation goes we could presumably use the C++ bindings to cubeb-rs, btw.
Happy to see this got merged. Thanks for quoting your references. I have resumed work on my other audio PRs, so this should make things a little simpler.
@AdmiralCurtiss
It seems like cubeb still hasn't re-enabled IAudioClient3
by default, which would allow for lower latency on Win 10/11:
PR commit
cubeb bug thread
I'd say we should give it a try given that it should sensibly reduce audio lag, and almost make it on par with exclusive mode WASAPI. If any issues arise, we could disable it again, but it's been a while since that bug was opened, so maybe Windows updates fixed it, and it seems like it had always been a problem exclusively with input stream, which are still prevented from using IAudioClient3
with my change.
@AdmiralCurtiss It seems like cubeb still hasn't re-enabled
IAudioClient3
by default
I've done it here: https://github.com/dolphin-emu/dolphin/pull/11466