Break Linux? Fix volume catcher? Only time will tell

indra/plugins/cef/linux_volume_catcher.cpp

@@ -155,55 +155,40 @@ extern "C" {
 }
 
 
-class VolumeCatcherImpl
+class VolumeCatcherImpl : public LLSingleton<VolumeCatcherImpl>
 {
-public:
+	friend LLSingleton<VolumeCatcherImpl>;
 	VolumeCatcherImpl();
+	// This is a singleton class -- both callers and the component implementation should use getInstance() to find the instance.
 	~VolumeCatcherImpl();
 
+public:
+
 	void setVolume(F32 volume);
-	void pump(void);
+	void setPan(F32 pan) {};
 
 	// for internal use - can't be private because used from our C callbacks
 
-	bool loadsyms(std::string pulse_dso_name);
-	void init();
-	void cleanup();
-
-	void update_all_volumes(F32 volume);
 	void update_index_volume(U32 index, F32 volume);
 	void connected_okay();
 
 	std::set<U32> mSinkInputIndices;
 	std::map<U32,U32> mSinkInputNumChannels;
-	F32 mDesiredVolume;
+	F32 mVolume;
 	pa_glib_mainloop *mMainloop;
 	pa_context *mPAContext;
 	bool mConnected;
 	bool mGotSyms;
+	F32 	mPan;
 };
 
 VolumeCatcherImpl::VolumeCatcherImpl()
-	: mDesiredVolume(0.0f),
+	: mVolume(0.0f),
-	  mMainloop(NULL),
+	  mMainloop(nullptr),
-	  mPAContext(NULL),
+	  mPAContext(nullptr),
 	  mConnected(false),
-	  mGotSyms(false)
+	  mGotSyms(false),
-{
+	mPan(0.f)				// default pan is centered
-	init();
-}
-
-VolumeCatcherImpl::~VolumeCatcherImpl()
-{
-	cleanup();
-}
-
-bool VolumeCatcherImpl::loadsyms(std::string pulse_dso_name)
-{
-	return grab_pa_syms(pulse_dso_name);
-}
-
-void VolumeCatcherImpl::init()
 {
 	// try to be as defensive as possible because PA's interface is a
 	// bit fragile and (for our purposes) we'd rather simply not function
@@ -213,7 +198,7 @@ void VolumeCatcherImpl::init()
 	// libpulse.so.0 - this isn't a great assumption, and the two DSOs should
 	// probably be loaded separately.  Our Linux DSO framework needs refactoring,
 	// we do this sort of thing a lot with practically identical logic...
-	mGotSyms = loadsyms("libpulse-mainloop-glib.so.0");
+	mGotSyms = grab_pa_syms("libpulse-mainloop-glib.so.0");
 	if (!mGotSyms) return;
 
 	// better make double-sure glib itself is initialized properly.
@@ -245,7 +230,7 @@ void VolumeCatcherImpl::init()
 	// PA context to a PA daemon.
 	if (mPAContext)
 	{
-		llpa_context_set_state_callback(mPAContext, callback_context_state, this);
+		llpa_context_set_state_callback(mPAContext, callback_context_state, nullptr);
 		pa_context_flags_t cflags = (pa_context_flags)0; // maybe add PA_CONTEXT_NOAUTOSPAWN?
 		if (llpa_context_connect(mPAContext, NULL, cflags, NULL) >= 0)
 		{
@@ -260,7 +245,7 @@ void VolumeCatcherImpl::init()
 	}
 }
 
-void VolumeCatcherImpl::cleanup()
+VolumeCatcherImpl::~VolumeCatcherImpl()
 {
 	mConnected = false;
 
@@ -269,33 +254,30 @@ void VolumeCatcherImpl::cleanup()
 		llpa_context_disconnect(mPAContext);
 		llpa_context_unref(mPAContext);
 	}
-	mPAContext = NULL;
+	mPAContext = nullptr;
 
 	if (mGotSyms && mMainloop)
 	{
 		llpa_glib_mainloop_free(mMainloop);
 	}
-	mMainloop = NULL;
+	mMainloop = nullptr;
 }
 
 void VolumeCatcherImpl::setVolume(F32 volume)
 {
-	mDesiredVolume = volume;
+	mVolume = volume;
 	
 	if (!mGotSyms) return;
 
 	if (mConnected && mPAContext)
 	{
-		update_all_volumes(mDesiredVolume);
+		for (auto& input : mSinkInputIndices)
+		{
+			update_index_volume(input, mVolume);
+		}
 	}
 
-	pump();
+	g_main_context_iteration(g_main_context_default(), /*may_block = */false);
-}
-
-void VolumeCatcherImpl::pump()
-{
-	gboolean may_block = FALSE;
-	g_main_context_iteration(g_main_context_default(), may_block);
 }
 
 void VolumeCatcherImpl::connected_okay()
