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

CMakeLists: Add flag to disable Cubeb #13306

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

Conversation

OatmealDome
Copy link
Member

@OatmealDome OatmealDome commented Jan 24, 2025

Cubeb doesn't support some platforms, so we should have the ability to disable it.

Outside of AudioCommon, CEXIMic uses Cubeb to get microphone input, so I added a panic alert when the Microphone device is selected and Cubeb is unavailable.

Comment on lines +106 to +107
if (CubebStream::IsValid())
backend = BACKEND_CUBEB;
Copy link
Member

Choose a reason for hiding this comment

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

Can we just move this out of the #ifdef soup and do it all the time? I'd expect that any platforms not in this chain (like ANDROID for example) to return false so it'd be skipped anyways.

Copy link
Member Author

Choose a reason for hiding this comment

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

afaik Android uses OpenSL ES by default, so moving this block out of the ifdefs would cause Android to default to Cubeb.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah well, you could move it in front, but I guess either is fine then 🤷

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would then affect Linux instead and make ALSA the default backend instead of Cubeb?

Copy link
Member

Choose a reason for hiding this comment

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

Hm true. Bleh. Nevermind then I guess. But such define chains usually irk the heck out of me, and I try to avoid (or rewrite) them in favor of making it easier to follow. Not today I suppose.

@@ -25,6 +28,7 @@ class CubebStream final : public SoundStream
bool Init() override;
bool SetRunning(bool running) override;
void SetVolume(int) override;
static bool IsValid() { return true; }
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this have a symmetric one in the #else portion that returns false instead? Unless I missed something, this shouldn't compile correctly without HAVE_CUBEB, since there's no CubebStream::IsValid member in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

If it isn't defined, then it will fall back to the IsValid definition in SoundStream:

static bool IsValid() { return false; }

Copy link
Member

Choose a reason for hiding this comment

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

Oh, great, I didn't realize it was a base class method. Too bad you can't slap override on a static method to make it explicit. Guess you can ignore both my comments then 👍

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

Successfully merging this pull request may close these issues.

2 participants