Re: [virtio-dev] virtio-snd comments/questions

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

Hello,

On Fri, Nov 03, 2023 at 07:05:30PM +0000, Radu Ocica wrote:
> Hi Matias,
>
>
> I will answer to your comments:
>
>
> > I agree that the STOP command could be better explain it. In our
> implementation[1], we implemented as your last point defines it, i.e.,
> stop host's pcm stream and keep messages in tx/rx. The STOP control
> command is marked as completed immediately. The virtio spec does not
> indicate that STOP requires to mark pending io messages as completed
> thus we do not do that. A note about this in the spec would help.
>
>
> In our implementation we made the assumption that STOP is meant to be implemented as pause, thus if the host stream supports the pause operation we pause it and keep io messages in tx/rx queue; the host stream is stopped (via the drop operation) and io messages are marked in this case as completed and returned to the guest driver only when receiving the RELEASE control request.
>
> However, if the host stream does not support pause the only option left is to perform a drop operation on the host stream and in this case all pending messages in the tx/rx queue need to be returned to the guest as completed, because at the next start possibly all messages will be needed to prime the host stream (in the case of playback). A host stream prepare will also be needed to be issued internally by the device if a START is issued by the guest right after the STOP. And there is some uncertainty whether a guest driver will send new io messages before or after the START control request.
>

Could you elaborate on this? Which audio backend would not support
PAUSE? I am not very familiar with audio engines.


> I think the difficulty of describing a clear behavior for STOP stems from attempting a minimal spec, with a minimal number of control requests. If dedicated STOP vs PAUSE requests were used, it would be more straightforward to specify an unambiguous behavior for each.
>
>
> > Checking the Linux kernel driver, the STOP cmd is issued when getting
> SNDRV_PCM_TRIGGER_PAUSE_PUSH. Documentation does not indicate when this
> msg is sent though. The RELEASE cmd is issued only during
> virtsnd_pcm_sync_stop(). In this sense, I would guess that STOP behaves
> like a PAUSE whereas RELEASE behaves like a STOP and dropping all
> pending IO messages. The spec left for the implementation if device
> would still playback those buffer or just drop them. I think you may
> find interesting the discussion we started at this thread[2] regarding
> the RELEASE control cmd semantics.
>
>
> We have made the call to never have the device drain the host pcm stream. If the host pcm stream supports pause, then STOP pauses it and RELEASE performs a drop and returns immediately all io messages as completed to the guest driver. If the host pcm stream does not support pause, then STOP results in a drop operation and return of all io messages as completed to the guest driver, and the RELEASE is a no-op. A drain like behavior for playback can be obtained if the guest driver stops writing new data and waits to underrun, time at which it sends STOP and RELEASE. This seems to work fine with the existing reference linux/android guest driver for success path handling. There are questions regarding failure path handling (failure of control or io requests) and what variability can/should be expected in terms of guest driver behavior.
>
>
> > Right, we discussed about this issue at [3]. I've recently proposed the
> following patch to fix that: '[PATCH v3] ALSA: virtio: use ack callback'
>
>
> This looks promising. I found it already at https://github.com/torvalds/linux/blob/master/sound/virtio/ and will give it a try. Is there a repository for this driver code where both this fix and the audio control support is available?
>
>

What do you mean with audio control support? Are you talking about the
device? The device implementation is at
https://github.com/rust-vmm/vhost-device/tree/main/staging.


Matias