@@ -305,7 +287,7 @@ void VolumeCatcherImpl::connected_okay()
 	// fetch global list of existing sinkinputs
 	if ((op = llpa_context_get_sink_input_info_list(mPAContext,
 							callback_discovered_sinkinput,
-							this)))
+							nullptr)))
 	{
 		llpa_operation_unref(op);
 	}
@@ -313,7 +295,7 @@ void VolumeCatcherImpl::connected_okay()
 	// subscribe to future global sinkinput changes
 	llpa_context_set_subscribe_callback(mPAContext,
 					    callback_subscription_alert,
-					    this);
+					    nullptr);
 	if ((op = llpa_context_subscribe(mPAContext, (pa_subscription_mask_t)
 					 (PA_SUBSCRIPTION_MASK_SINK_INPUT),
 					 NULL, NULL)))
@@ -322,15 +304,6 @@ void VolumeCatcherImpl::connected_okay()
 	}
 }
 
-void VolumeCatcherImpl::update_all_volumes(F32 volume)
-{
-	for (std::set<U32>::iterator it = mSinkInputIndices.begin();
-	     it != mSinkInputIndices.end(); ++it)
-	{
-		update_index_volume(*it, volume);
-	}
-}
-
 void VolumeCatcherImpl::update_index_volume(U32 index, F32 volume)
 {
 	static pa_cvolume cvol;
@@ -340,10 +313,10 @@ void VolumeCatcherImpl::update_index_volume(U32 index, F32 volume)
 	pa_context *c = mPAContext;
 	uint32_t idx = index;
 	const pa_cvolume *cvolumep = &cvol;
-	pa_context_success_cb_t cb = NULL; // okay as null
+	pa_context_success_cb_t cb = nullptr; // okay as null
-	void *userdata = NULL; // okay as null
+	void* userdata = nullptr; // okay as null
 
-	pa_operation *op;
+	pa_operation* op;
 	if ((op = llpa_context_set_sink_input_volume(c, idx, cvolumep, cb, userdata)))
 	{
 		llpa_operation_unref(op);
@@ -353,9 +326,6 @@ void VolumeCatcherImpl::update_index_volume(U32 index, F32 volume)
 
 void callback_discovered_sinkinput(pa_context *context, const pa_sink_input_info *sii, int eol, void *userdata)
 {
-	VolumeCatcherImpl *impl = dynamic_cast<VolumeCatcherImpl*>((VolumeCatcherImpl*)userdata);
-	llassert(impl);
-
 	if (0 == eol)
 	{
 		pa_proplist *proplist = sii->proplist;
@@ -363,6 +333,7 @@ void callback_discovered_sinkinput(pa_context *context, const pa_sink_input_info
 		
 		if (sinkpid == getpid()) // does the discovered sinkinput belong to this process?
 		{
+			auto impl = VolumeCatcherImpl::getInstance();
 			bool is_new = (impl->mSinkInputIndices.find(sii->index) ==
 				       impl->mSinkInputIndices.end());
 			
@@ -371,12 +342,7 @@ void callback_discovered_sinkinput(pa_context *context, const pa_sink_input_info
 			
 			if (is_new)
 			{
-				// new!
+				impl->update_index_volume(sii->index, impl->mVolume);
-				impl->update_index_volume(sii->index, impl->mDesiredVolume);
-			}
-			else
-			{
-				// seen it already, do nothing.
 			}
 		}
 	}
@@ -384,8 +350,7 @@ void callback_discovered_sinkinput(pa_context *context, const pa_sink_input_info
 
 void callback_subscription_alert(pa_context *context, pa_subscription_event_type_t t, uint32_t index, void *userdata)
 {
-	VolumeCatcherImpl *impl = dynamic_cast<VolumeCatcherImpl*>((VolumeCatcherImpl*)userdata);
+	VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();
-	llassert(impl);
 
 	switch (t & PA_SUBSCRIPTION_EVENT_FACILITY_MASK) {
         case PA_SUBSCRIPTION_EVENT_SINK_INPUT:
@@ -401,15 +366,11 @@ void callback_subscription_alert(pa_context *context, pa_subscription_event_type
 		{
 			// ask for more info about this new sinkinput
 			pa_operation *op;
-			if ((op = llpa_context_get_sink_input_info(impl->mPAContext, index, callback_discovered_sinkinput, impl)))
+			if ((op = llpa_context_get_sink_input_info(impl->mPAContext, index, callback_discovered_sinkinput, nullptr)))
 			{
 				llpa_operation_unref(op);
 			}
 		}
-		else
-		{
-			// property change on this sinkinput - we don't care.
-		}
 		break;
 		
 	default:;
@@ -418,8 +379,7 @@ void callback_subscription_alert(pa_context *context, pa_subscription_event_type
 
 void callback_context_state(pa_context *context, void *userdata)
 {
-	VolumeCatcherImpl *impl = dynamic_cast<VolumeCatcherImpl*>((VolumeCatcherImpl*)userdata);
+	VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();
-	llassert(impl);
 	
 	switch (llpa_context_get_state(context))
 	{
@@ -441,28 +401,25 @@ void callback_context_state(pa_context *context, void *userdata)
 
 VolumeCatcher::VolumeCatcher()
 {
-	pimpl = new VolumeCatcherImpl();
+	pimpl = VolumeCatcherImpl::getInstance();
 }
 
 VolumeCatcher::~VolumeCatcher()
 {
-	delete pimpl;
+	// Let the instance persist until exit.
-	pimpl = NULL;
 }
 
 void VolumeCatcher::setVolume(F32 volume)
 {
-	llassert(pimpl);
 	pimpl->setVolume(volume);
 }
 
 void VolumeCatcher::setPan(F32 pan)
 {
-	// TODO: implement this (if possible)
+	pimpl->setPan(pan);
 }
 
 void VolumeCatcher::pump()
 {
-	llassert(pimpl);
+	g_main_context_iteration(g_main_context_default(), /*may_block = */false);
-	pimpl->pump();
 }

indra/plugins/cef/mac_volume_catcher.cpp

@@ -45,67 +45,47 @@
 
 struct VolumeCatcherStorage;
 
-class VolumeCatcherImpl
+class VolumeCatcherImpl : public LLSingleton<VolumeCatcherImpl>
 {
+	friend LLSingleton<VolumeCatcherImpl>;
+	VolumeCatcherImpl();
+	// This is a singleton class -- both callers and the component implementation should use getInstance() to find the instance.
+	~VolumeCatcherImpl();
+
 public:
 
 	void setVolume(F32 volume);
 	void setPan(F32 pan);
 	
-	void setInstanceVolume(VolumeCatcherStorage *instance);
-	
 	std::list<VolumeCatcherStorage*> mComponentInstances;
 	Component mOriginalDefaultOutput;
 	Component mVolumeAdjuster;
-	
+
-	static VolumeCatcherImpl *getInstance();
 private:
-	// This is a singleton class -- both callers and the component implementation should use getInstance() to find the instance.
-	VolumeCatcherImpl();
-	static VolumeCatcherImpl *sInstance;
-	
-	// The singlar instance of this class is expected to last until the process exits.
-	// To ensure this, we declare the destructor here but never define it, so any code which attempts to destroy the instance will not link.
-	~VolumeCatcherImpl();	
-	
 	F32 mVolume;
 	F32 mPan;
 };
 
-VolumeCatcherImpl *VolumeCatcherImpl::sInstance = NULL;;
-
 struct VolumeCatcherStorage
 {
-	ComponentInstance self;
+	ComponentInstance self, delegate;
-	ComponentInstance delegate;
 };
 
 static ComponentResult volume_catcher_component_entry(ComponentParameters *cp, Handle componentStorage);
 static ComponentResult volume_catcher_component_open(VolumeCatcherStorage *storage, ComponentInstance self);
 static ComponentResult volume_catcher_component_close(VolumeCatcherStorage *storage, ComponentInstance self);
 
-VolumeCatcherImpl *VolumeCatcherImpl::getInstance()
-{
-	if(!sInstance)
-	{
-		sInstance = new VolumeCatcherImpl;
-	}
-	
-	return sInstance;
-}
-
 VolumeCatcherImpl::VolumeCatcherImpl()
+:	mVolume(1.0f),			// default volume is max
+	mPan(0.f)				// default pan is centered
 {
-	mVolume = 1.0;	// default to full volume
-	mPan = 0.0;		// and center pan
-		
 	ComponentDescription desc;
 	desc.componentType = kAudioUnitType_Output;
 	desc.componentSubType = kAudioUnitSubType_DefaultOutput;
 	desc.componentManufacturer = kAudioUnitManufacturer_Apple;
 	desc.componentFlags = 0;
 	desc.componentFlagsMask = 0;
-	
+
 	// Find the original default output component
 	mOriginalDefaultOutput = FindNextComponent(NULL, &desc);
 
@@ -114,7 +94,6 @@ VolumeCatcherImpl::VolumeCatcherImpl()
 
 	// Capture the original component, so we always get found instead.
 	CaptureComponent(mOriginalDefaultOutput, mVolumeAdjuster);
-
 }
 
 static ComponentResult volume_catcher_component_entry(ComponentParameters *cp, Handle componentStorage)
@@ -146,20 +125,18 @@ static ComponentResult volume_catcher_component_entry(ComponentParameters *cp, H
 
 static ComponentResult volume_catcher_component_open(VolumeCatcherStorage *storage, ComponentInstance self)
 {
-	ComponentResult result = noErr;
 	VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();	
 	
 	storage = new VolumeCatcherStorage;
 
 	storage->self = self;
-	storage->delegate = NULL;
+	storage->delegate = nullptr;
 
-	result = OpenAComponent(impl->mOriginalDefaultOutput, &(storage->delegate));
+	ComponentResult result = OpenAComponent(impl->mOriginalDefaultOutput, &(storage->delegate));
 	
 	if(result != noErr)
 	{
 //		std::cerr << "OpenAComponent result = " << result << ", component ref = " << storage->delegate << std::endl;
-		
 		// If we failed to open the delagate component, our open is going to fail.  Clean things up.
 		delete storage;
 	}
@@ -172,7 +149,7 @@ static ComponentResult volume_catcher_component_open(VolumeCatcherStorage *stora
 		impl->mComponentInstances.push_back(storage);	
 		
 		// and set up the initial volume
-		impl->setInstanceVolume(storage);
+		impl->setVolume(storage);
 	}
 
 	return result;
@@ -180,14 +157,12 @@ static ComponentResult volume_catcher_component_open(VolumeCatcherStorage *stora
 
 static ComponentResult volume_catcher_component_close(VolumeCatcherStorage *storage, ComponentInstance self)
 {
-	ComponentResult result = noErr;
-	
 	if(storage)
 	{
 		if(storage->delegate)
 		{
 			CloseComponent(storage->delegate);
-			storage->delegate = NULL;
+			storage->delegate = nullptr;
 		}
 		
 		VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();	
@@ -195,18 +170,30 @@ static ComponentResult volume_catcher_component_close(VolumeCatcherStorage *stor
 		delete[] storage;
 	}
 		
-	return result;
+	return noErr;
 }
 
 void VolumeCatcherImpl::setVolume(F32 volume)
 {
-	VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();	
+	mVolume = volume;
-	impl->mVolume = volume;
+
-	
 	// Iterate through all known instances, setting the volume on each.
-	for(std::list<VolumeCatcherStorage*>::iterator iter = mComponentInstances.begin(); iter != mComponentInstances.end(); ++iter)
+	for(auto& instance : mComponentInstances)
 	{
-		impl->setInstanceVolume(*iter);
+//		std::cerr << "Setting volume on component instance: " << (instance->delegate) << " to " << mVolume << std::endl;
+		if (instance && instance->delegate)
+		{
+			if (OSStatus err = AudioUnitSetParameter(
+					instance->delegate,
+					kHALOutputParam_Volume,
+					kAudioUnitScope_Global,
+					0,
+					mVolume,
+					0))
+			{
+//				std::cerr << "    AudioUnitSetParameter returned " << err << std::endl;
+			}
+		}
 	}
 }
 
@@ -214,35 +201,12 @@ void VolumeCatcherImpl::setPan(F32 pan)
 {
 	VolumeCatcherImpl *impl = VolumeCatcherImpl::getInstance();	
 	impl->mPan = pan;
-	
+
 	// TODO: implement this.
 	// This will probably require adding a "panner" audio unit to the chain somehow.
 	// There's also a "3d mixer" component that we might be able to use...
 }
 
-void VolumeCatcherImpl::setInstanceVolume(VolumeCatcherStorage *instance)
-{
-//	std::cerr << "Setting volume on component instance: " << (instance->delegate) << " to " << mVolume << std::endl;
-	
-	OSStatus err = noErr;
-	
-	if(instance && instance->delegate)
-	{
-		err = AudioUnitSetParameter(
-				instance->delegate, 
-				kHALOutputParam_Volume, 
-				kAudioUnitScope_Global,
-				0, 
-				mVolume, 
-				0);
-	}
-	
-	if(err)
-	{
-//		std::cerr << "    AudioUnitSetParameter returned " << err << std::endl;
-	}
-}
-
 /////////////////////////////////////////////////////
 
 VolumeCatcher::VolumeCatcher()