[pulseaudio-discuss] [PATCH 1/2] protocol-native: Add commands for communication with modules

[Georg Chini]

On 07.07.2017 16:12, Tanu Kaskinen wrote:
> On Thu, 2017-07-06 at 21:35 +0200, Georg Chini wrote:
>> On 06.07.2017 15:25, Tanu Kaskinen wrote:
>>> On Wed, 2017-07-05 at 08:56 +0200, Georg Chini wrote:
>>>> On 04.07.2017 18:15, Tanu Kaskinen wrote:
>>>>> On Sun, 2017-05-21 at 14:56 +0200, Georg Chini wrote:
>>>>>> This patch adds two new commands to the native protocol that enable direct
>>>>>> communication with modules during run time. SEND_MODULE_COMMAND is used to
>>>>>> send a command string to a module if no reply is needed. This can for example
>>>>>> be employed to change parameters on the fly. SEND_MODULE_QUERY is similar to
>>>>>> SEND_MODULE_COMMAND but expects a reply string from the module. This command
>>>>>> can for example be used to query the current value of parameters.
>>>>>> Within the module, the functions set_command_callback() and get_command_callback()
>>>>>> need to be implemented to handle the two types of requests.
>>>>>>
>>>>>> This is a rather generic interface because it only passes strings between the
>>>>>> module and a client without making any assumptions about the content of the
>>>>>> strings.
>>>>
>>>>
>>>>> I'm not necessarily opposed to replacing the extension system with
>>>>> something new. The extension system is cumbersome to work with, and
>>>>> it's probably possible to come up with something better.
>>>>>
>>>>> If I recall correctly, you mentioned (probably in IRC) configuring
>>>>> equalizer parameters as one possible use case, and my reply was that
>>>>> equalizer instances don't necessarily map directly to module instances,
>>>>> so "pa_context_send_module_command()" isn't really appropriate; at
>>>>> least the function name needs to be changed. D-Bus uses "objects" as
>>>>> message targets, and each object has a unique path string as an
>>>>> identifier. I think a similar approach would be good here: messages
>>>>> should just carry an arbitrary string as the recipient indentifier, it
>>>>> doesn't matter whether the recipient is a module or something else.
>>>> I see the point. We could let any entity register a command handler.
>>>> Any idea about the naming scheme?
>>> My suggestion:
>>>
>>> pa_context_send_message(context, recipient, message_name, message_params, cb, userdata);
>> Looks good but does not really answer my intended question. Let me
>> ask differently: How should the recipients be named?
> If the recipient is an object as I originally intended, then the object
> name could be used (e.g. sink name). If the object doesn't currently
> have a name (e.g. sink inputs), then I think a name should be added. I
> think it would be good to have names on all objects, as long as they
> are more informative than just the object index.
>
> If the recipient identifies just the implementor of a collection of
> functions like you suggest, then the name should be something suitable
> for that. For example, the loopback API could be named "loopback". I
> don't want to use "module-loopback" as the name, because the API may
> not be forever provided by a module called that. Loopbacks could very
> well end up being included in the core, or loopbacks could be provided
> by a library that is used by other modules to create loopbacks without
> needing to load any module-loopback instances. Loading and unloading
> modules and then controlling those modules from policy modules is more
> cumbersome than using a direct C API like pa_loopback_new() and
> pa_loopback_free() and pa_loopback_set_foo().

I do not really see the difference in that case. The "implementor
of a collection of functions" is an object. If we allow any object
to register a message handler, a module is as valid as an object
as any other entity. I was just saying that I do not see the need
to chain message handlers if each object registers its handler
under a unique name. A handler for object A may contain functions
that operate on object B if these functions are specific for object
A. If they are not specific for object A, they belong to the handler
of object B in the first place and should not be implemented together
with object A.
Regarding your naming - I don't see why we should not use module-xyz
as a name. As said above, a module is a valid entity and as such entitled
to register its own handler. When the functionality of a module is moved
to the core or somewhere else, there will be a new valid object which on
the long run replaces the old.


>
>>> I now noticed that in your patch the message name and parameters are
>>> not separate. I think it's good to be able to register separate
>>> handlers for separate messages. Multiple modules might want to register
>>> different message handlers for a single recipient object. For example,
>>> the alsa modules could register alsa specific sink messages and an
>>> equalizer module could register equalizer messages for the same sink.

This is a good example for what I mean. You are saying "alsa specific
sink messages" and "equalizer messages for the same sink".
This shows for me, that the messages do not belong to the sink
object itself, but to the underlying alsa backend or the equalizer. They
just operate on the same sink.

>> You lost me here. For me there is one handler per recipient. In your
>> example above, the recipients would be the modules, even though
>> they work on the same sink. And if a module uses some kind of plug-in,
>> the plug-in could also register its own handler. A module could
>> even register multiple handlers with different recipient (object) names.
>>
>> I do not see the necessity to chain handlers for the same object. In
>> my opinion this would also raise the complexity level and my intention
>> was to keep it as simple as possible.
> Well, do we want an object oriented interface, where operations are
> performed on devices, streams, loopback instances, etc. or do we want a
> function oriented interface, where there are just plain functions
> grouped by API providers (where there is often, but not always, a 1:1
> relationship between modules and API providers)? Object oriented
> programming is the "mainstream" approach, and D-Bus has that design
> too, but functional programming is popular too, so I don't think
> there's any clear "right answer".

See above.

>
> When access control is added, the access control policy will be
> interested in the function/method name, and often it has to check the
> operand object's attributes too. I imagine it would be easier for the
> access policy if the messages contained the object name without needing
> to parse it from the function parameters, but probably the difference
> isn't huge if the operand objects are always the first function
> parameters, so I don't have very strong opinion.
>
> For example, let's say that there's a "set_latency" function that
> operates on a loopback instance. The access control policy will get
> either the following information:
>
> object_name: "foo_loopback"
> method_name: "set_latency"
> parameters: "1234"
>
> or
>
> api: "loopback"
> function_name: "set_latency"
> parameters: "foo_loopback 1234"
>
> The first case doesn't involve any parsing work to get the object name,
> the second case does.
>
As said above, my assumption is, that each handler is registered under
a unique (object) name that fully specifies the recipient of the message,
so a call would look like your first example.
The only problem I see here is to find an appropriate naming convention
that is easy to use and accounts for multiple instances of the same
object (multiple equalizers or loopbacks for example). Maybe some path-like
approach would work:
/core/object_name
/sinks/{name|index}
/modules/{name|index}[/object_within_module]
...

If you are OK with that approach or can provide a better idea, I could 
try and
code a new patch. I believe our views do not differ too much, it is just 
a matter
of perspective.