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

[Georg Chini]

On 06.08.2017 15:29, Georg Chini wrote:
> On 06.08.2017 07:26, Tanu Kaskinen wrote:
>> 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)
>>>>
>>>
>
> I wonder if you are at all willing to accept something more general or
> are completely focused on the client-only approach. I hoped, that the
> features and the two use cases (internal: implementation of signals,
> external: message handler to list handlers) could convince you that
> the new messaging system is a definite win for pulseaudio even if it
> partly duplicates existing functionality.
> My current implementation might have drawbacks and problems
> that I do not see, but I am willing to put more work into it until those
> potential issues are solved. I already pointed out multiple advantages
> over existing mechanisms and your categoric denial currently seems
> only based on the hook discussion and therefore a bit unfounded to
> me.
Meanwhile I have seen a few points where my code seems not general
enough.

If you are willing to accept some "versatile message interface", I would
re-work it again. I would then appreciate your additional input regarding
design.

Otherwise I will reluctantly strip it down to what you want because further
discussion seems pointless.