From a1cd0598ec155d847369804dae59f87333e8d442 Mon Sep 17 00:00:00 2001 From: fritsch Date: Thu, 18 Apr 2024 19:06:05 +0200 Subject: [PATCH 1/3] Revert "PlatformLinux: Prefer Pipewire over Pulse" This reverts commit 01485fa889fe04aa17bc02f5075c371551b45e82. --- xbmc/platform/linux/PlatformLinux.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/xbmc/platform/linux/PlatformLinux.cpp b/xbmc/platform/linux/PlatformLinux.cpp index 6be5163883223..25eb9818da4c4 100644 --- a/xbmc/platform/linux/PlatformLinux.cpp +++ b/xbmc/platform/linux/PlatformLinux.cpp @@ -118,9 +118,9 @@ bool CPlatformLinux::InitStageOne() } else { - if (!OPTIONALS::PipewireRegister()) + if (!OPTIONALS::PulseAudioRegister()) { - if (!OPTIONALS::PulseAudioRegister()) + if (!OPTIONALS::PipewireRegister()) { if (!OPTIONALS::ALSARegister()) { From 180b099125edcbc5393376ef6820795c9f527d26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20H=C3=A4rer?= Date: Mon, 22 Apr 2024 02:16:33 +0200 Subject: [PATCH 2/3] [AudioEngine] Make a smarter choice between PulseAudio and PipeWire --- xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp | 49 ++++++++++++++++++-- xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h | 2 +- xbmc/platform/linux/OptionalsReg.cpp | 6 +-- xbmc/platform/linux/OptionalsReg.h | 2 +- xbmc/platform/linux/PlatformLinux.cpp | 6 +-- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp index c6763bae13877..bfbafa969ef23 100644 --- a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp +++ b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp @@ -19,6 +19,9 @@ #include #include +#include +#include + //----------------------------------------------------------------------------- //----------------------------------------------------------------------------- @@ -27,7 +30,7 @@ class CDriverMonitor public: CDriverMonitor() = default; virtual ~CDriverMonitor(); - bool Start(); + bool Start(bool allowPipeWireCompatServer); bool IsInitialized(); CCriticalSection m_sec; @@ -315,6 +318,24 @@ static void SinkChangedCallback(pa_context *c, pa_subscription_event_type_t t, u } } +struct ServerInfoStruct +{ + std::mutex m_mutex; + std::condition_variable m_cv; + bool m_isInfoSet{false}; + + bool m_isPipeWireServer{}; ///< true if server name contains "PipeWire" in any casing +}; + +static void ServerInfoCallback(pa_context* c, const pa_server_info* i, void* userdata) +{ + auto serverInfo = reinterpret_cast(userdata); + std::unique_lock l(serverInfo->m_mutex); + serverInfo->m_isPipeWireServer = StringUtils::Contains(i->server_name, "pipewire", true); + serverInfo->m_isInfoSet = true; + serverInfo->m_cv.notify_one(); +} + struct SinkInfoStruct { AEDeviceInfoList *list; @@ -656,7 +677,7 @@ bool CDriverMonitor::IsInitialized() return m_isInit; } -bool CDriverMonitor::Start() +bool CDriverMonitor::Start(bool allowPipeWireCompatServer) { if (!SetupContext(nullptr, "KodiDriver", &m_pContext, &m_pMainLoop)) { @@ -664,6 +685,22 @@ bool CDriverMonitor::Start() return false; } +#ifdef HAS_PIPEWIRE + // If Kodi is build with PipeWire support we prefer native PipeWire over the + // PulseAudio compatibility layer IF PulseAudio isn't explicitly selected + if (!allowPipeWireCompatServer) + { + // Check the PulseAudio server name. If it contains PipeWire in any form + // it is the compatibility layer provided by PipeWire + ServerInfoStruct serverInfo; + std::unique_lock l(serverInfo.m_mutex); + pa_context_get_server_info(m_pContext, ServerInfoCallback, &serverInfo); + serverInfo.m_cv.wait(l, [&]() { return serverInfo.m_isInfoSet; }); + if (serverInfo.m_isPipeWireServer) + return false; + } +#endif + pa_threaded_mainloop_lock(m_pMainLoop); m_isInit = true; @@ -687,7 +724,7 @@ bool CDriverMonitor::Start() std::unique_ptr CAESinkPULSE::m_pMonitor; -bool CAESinkPULSE::Register() +bool CAESinkPULSE::Register(bool allowPipeWireCompatServer) { // check if pulseaudio is actually available pa_simple *s; @@ -708,7 +745,11 @@ bool CAESinkPULSE::Register() } m_pMonitor = std::make_unique(); - m_pMonitor->Start(); + if (!m_pMonitor->Start(allowPipeWireCompatServer)) + { + m_pMonitor.reset(); + return false; + } AE::AESinkRegEntry entry; entry.sinkName = "PULSE"; diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h index 3606a0b4f55bb..48f73f9a10af1 100644 --- a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h +++ b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.h @@ -31,7 +31,7 @@ class CAESinkPULSE : public IAESink CAESinkPULSE(); ~CAESinkPULSE() override; - static bool Register(); + static bool Register(bool allowPipeWireCompatServer); static std::unique_ptr Create(std::string& device, AEAudioFormat& desiredFormat); static void EnumerateDevicesEx(AEDeviceInfoList &list, bool force = false); static void Cleanup(); diff --git a/xbmc/platform/linux/OptionalsReg.cpp b/xbmc/platform/linux/OptionalsReg.cpp index 82fb176d1d038..3992f2a0d05b0 100644 --- a/xbmc/platform/linux/OptionalsReg.cpp +++ b/xbmc/platform/linux/OptionalsReg.cpp @@ -33,13 +33,13 @@ bool OPTIONALS::ALSARegister() #ifdef HAS_PULSEAUDIO #include "cores/AudioEngine/Sinks/AESinkPULSE.h" -bool OPTIONALS::PulseAudioRegister() +bool OPTIONALS::PulseAudioRegister(bool allowPipeWireCompatServer) { - bool ret = CAESinkPULSE::Register(); + bool ret = CAESinkPULSE::Register(allowPipeWireCompatServer); return ret; } #else -bool OPTIONALS::PulseAudioRegister() +bool OPTIONALS::PulseAudioRegister(bool) { return false; } diff --git a/xbmc/platform/linux/OptionalsReg.h b/xbmc/platform/linux/OptionalsReg.h index 75fdb6ae62c7a..3a777b70036e7 100644 --- a/xbmc/platform/linux/OptionalsReg.h +++ b/xbmc/platform/linux/OptionalsReg.h @@ -23,7 +23,7 @@ bool ALSARegister(); namespace OPTIONALS { -bool PulseAudioRegister(); +bool PulseAudioRegister(bool allowPipeWireCompatServer); } //----------------------------------------------------------------------------- diff --git a/xbmc/platform/linux/PlatformLinux.cpp b/xbmc/platform/linux/PlatformLinux.cpp index 25eb9818da4c4..e01f8216bf0d8 100644 --- a/xbmc/platform/linux/PlatformLinux.cpp +++ b/xbmc/platform/linux/PlatformLinux.cpp @@ -101,7 +101,7 @@ bool CPlatformLinux::InitStageOne() } else if (sink == "pulseaudio") { - OPTIONALS::PulseAudioRegister(); + OPTIONALS::PulseAudioRegister(true); } else if (sink == "pipewire") { @@ -114,11 +114,11 @@ bool CPlatformLinux::InitStageOne() else if (sink == "alsa+pulseaudio") { OPTIONALS::ALSARegister(); - OPTIONALS::PulseAudioRegister(); + OPTIONALS::PulseAudioRegister(true); } else { - if (!OPTIONALS::PulseAudioRegister()) + if (!OPTIONALS::PulseAudioRegister(false)) { if (!OPTIONALS::PipewireRegister()) { From 6cad37352412345c3d1868b97a4a8a19fe4769c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20H=C3=A4rer?= Date: Tue, 23 Apr 2024 01:21:08 +0200 Subject: [PATCH 3/3] fixup! [AudioEngine] Make a smarter choice between PulseAudio and PipeWire --- xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp | 35 +++++++++++--------- xbmc/platform/freebsd/PlatformFreebsd.cpp | 6 ++-- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp index bfbafa969ef23..5ec64fe9f0bb8 100644 --- a/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp +++ b/xbmc/cores/AudioEngine/Sinks/AESinkPULSE.cpp @@ -320,20 +320,17 @@ static void SinkChangedCallback(pa_context *c, pa_subscription_event_type_t t, u struct ServerInfoStruct { - std::mutex m_mutex; - std::condition_variable m_cv; + pa_threaded_mainloop* m_mainloop; bool m_isInfoSet{false}; - - bool m_isPipeWireServer{}; ///< true if server name contains "PipeWire" in any casing + std::string m_serverName; }; static void ServerInfoCallback(pa_context* c, const pa_server_info* i, void* userdata) { auto serverInfo = reinterpret_cast(userdata); - std::unique_lock l(serverInfo->m_mutex); - serverInfo->m_isPipeWireServer = StringUtils::Contains(i->server_name, "pipewire", true); + serverInfo->m_serverName = i->server_name; serverInfo->m_isInfoSet = true; - serverInfo->m_cv.notify_one(); + pa_threaded_mainloop_signal(serverInfo->m_mainloop, 0); } struct SinkInfoStruct @@ -685,24 +682,32 @@ bool CDriverMonitor::Start(bool allowPipeWireCompatServer) return false; } + pa_threaded_mainloop_lock(m_pMainLoop); + + ServerInfoStruct serverInfo; + serverInfo.m_mainloop = m_pMainLoop; + pa_context_get_server_info(m_pContext, ServerInfoCallback, &serverInfo); + while (!serverInfo.m_isInfoSet) + pa_threaded_mainloop_wait(m_pMainLoop); + + CLog::Log(LOGINFO, "PulseAudio: Server name: {}", serverInfo.m_serverName); + #ifdef HAS_PIPEWIRE // If Kodi is build with PipeWire support we prefer native PipeWire over the - // PulseAudio compatibility layer IF PulseAudio isn't explicitly selected + // PulseAudio compatibility layer IF NOT PulseAudio is explicitly selected if (!allowPipeWireCompatServer) { // Check the PulseAudio server name. If it contains PipeWire in any form // it is the compatibility layer provided by PipeWire - ServerInfoStruct serverInfo; - std::unique_lock l(serverInfo.m_mutex); - pa_context_get_server_info(m_pContext, ServerInfoCallback, &serverInfo); - serverInfo.m_cv.wait(l, [&]() { return serverInfo.m_isInfoSet; }); - if (serverInfo.m_isPipeWireServer) + if (StringUtils::Contains(serverInfo.m_serverName, "pipewire", true)) + { + CLog::Log(LOGINFO, "PulseAudio: Server is PipeWire, bail and use native PipeWire interface"); + pa_threaded_mainloop_unlock(m_pMainLoop); return false; + } } #endif - pa_threaded_mainloop_lock(m_pMainLoop); - m_isInit = true; std::unique_lock lock(m_sec); diff --git a/xbmc/platform/freebsd/PlatformFreebsd.cpp b/xbmc/platform/freebsd/PlatformFreebsd.cpp index c949a339cbebc..6ab3f46ed2564 100644 --- a/xbmc/platform/freebsd/PlatformFreebsd.cpp +++ b/xbmc/platform/freebsd/PlatformFreebsd.cpp @@ -89,7 +89,7 @@ bool CPlatformFreebsd::InitStageOne() } else if (sink == "pulseaudio") { - OPTIONALS::PulseAudioRegister(); + OPTIONALS::PulseAudioRegister(true); } else if (sink == "oss") { @@ -102,11 +102,11 @@ bool CPlatformFreebsd::InitStageOne() else if (sink == "alsa+pulseaudio") { OPTIONALS::ALSARegister(); - OPTIONALS::PulseAudioRegister(); + OPTIONALS::PulseAudioRegister(true); } else { - if (!OPTIONALS::PulseAudioRegister()) + if (!OPTIONALS::PulseAudioRegister(false)) { if (!OPTIONALS::ALSARegister()) {