Re: [PATCH v7] audio/jack: add JACK client audiodev

[Markus Armbruster]

I sincerely apologize for the excessive delay.

Geoffrey McRae <address@hidden> writes:

> On 2020-05-08 16:34, Markus Armbruster wrote:
>> Geoffrey McRae <address@hidden> writes:
>>
>>> On 2020-05-06 16:06, Markus Armbruster wrote:
>>>> You neglected to cc: the audio maintainer.  Doing that for you now.
>>>> You
>>>> can use scripts/get_maintainer.pl to find maintainers.
>>>
>>> Thanks, I was unaware.
>>
>> Happy to help :)
>>
>>>>
>>>> Find my QAPI schema review inline.
>>>>
>>>
>>> Ditto
>>>
>>>> Geoffrey McRae <address@hidden> writes:
>>>>
>>>>> This commit adds a new audiodev backend to allow QEMU to use JACK as
>>>>> both an audio sink and source.
>>>>>
>>>>> Signed-off-by: Geoffrey McRae <address@hidden>
>>>>> ---
>>>>>  audio/Makefile.objs    |   5 +
>>>>>  audio/audio.c          |   1 +
>>>>>  audio/audio_template.h |   2 +
>>>>>  audio/jackaudio.c      | 677
>>>>> +++++++++++++++++++++++++++++++++++++++++
>>>>>  configure              |  17 ++
>>>>>  qapi/audio.json        |  56 +++-
>>>>>  6 files changed, 756 insertions(+), 2 deletions(-)
>>>>>  create mode 100644 audio/jackaudio.c
>> [...]
>>>>> diff --git a/qapi/audio.json b/qapi/audio.json
>>>>> index c31251f45b..bdb0552d15 100644
>>>>> --- a/qapi/audio.json
>>>>> +++ b/qapi/audio.json
>>>>> @@ -152,6 +152,55 @@
>>>>>      '*out':     'AudiodevPerDirectionOptions',
>>>>>      '*latency': 'uint32' } }
>>>>>
>>>>> +##
>>>>> +# @AudiodevJackPerDirectionOptions:
>>>>> +#
>>>>> +# Options of the JACK backend that are used for both playback and
>>>>> +# recording.
>>>>> +#
>>>>> +# @server-name: select from among several possible concurrent
>>>>> server instances.
>>>>> +# If unspecified, use "default" unless $JACK_DEFAULT_SERVER is
>>>>> defined in the
>>>>> +# process environment.
>>>>
>>>> Suggest something like
>>>>
>>>>    # (default environment variable $JACK_DEFAULT_SERVER if set, else
>>>>    # "default").
>>
>> Uh, needs a colon after "(default" for clarity.
>>
>>>>
>>>
>>> I'd be happy to change this, however, this terminology reflects the
>>> JACK documentation which people already using JACK will be familiar
>>> with.
>>
>> There's a tension between your (sensible) desire to stay close to JACK
>> documentation, and our need for reasonably consistent QMP reference
>> documentation.
>>
>> Where we're talking about genuine JACK stuff, staying close to JACK
>> documentation feels more important.
>>
>> Where we're doing general QMP things, doing them the established QMP
>> way
>> feels more important.
>>
>> Phrasing "this is the optional member's default value" is QMP.  We
>> do it
>> all over the place like (default: WHATEVER).  Please stick to this
>> conventional QMP format.
>>
>> The WHATEVER is JACK stuff.  Feel free to improve on my suggested
>> phrasing there.
>>
>
> Not a problem, I will reword this as requested.
>
>>>>> +#
>>>>> +# @client-name: the client name to use. The server will modify
>>>>> this name to
>>>>> +# create a unique variant, if needed unless @exact_name is true.
>>>>
>>>> Do we really need this much magic?
>>>>
>>>> What would we lose with just @client-name?  If it's present, use
>>>> it as
>>>> is (no magic), else make up a client name.
>>>
>>> There is no magic on our part, this is how the JACK server works and
>>> is the default.
>>
>> Aha!
>>
>> Please bear with my ignorant questions...
>>
>> QEMU doesn'r have to expose everything the JACK library and server
>> provide, only the stuff that is useful to QEMU's users.
>>
>> There seem to be several variations:
>>
>> * @client-name absent, @exact_name false
>>
>>   JACK picks a name.
>>
>> * @client-name absent, @exact_name true
>>
>>   Error?  @exact_name silently ignored?
>>
>
> The @client-name is set to the VM's name as the default if absent.
>
>> * @client-name present, @exact_name false
>>
>>   JACK picks a name, using @client-name as inspiration somehow.
>>
>
> JACK picks a name only if there is a name clash, a duplicate,
> otherwise, it honours the requested name.
>
>> * @client-name present, @exact_name true
>>
>>   JACK uses @client-name if possible, else error.
>>
>> Use cases for the three variations that make sense?
>
> JACK is extremely configurable and is used in both general-purpose
> audio through to professional audio mastering and distribution in
> conferences and stadiums. As such all these use cases are completely
> valid and need to be present otherwise it limits the usefulness of
> this audio dev. The plumbing of jack is entirely left up to the user
> and as such there are countless different possible configurations and
> use cases.
>
>>
>> When letting JACK pick a name, how do users learn the name?
>
> We print it out if jack did this:
>
>     if (status & JackNameNotUnique) {
>         dolog("JACK unique name assigned %s\n",
>           jack_get_client_name(c->client));
>     }

Having to extract the name out of a log file is not machine-friendly.
Should we have some query-audio command?

> The user can also learn the name through various methods such as
> running `jack_lsp` or using `qjackctl`, or any number of potential
> jack plugboards such as `Carla`, etc.

How would I identify which of the possibly several names belongs to the
thing I set up via QMP?

>>>>> +#
>>>>> +# @connect-ports: if set, a regular expression of port name(s) to
>>>>> match to auto
>>>>> +# connect to at startup.
>>>>
>>>> Pardon my ignorance... where do the port names being matched come
>>>> from?
>>>
>>> Other JACK client ports. QEMU become a JACK client which has audio
>>> ports and can be patched into the system sound device, or any number
>>> of other ports for DSP.
>>
>> I think a slightly less terse doc comment would be a lot easier to
>> understand.  At least "s/ of port name(s)/of JACK client port name(s)/.
>> Also, "to auto connect to" is quite vague on what gets connected to
>> these ports.  Can we do a bit better?
>>
>
> The wording may need correcting however as jack is so flexible, what
> it gets connected to can not be stated. It could be anything depending
> on what clients/plugins the user are using already, the jack allows
> for a very complex and complete audio system. I have attached a
> screenshot of my JACK graph for a simple example of what this looks
> like.

Okay, I understand specifying here what JACK does makes no sense.  But
let's try to phrase what we can specify as clearly as possible.

[...]