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

Merged
merged 1 commit into from
Feb 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ option(ENABLE_GENERIC "Enables generic build that should run on any little-endia
option(ENABLE_HEADLESS "Enables running Dolphin as a headless variant" OFF)
option(ENABLE_ALSA "Enables ALSA sound backend" ON)
option(ENABLE_PULSEAUDIO "Enables PulseAudio sound backend" ON)
option(ENABLE_CUBEB "Enables Cubeb sound backend" ON)
option(ENABLE_LLVM "Enables LLVM support, for disassembly" ON)
option(ENABLE_TESTS "Enables building the unit tests" ON)
option(ENABLE_VULKAN "Enables vulkan video backend" ON)
Expand Down Expand Up @@ -700,7 +701,10 @@ endif()
add_subdirectory(Externals/soundtouch)
include_directories(Externals/soundtouch)

dolphin_find_optional_system_library(CUBEB Externals/cubeb)
if(ENABLE_CUBEB)
dolphin_find_optional_system_library(CUBEB Externals/cubeb)
add_definitions(-DHAVE_CUBEB)
endif()

if(NOT ANDROID)
dolphin_find_optional_system_library_pkgconfig(
Expand Down
10 changes: 6 additions & 4 deletions Source/Core/AudioCommon/AudioCommon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ constexpr int AUDIO_VOLUME_MAX = 100;

static std::unique_ptr<SoundStream> CreateSoundStreamForBackend(std::string_view backend)
{
if (backend == BACKEND_CUBEB)
if (backend == BACKEND_CUBEB && CubebStream::IsValid())
return std::make_unique<CubebStream>();
else if (backend == BACKEND_OPENAL && OpenALStream::IsValid())
return std::make_unique<OpenALStream>();
Expand Down Expand Up @@ -100,10 +100,11 @@ std::string GetDefaultSoundBackend()
#elif defined __linux__
if (AlsaSound::IsValid())
backend = BACKEND_ALSA;
else
else if (CubebStream::IsValid())
backend = BACKEND_CUBEB;
#elif defined(__APPLE__) || defined(_WIN32) || defined(__OpenBSD__)
backend = BACKEND_CUBEB;
if (CubebStream::IsValid())
backend = BACKEND_CUBEB;
Comment on lines +106 to +107
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.

#endif
return backend;
}
Expand All @@ -118,7 +119,8 @@ std::vector<std::string> GetSoundBackends()
std::vector<std::string> backends;

backends.emplace_back(BACKEND_NULLSOUND);
backends.emplace_back(BACKEND_CUBEB);
if (CubebStream::IsValid())
backends.emplace_back(BACKEND_CUBEB);
if (AlsaSound::IsValid())
backends.emplace_back(BACKEND_ALSA);
if (PulseAudio::IsValid())
Expand Down
18 changes: 14 additions & 4 deletions Source/Core/AudioCommon/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,7 @@ add_library(audiocommon
AudioCommon.h
AudioStretcher.cpp
AudioStretcher.h
CubebStream.cpp
CubebStream.h
CubebUtils.cpp
CubebUtils.h
Enums.h
Mixer.cpp
Mixer.h
Expand All @@ -18,6 +15,16 @@ add_library(audiocommon
WaveFile.h
)

if(ENABLE_CUBEB)
message(STATUS "Cubeb found, enabling Cubeb sound backend")
target_sources(audiocommon PRIVATE
CubebStream.cpp
CubebUtils.cpp
CubebUtils.h
)
target_link_libraries(audiocommon PRIVATE cubeb)
endif()

if(ANDROID)
find_package(OpenSLES)
if(OPENSLES_FOUND)
Expand Down Expand Up @@ -83,10 +90,13 @@ PUBLIC
common

PRIVATE
cubeb::cubeb
SoundTouch
FreeSurround)

if(ENABLE_CUBEB)
target_link_libraries(audiocommon PRIVATE cubeb::cubeb)
endif()

if(MSVC)
# Add precompiled header
target_link_libraries(audiocommon PRIVATE use_pch)
Expand Down
5 changes: 5 additions & 0 deletions Source/Core/AudioCommon/CubebStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,13 @@
#include "AudioCommon/SoundStream.h"
#include "Common/WorkQueueThread.h"

#ifdef HAVE_CUBEB
#include <cubeb/cubeb.h>
#endif

class CubebStream final : public SoundStream
{
#ifdef HAVE_CUBEB
public:
CubebStream();
CubebStream(const CubebStream& other) = delete;
Expand All @@ -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 👍


private:
bool m_stereo = false;
Expand All @@ -43,4 +47,5 @@ class CubebStream final : public SoundStream
static long DataCallback(cubeb_stream* stream, void* user_data, const void* /*input_buffer*/,
void* output_buffer, long num_frames);
static void StateCallback(cubeb_stream* stream, void* user_data, cubeb_state state);
#endif
};
11 changes: 8 additions & 3 deletions Source/Core/Core/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,6 @@ add_library(core
HW/EXI/EXI_DeviceIPL.h
HW/EXI/EXI_DeviceMemoryCard.cpp
HW/EXI/EXI_DeviceMemoryCard.h
HW/EXI/EXI_DeviceMic.cpp
HW/EXI/EXI_DeviceMic.h
HW/EXI/EXI_DeviceModem.cpp
HW/EXI/EXI_DeviceModem.h
HW/EXI/EXI.cpp
Expand Down Expand Up @@ -645,7 +643,6 @@ PUBLIC
videosoftware

PRIVATE
cubeb::cubeb
FatFs
fmt::fmt
LZO::LZO
Expand All @@ -668,6 +665,14 @@ elseif (ANDROID)
)
endif()

if(ENABLE_CUBEB)
target_link_libraries(core PUBLIC cubeb)
target_sources(core PRIVATE
HW/EXI/EXI_DeviceMic.cpp
HW/EXI/EXI_DeviceMic.h
)
endif()

if(TARGET LibUSB::LibUSB)
target_link_libraries(core PUBLIC LibUSB::LibUSB)
target_sources(core PRIVATE
Expand Down
11 changes: 10 additions & 1 deletion Source/Core/Core/HW/EXI/EXI_Device.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,22 @@
#include <memory>

#include "Common/CommonTypes.h"
#include "Common/MsgHandler.h"
#include "Core/HW/EXI/EXI_DeviceAD16.h"
#include "Core/HW/EXI/EXI_DeviceAGP.h"
#include "Core/HW/EXI/EXI_DeviceDummy.h"
#include "Core/HW/EXI/EXI_DeviceEthernet.h"
#include "Core/HW/EXI/EXI_DeviceGecko.h"
#include "Core/HW/EXI/EXI_DeviceIPL.h"
#include "Core/HW/EXI/EXI_DeviceMemoryCard.h"
#include "Core/HW/EXI/EXI_DeviceMic.h"
#include "Core/HW/EXI/EXI_DeviceModem.h"
#include "Core/HW/Memmap.h"
#include "Core/System.h"

#ifdef HAVE_CUBEB
#include "Core/HW/EXI/EXI_DeviceMic.h"
#endif

namespace ExpansionInterface
{
IEXIDevice::IEXIDevice(Core::System& system) : m_system(system)
Expand Down Expand Up @@ -131,7 +135,12 @@ std::unique_ptr<IEXIDevice> EXIDevice_Create(Core::System& system, const EXIDevi
break;

case EXIDeviceType::Microphone:
#ifdef HAVE_CUBEB
result = std::make_unique<CEXIMic>(system, channel_num);
#else
PanicAlertFmtT("Dolphin was built with Cubeb disabled. The Microphone device cannot be used.");
result = std::make_unique<IEXIDevice>(system);
#endif
break;

case EXIDeviceType::Ethernet:
Expand Down
1 change: 1 addition & 0 deletions Source/VSProps/Base.Dolphin.props
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
<PreprocessorDefinitions Condition="'$(Steam)'=='true'">STEAM;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions>USE_RETRO_ACHIEVEMENTS;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions>RC_CLIENT_SUPPORTS_HASH;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions>HAVE_CUBEB;%(PreprocessorDefinitions)</PreprocessorDefinitions>

<!-- Warnings one may want to ignore when using Level4.
4201 nonstandard extension used : nameless struct/union
Expand Down