[pulseaudio-discuss] [PATCH 1/3] core: add generic message interface

[Tanu Kaskinen]

On Sat, 2017-08-05 at 21:32 +0200, Georg Chini wrote:
> On 05.08.2017 13:37, Tanu Kaskinen wrote:
> > On Fri, 2017-08-04 at 15:37 +0200, Georg Chini wrote:
> > > This patch adds a new feature to the core which allows to exchange
> > > messages between objects. An object can register/unregister a message
> > > handler with pa_core_message_handler_{register, unregister}() while
> > > any other object can send a message to the handler using the
> > > pa_core_send_message() function. A message has 5 arguments (apart
> > > from passing the core):
> > > 
> > > recipient: The name of the message handler that will receive the message
> > > message: message command
> > > message_parameters: A string containing additional parameters
> > > message_data: void pointer to some parameter structure, can be used
> > >                as alternative to message_parameters
> > > response: Pointer to a response string that will be filled by the
> > >            message handler. The caller is responsible to free the string.
> > > 
> > > The patch is a precondition for the following patches that also allow
> > > clients to send messages to pulseaudio objects.
> > > 
> > > Because not every message handler should be visible to clients, a flag
> > > was added to the handler structure which allows to mark a handler as
> > > public or private.
> > > 
> > > There is no restriction on object names, except that a handler name
> > > always starts with a "/". The intention is to use a path-like syntax,
> > > for example /core/sink_1 for a sink or /name/instances/index for modules.
> > > The exact naming convention still needs to be agreed.
> > > 
> > > Message groups are also implemented, so that a handler can subscribe
> > > to a message group using pa_core_message_handler_group_[un]subscribe()
> > > to receive messages sent to the group. To distinguish group and handler
> > > names, group names lack the leading "/". Some of the code used to
> > > implement the message groups was adapted from hook-list.c. Message
> > > groups are created/deleted implicitely on subscription/unsubscription.
> > > 
> > > The messaging interface can serve as a full replacement for the current
> > > hook system with several advantages:
> > > - no need to change header files when a new handler/group is implemented
> > > - slightly simpler registration interface
> > > - multi-purpose message handlers that can handle multiple events
> > > - mesage handlers may also be accessible from the client side
> > 
> > We agree that it's good to allow clients to send messages to modules.
> > Unfortunately, in this patch you're assuming that we'll also replace
> > hooks with the same system. Can we please keep things simple and do one
> > change at a time? I'm not enthusiastic about replacing hooks, and I'd
> > rather move on with the client message passing before getting consensus
> > on the hook stuff.
> > 
> > For reference, here's a list of unnecessary (from pure client message
> > passing point of view) things I can gather from the commit message:
> > 
> > - void pointer argument in message handlers
> > - public/private flag
> > - message groups (signals would seem like a better fit for the purpose
> > anyway)
> > 
> 
> Possibly I am only using the wrong words. Let me put it like that:
> My intention is not to replace the hooks but to incorporate them
> into a more general concept. I even used a lot of the code and
> put it in the new message context. The functionality of the hooks
> is a subset of the messages concept and is fully preserved.
> So by replacing hooks with messages, nothing would be lost
> (at least not intentionally), only the interface would change.
> Take a look at the second patch of the series for an example.
> 
> The only disadvantage I can think of is that there is one more
> lookup step required compared to the hooks when finding
> the right handler.

For starters, the conversion has to be implemented, reviewed, and the
new interface has to be learned by everyone. Even if I thought that the
message system didn't have any API design problems, it's not clear that
the conversion would be worth the effort. I would probably be ok with
some new API that allows using one callback for multiple hooks, but I
think you'll need to get an ok from Arun too. I won't elaborate on the
API design issues now, because I don't think it's good to block the
client message passing feature.

> Perhaps you have an example for something that can be
> done with the hooks that would not be possible with the
> message interface. If so, please let me know. Maybe then
> I can understand your concerns better.

I don't doubt that it can be used as a replacement.

> Just to make one point clear again: I am not saying in the
> commit message that the hooks should be replaced, I am
> merely stating that the code has the ability to do so. So
> from my perspective we don't need any consensus on
> what to do with the hooks. You only have to accept a
> parallel mechanism.

Well, I don't accept a parallel mechanism without an agreement and
commitment to get rid of the old mechanism. Having two mechanisms for
doing the same thing doesn't make sense to me.

In any case, I don't think it's an unreasonable request to first
implement the client message passing feature, and only then start
pushing for a hook replacement system.

-- 
Tanu

https://www.patreon.com/tanuk