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

[Georg Chini]

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)
>
Somehow I expected your reaction. It's a pity, because I believe
that the messaging system really has benefits over what currently
exists. That's why I decided to post the patches anyway and still
hope that I can say something to convince you.

I put some work in it and in fact incorporated everything that
the hooks are doing today (partly by stealing code). If you take a
closer look at it or refer to the second patch, you will see, that the
implementation is very similar in many respects, so moving from
hooks to messages would not be a big task. The send_message()
function is the equivalent to hook_fire() and handler_register() +
group_subscribe() is the equivalent to hook_connect(), you can
nearly replace them one-to-one.

I think however, that the two mechanisms can co-exist, at least
for the moment. I was not planning to replace all hooks any time
soon and my patch only adds the functionality that would be
needed. That does not prevent us from using the patch for the
client interface without replacing anything. The additional
functionality is not completely unused, the patch that implements
signals uses it (as an example for how it is done with messages
instead of hooks).

The message groups are more or less that what the hooks are
today, but they have several advantages. One of the main points
is that the message handlers can implement multiple commands.
The hooks only support one-to-one or one-to-many situations.
This means that one message can be sent to one or more hooks,
but it is not possible to have a many-to-one relationship so that
one hook can process multiple different messages.
Other points are that the same mechanism can be used for
internal and client communication and that the message handlers
can still be addressed individually, even if they are part of a group.
Overall, the messaging interface is more flexible than the hook
system and even provides a more or less one-to-one replacement.
I don't see why we should not at least give it a try, even if it is
some more code to review.

On the long run, I would volunteer to do the replacement of the
hooks if we decide at some point that it makes sense after a test
phase for the messages.
It would be nice to have a unified messaging system that is used
for inter-object communication throughout pulseaudio and in the
communication with the clients.