From 19c6368991706d1bcb4a3b02e08cd70e1e6324c7 Mon Sep 17 00:00:00 2001 From: Nicky Date: Sun, 21 Apr 2024 18:50:38 +0200 Subject: Stream the volume catcher a little: - Use LL_DEBUGS() for potential debug output. - Enclose mutex locking in their own scope, to make unlocking automatic and also limit the life time of a lock to as short as possible - Introduce mCleanupMutex to replace std::unique_lock pwLock(*this). I'm baffled using lock as a mutex like that did even compile. - Remove virtual inheritance, as it is not needed here. --- .../cef/linux/volume_catcher_linux.cpp | 28 ++-- .../media_plugins/cef/linux/volume_catcher_linux.h | 30 +---- .../cef/linux/volume_catcher_pipewire.cpp | 141 ++++++++++----------- indra/media_plugins/cef/volume_catcher.h | 2 +- 4 files changed, 86 insertions(+), 115 deletions(-) diff --git a/indra/media_plugins/cef/linux/volume_catcher_linux.cpp b/indra/media_plugins/cef/linux/volume_catcher_linux.cpp index 3c037b45ba..439c0a6768 100644 --- a/indra/media_plugins/cef/linux/volume_catcher_linux.cpp +++ b/indra/media_plugins/cef/linux/volume_catcher_linux.cpp @@ -32,28 +32,29 @@ VolumeCatcher::VolumeCatcher() { - // only init once we receive which implementation to use - - // debugClear(); - // debugPrint("init volume catcher\n"); } -void VolumeCatcher::onEnablePipeWireVolumeCatcher(bool enable) { +void VolumeCatcher::onEnablePipeWireVolumeCatcher(bool enable) +{ if (pimpl != nullptr) return; - if (enable) { - // debugPrint("volume catcher using pipewire\n"); + if (enable) + { + LL_DEBUGS() << "volume catcher using pipewire" << LL_ENDL; pimpl = new VolumeCatcherPipeWire(); - } else { - // debugPrint("volume catcher using pulseaudio\n"); + } + else + { + LL_DEBUGS() << "volume catcher using pulseaudio" << LL_ENDL; pimpl = new VolumeCatcherPulseAudio(); } } VolumeCatcher::~VolumeCatcher() { - if (pimpl != nullptr) { + if (pimpl != nullptr) + { delete pimpl; pimpl = nullptr; } @@ -68,15 +69,12 @@ void VolumeCatcher::setVolume(F32 volume) void VolumeCatcher::setPan(F32 pan) { - if (pimpl != nullptr) { + if (pimpl != nullptr) pimpl->setPan(pan); - } } void VolumeCatcher::pump() { - if (pimpl != nullptr) { + if (pimpl != nullptr) pimpl->pump(); - } } - diff --git a/indra/media_plugins/cef/linux/volume_catcher_linux.h b/indra/media_plugins/cef/linux/volume_catcher_linux.h index 0f11c537b1..ff00d0672e 100644 --- a/indra/media_plugins/cef/linux/volume_catcher_linux.h +++ b/indra/media_plugins/cef/linux/volume_catcher_linux.h @@ -49,7 +49,7 @@ extern "C" { #include "media_plugin_base.h" -class VolumeCatcherPulseAudio : public virtual VolumeCatcherImpl +class VolumeCatcherPulseAudio : public VolumeCatcherImpl { public: VolumeCatcherPulseAudio(); @@ -78,7 +78,7 @@ public: bool mGotSyms; }; -class VolumeCatcherPipeWire : public virtual VolumeCatcherImpl +class VolumeCatcherPipeWire : public VolumeCatcherImpl { public: VolumeCatcherPipeWire(); @@ -121,31 +121,15 @@ public: F32 mVolume = 1.0f; // max by default // F32 mPan = 0.0f; // center - pw_thread_loop* mThreadLoop; - pw_context* mContext; - pw_core* mCore; - pw_registry* mRegistry; + pw_thread_loop* mThreadLoop = nullptr; + pw_context* mContext = nullptr; + pw_core* mCore = nullptr; + pw_registry* mRegistry = nullptr; spa_hook mRegistryListener; std::unordered_set mChildNodes; std::mutex mChildNodesMutex; + std::mutex mCleanupMutex; }; -// static void debugClear() -// { -// auto file = fopen("volume-catcher-log.txt", "w"); -// fprintf(file, "\n"); -// fclose(file); -// } - -// static void debugPrint(const char* format, ...) -// { -// va_list args; -// va_start(args, format); -// auto file = fopen("volume-catcher-log.txt", "a"); -// vfprintf(file, format, args); -// fclose(file); -// va_end(args); -// } - #endif // VOLUME_CATCHER_LINUX_H diff --git a/indra/media_plugins/cef/linux/volume_catcher_pipewire.cpp b/indra/media_plugins/cef/linux/volume_catcher_pipewire.cpp index ca1808d63c..027fdb77f6 100755 --- a/indra/media_plugins/cef/linux/volume_catcher_pipewire.cpp +++ b/indra/media_plugins/cef/linux/volume_catcher_pipewire.cpp @@ -62,8 +62,7 @@ VolumeCatcherPipeWire::~VolumeCatcherPipeWire() static void registryEventGlobal( void *data, uint32_t id, uint32_t permissions, const char *type, - uint32_t version, const struct spa_dict *props -) + uint32_t version, const struct spa_dict *props) { static_cast(data)->handleRegistryEventGlobal( id, permissions, type, version, props @@ -82,14 +81,14 @@ bool VolumeCatcherPipeWire::loadsyms(std::string pw_dso_name) void VolumeCatcherPipeWire::init() { - // debugPrint("init\n"); - + LL_DEBUGS() << "init" << LL_ENDL; + mGotSyms = loadsyms("libpipewire-0.3.so.0"); if (!mGotSyms) return; - // debugPrint("got syms\n"); + LL_DEBUGS() << "successfully got symbols" << LL_ENDL; llpw_init(NULL, NULL); @@ -115,7 +114,7 @@ void VolumeCatcherPipeWire::init() mRegistry = pw_core_get_registry(mCore, PW_VERSION_REGISTRY, 0); - // debugPrint("got registry\n"); + LL_DEBUGS() << "pw_core_get_registry: " << (mRegistry?"success":"nullptr") << LL_ENDL; spa_zero(mRegistryListener); @@ -125,27 +124,31 @@ void VolumeCatcherPipeWire::init() llpw_thread_loop_start(mThreadLoop); - // debugPrint("started thread loop\n"); + LL_DEBUGS() << "thread loop started" << LL_ENDL; } void VolumeCatcherPipeWire::cleanup() { - std::unique_lock childNodesLock(mChildNodesMutex); - for (auto* childNode : mChildNodes) { - childNode->destroy(); - } - mChildNodes.clear(); - childNodesLock.unlock(); - - std::unique_lock pwLock(*this); - if (mRegistry) - llpw_proxy_destroy((struct pw_proxy*)mRegistry); - spa_zero(mRegistryListener); - if (mCore) - llpw_core_disconnect(mCore); - if (mContext) - llpw_context_destroy(mContext); - pwLock.unlock(); + { + std::unique_lock childNodesLock(mChildNodesMutex); + for (auto *childNode: mChildNodes) + childNode->destroy(); + + mChildNodes.clear(); + } + + { + std::unique_lock pwLock(mCleanupMutex); + if (mRegistry) + llpw_proxy_destroy((struct pw_proxy *) mRegistry); + + spa_zero(mRegistryListener); + + if (mCore) + llpw_core_disconnect(mCore); + if (mContext) + llpw_context_destroy(mContext); + } if (!mThreadLoop) return; @@ -153,49 +156,33 @@ void VolumeCatcherPipeWire::cleanup() llpw_thread_loop_stop(mThreadLoop); llpw_thread_loop_destroy(mThreadLoop); - // debugPrint("cleanup done\n"); + LL_DEBUGS() << "cleanup done" << LL_ENDL; } -void VolumeCatcherPipeWire::lock() { +void VolumeCatcherPipeWire::lock() +{ if (!mThreadLoop) return; llpw_thread_loop_lock(mThreadLoop); } -void VolumeCatcherPipeWire::unlock() { +void VolumeCatcherPipeWire::unlock() +{ if (!mThreadLoop) return; llpw_thread_loop_unlock(mThreadLoop); } -// #define INV_LERP(a, b, v) (v - a) / (b - a) - -// #include - void VolumeCatcherPipeWire::ChildNode::updateVolume() { if (!mActive) return; F32 volume = std::clamp(mImpl->mVolume, 0.0f, 1.0f); - // F32 pan = std::clamp(mImpl->mPan, -1.0f, 1.0f); - - // debugClear(); - // struct timeval time; - // gettimeofday(&time, NULL); - // double t = (double)time.tv_sec + (double)(time.tv_usec / 1000) / 1000; - // debugPrint("time is %f\n", t); - // F32 pan = std::sin(t * 2.0d); - // debugPrint("pan is %f\n", pan); - - // uint32_t channels = 2; - // float volumes[channels]; - // volumes[1] = INV_LERP(-1.0f, 0.0f, pan) * volume; // left - // volumes[0] = INV_LERP(1.0f, 0.0f, pan) * volume; // right - - uint32_t channels = 1; + + const uint32_t channels = 1; float volumes[channels]; volumes[0] = volume; @@ -210,9 +197,10 @@ void VolumeCatcherPipeWire::ChildNode::updateVolume() spa_pod_builder_array(&builder, sizeof(float), SPA_TYPE_Float, channels, volumes); spa_pod* pod = static_cast(spa_pod_builder_pop(&builder, &frame)); - std::lock_guard pwLock(*mImpl); - - pw_node_set_param(mProxy, SPA_PARAM_Props, 0, pod); + { + std::lock_guard pwLock(*mImpl); + pw_node_set_param(mProxy, SPA_PARAM_Props, 0, pod); + } } void VolumeCatcherPipeWire::ChildNode::destroy() @@ -222,16 +210,18 @@ void VolumeCatcherPipeWire::ChildNode::destroy() mActive = false; - std::unique_lock childNodesLock(mImpl->mChildNodesMutex); - mImpl->mChildNodes.erase(this); - childNodesLock.unlock(); - + { + std::unique_lock childNodesLock(mImpl->mChildNodesMutex); + mImpl->mChildNodes.erase(this); + } + spa_hook_remove(&mNodeListener); spa_hook_remove(&mProxyListener); - std::lock_guard pwLock(*mImpl); - - llpw_proxy_destroy(mProxy); + { + std::lock_guard pwLock(*mImpl); + llpw_proxy_destroy(mProxy); + } } static void nodeEventInfo(void* data, const struct pw_node_info* info) @@ -246,17 +236,18 @@ static void nodeEventInfo(void* data, const struct pw_node_info* info) if (!isPluginPid(pid)) return; - // const char* appName = spa_dict_lookup(info->props, PW_KEY_APP_NAME); - // debugPrint("got app: %s\n", appName); + const char* appName = spa_dict_lookup(info->props, PW_KEY_APP_NAME); + LL_DEBUGS() << "got app: " << appName << LL_ENDL; auto* const childNode = static_cast(data); - // debugPrint("init volume to: %f\n", childNode->mImpl->mVolume); + LL_DEBUGS() << "init volume: " << childNode->mImpl->mVolume << LL_ENDL; childNode->updateVolume(); - std::lock_guard childNodesLock(childNode->mImpl->mChildNodesMutex); - - childNode->mImpl->mChildNodes.insert(childNode); + { + std::lock_guard childNodesLock(childNode->mImpl->mChildNodesMutex); + childNode->mImpl->mChildNodes.insert(childNode); + } } static const struct pw_node_events NODE_EVENTS = { @@ -284,9 +275,9 @@ static const struct pw_proxy_events PROXY_EVENTS = { void VolumeCatcherPipeWire::handleRegistryEventGlobal( uint32_t id, uint32_t permissions, const char *type, uint32_t version, - const struct spa_dict *props -) { - if (props == nullptr || strcmp(type, PW_TYPE_INTERFACE_Node) != 0) + const struct spa_dict *props) +{ + if (props == nullptr || type == nullptr || strcmp(type, PW_TYPE_INTERFACE_Node) != 0) return; const char* mediaClass = spa_dict_lookup(props, PW_KEY_MEDIA_CLASS); @@ -310,27 +301,25 @@ void VolumeCatcherPipeWire::handleRegistryEventGlobal( void VolumeCatcherPipeWire::setVolume(F32 volume) { - // debugPrint("setting all volume to: %f\n", volume); + LL_DEBUGS() << "setting volume to: " << volume << LL_ENDL; mVolume = volume; - std::unique_lock childNodeslock(mChildNodesMutex); - std::unordered_set copyOfChildNodes(mChildNodes); - childNodeslock.unlock(); + { + std::unique_lock childNodeslock(mChildNodesMutex); + std::unordered_set copyOfChildNodes(mChildNodes); - // debugPrint("for %d nodes\n", copyOfChildNodes.size()); + LL_DEBUGS() << "found " << copyOfChildNodes.size() << " child nodes" << LL_ENDL; - for (auto* childNode : copyOfChildNodes) { - childNode->updateVolume(); - } + for (auto* childNode : copyOfChildNodes) + childNode->updateVolume(); + } } void VolumeCatcherPipeWire::setPan(F32 pan) { - // mPan = pan; - // setVolume(mVolume); } void VolumeCatcherPipeWire::pump() { -} \ No newline at end of file +} diff --git a/indra/media_plugins/cef/volume_catcher.h b/indra/media_plugins/cef/volume_catcher.h index e11ecf5881..3e53a7e961 100644 --- a/indra/media_plugins/cef/volume_catcher.h +++ b/indra/media_plugins/cef/volume_catcher.h @@ -43,7 +43,7 @@ public: virtual void pump() = 0; // call this at least a few times a second if you can - it affects how quickly we can 'catch' a new audio source and adjust its volume }; -class VolumeCatcher : public virtual VolumeCatcherImpl +class VolumeCatcher : public VolumeCatcherImpl { public: VolumeCatcher(); -- cgit v1.2.3