On 26.07.2017 20:34, Tanu Kaskinen wrote: > On Wed, 2017-07-26 at 08:33 +0200, Georg Chini wrote: >> On 26.07.2017 02:21, Tanu Kaskinen wrote: >>> On Mon, 2017-07-24 at 14:19 +0200, Georg Chini wrote: >>>> This patch extends the subscription API, so that signals sent from PulseAudio >>>> can be processed. A signal can be emitted using pa_signal_post(). >>>> >>>> A client can subscribe to signals by specifying PA_SUBSCRIPTION_MASK_SIGNAL as >>>> one of the subscription mask flags in pa_context_subscribe(). It then needs to >>>> install a signal handler in addtion to (or instead of) the subsription >>>> callback. A new function pa_context_set_signal_callback() has been implemented >>>> to set the signal handler. >>>> >>>> The signal handler will receive three arguments in addition to the usual context >>>> and userdata: >>>> sender - string that specifies the origin of the signal >>>> signal - string that specifies the type of the signal >>>> signal_info - optional string for additional information >>>> >>>> Implementing signal handling as an extension of the subscription API avoids >>>> unnecessary code duplication. >>> With this interface you either subscribe to all signals or none. I >>> think there needs to be a way to set a filter. Something like this: >>> >>> pa_signal_set *signals = pa_signal_set_new(); >>> pa_signal_set_add(signals, "some_signal"); >>> pa_signal_set_add(signals, "..."); >>> ... >>> operation = pa_context_subscribe_to_signals(context, signals, >>> success_cb, userdata); >>> >>> Alternatively, pa_signal_set_new() could have a variable length >>> argument list for listing all the signal names in one go. >> You are right, we should do some filtering here, but I don't see the need >> to be able to subscribe to individual signals. I would rather go for a >> "category mask" like the subscription mask and let the client do at least >> some of the filtering. >> The mask could be passed as argument to pa_context_set_signal_callback(), >> no need to implement additional functions. > The filtering should be done at the server end, so > pa_context_set_signal_callback() can't be used, because it doesn't talk > to the server. I don't understand. With the subscription mask, the filtering is also done on the server end. A signal mask could be similar. And really, I don't see why the client should not also do his part of the filtering. There won't be that many signals that a client could not filter let's say 1 out of 10 signals that belong to some category. > > Also, can you elaborate on how you would define the masks? They can't > be bitfields like the existing subscription masks, because modules > can't extend a bitfield (at least out-of-tree modules, which I would > like to be able to use the signal system). Why would any module need to extend a bitfield? The module is either a signal receiver, in that case it specifies with the signal mask which signals it wants to receive, or a signal sender. In this case it will specify the category of the signal when it calls signal_post(). (The category of the signal should be a parameter of signal_post().) So even if it wants to have a special category of it's own it should be possible. At the moment I have only two categories of signals, internal and miscellaneous, but we could invent the categories as need arises. > >> For me, the simplicity of my approach is one of the big advantages. >> You subscribe to just another event and set up a handler with one >> additional function. There is no need for a new "signal API", the only >> difference to the current subscription is that it uses another handler. > Describing that as "the only difference" seems, if not incorrect, a > little bit of stretching the words. One could also say that it's almost > completely different, the only thing shared (from an application > perspective) is that both systems pa_context_subscribe()... That's a matter of perspective, so I won't comment further. > >>> I'm not convinced that these signals are a good mechanism for inter- >>> module communication, especially if it's implemented in an asynchronous >>> manner, so can we drop the change to the pa_subscription_new() >>> interface? >> Well, subscriptions are also handled asynchronously and a signal can >> carry a lot more information. So for me signals are better suited for >> inter-module communication than the current subscriptions. > Yes, the signal system is a better inter-module communication mechanism > than the subscription system, but that's hardly a fair comparison, > because the subscription system is not an inter-module communication > mechanism at all. Modules can't define their own subscription events. But modules can post and receive subscriptions, even though they cannot define their own events. What other inter-module communication mechanisms do we have currently apart from hooks? > > Modules can define hooks, if they want to send events to other modules. > If your grand plan is to replace hooks with something else, can we > leave that discussion for a later time? I don't want to replace them, but just have other mechanisms like signals and messages beside them because I see that they could be useful and simple to handle. > >> (The >> message interface opens another - synchronous - way of inter-module >> communication, but that's a different patch set.) We could add a >> signal category "internal" that will not be passed on to clients. >> >> Additionally the native protocol is using pa_subscription_new() to set up >> the client subscription, so both are coupled and you cannot easily change >> one without the other. I tried to avoid code duplication wherever possible. > You could define a new hook that the native protocol can use to get the > signals: PA_CORE_HOOK_SEND_SIGNAL. Why do you want to make things complicated? I don't understand what problem you have with the additional argument of pa_subscription_new(), which also has possibilities to use it. Why should a module not be able to subscribe to signals? You say above, you want modules to use the signal system. Then they must be able to subscribe to signals. Your approach is more restricted, more complicated and I cannot see any advantage.
