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 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
