Re: [virtio-comment] [PATCH] virtio-snd: add support for audio controls

[Cornelia Huck <cohuck@redhat.com>]

On Fri, Jun 17 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Tue, Jul 27, 2021 at 05:57:23PM +0200, Cornelia Huck wrote:
>> On Tue, Apr 20 2021, Anton Yakovlev <anton.yakovlev@opensynergy.com> wrote:
>> 
>> > This patch extends the virtio sound device specification by adding
>> > support for audio controls. Audio controls can be used to set the volume
>> > level, mute/unmute the audio signal, switch different modes/states of
>> > the virtual sound device, etc.
>> >
>> > Signed-off-by: Anton Yakovlev <anton.yakovlev@opensynergy.com>
>> > ---
>> >  conformance.tex  |   5 +
>> >  virtio-sound.tex | 374 ++++++++++++++++++++++++++++++++++++++++++++++-
>> >  2 files changed, 378 insertions(+), 1 deletion(-)
>> >
>> 
>> > @@ -50,6 +53,7 @@ \subsection{Device Configuration Layout}\label{sec:Device Types / Sound Device /
>> >      le32 jacks;
>> >      le32 streams;
>> >      le32 chmaps;
>> > +    le32 controls;
>> >  };
>> >  \end{lstlisting}
>> >  
>> > @@ -62,6 +66,9 @@ \subsection{Device Configuration Layout}\label{sec:Device Types / Sound Device /
>> >  PCM streams.
>> >  \item[\field{chmaps}] (driver-read-only) indicates a total number of all available
>> >  channel maps.
>> > +\item[\field{controls}] (driver-read-only) indicates a total number of all available
>> > +control elements if VIRTIO_SND_F_CTLS has been negotiated.
>> 
>> While the driver is not supposed to read this if the feature has not
>> been negotiated, do we also want to add a normative statement about what
>> the device is supposed to do in this case? Does the field always exist?
>
>
> Cornelia this is a general issue in the spec. We should be clearer
> that
> - drivers should be ready for config space to be shorter
> or longer than described here, as long as they do not
> need to access outside of it, it is ok.
> - negotiated/offered confusion

Yes, I remember we had some discussions about that in the meantime...

>
>
> I will revive my proposal where feature is negotiated even before
> FEATURES_OK
>
> I think this can stand as is for now.

Fine with me.