Re: [PATCH v7 10/12] virtio-sound: implement audio output (TX)

[Philippe Mathieu-Daudé]

Hi Emmanouil,

On 22/8/23 21:18, Emmanouil Pitsidianakis wrote:
> Handle output IO messages in the transmit (TX) virtqueue.
>
> It allocates a VirtIOSoundPCMBlock for each IO message and copies the
> data buffer to it. When the IO buffer is written to the host's sound
> card, the guest will be notified that it has been consumed.
>
> The lifetime of an IO message is:
>
> 1. Guest sends IO message to TX virtqueue.
> 2. QEMU adds it to the appropriate stream's IO buffer queue.
> 3. Sometime later, the host audio backend calls the output callback,
>     virtio_snd_pcm_out_cb(), which is defined with an AUD_open_out()
>     call. The callback gets an available number of bytes the backend can
>     receive. Then it writes data from the IO buffer queue to the backend.
>     If at any time a buffer is exhausted, it is returned to the guest as
>     completed.
> 4. If the guest releases the stream, its buffer queue is flushed by
>     attempting to write any leftover data to the audio backend and
>     releasing all IO messages back to the guest. This is how according to
>     the spec the guest knows the release was successful.
>
> Signed-off-by: Emmanouil Pitsidianakis <manos.pitsidianakis@linaro.org>
> ---
>   hw/virtio/trace-events         |   2 +
>   hw/virtio/virtio-snd.c         | 250 ++++++++++++++++++++++++++++++++-
>   include/hw/virtio/virtio-snd.h |  11 ++
>   3 files changed, 259 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 60ab62a80d..3b95e745c2 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -171,3 +171,5 @@ virtio_snd_handle_pcm_release(uint32_t stream) 
> "VIRTIO_SND_PCM_RELEASE called fo
>   virtio_snd_handle_code(uint32_t val, const char *code) "ctrl code msg val = 
> %"PRIu32" == %s"
>   virtio_snd_handle_chmap_info(void) "VIRTIO_SND_CHMAP_INFO called"
>   virtio_snd_handle_event(void) "event queue callback called"
> +virtio_snd_pcm_stream_flush(uint32_t stream) "flushing stream %"PRIu32
> +virtio_snd_handle_xfer(void) "tx/rx queue callback called"


> +/*
> + * The tx virtqueue handler. Makes the buffers available to their respective
> + * streams for consumption.
> + *
> + * @vdev: VirtIOSound device
> + * @vq: tx virtqueue
> + */
> +static void virtio_snd_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtIOSound *s = VIRTIO_SND(vdev);
> +    VirtIOSoundPCMStream *stream = NULL;
> +    VirtQueueElement *elem;
> +    size_t sz;
> +    virtio_snd_pcm_xfer hdr;
> +    virtio_snd_pcm_status resp = { 0 };
> +
> +    trace_virtio_snd_handle_xfer();

Maybe ...

> +    for (;;) {
> +        elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
> +        if (!elem) {
> +            break;
> +        }
> +        /* get the message hdr object */
> +        sz = iov_to_buf(elem->out_sg,
> +                        elem->out_num,
> +                        0,
> +                        &hdr,
> +                        sizeof(hdr));
> +        if (sz != sizeof(hdr)
> +            || hdr.stream_id >= s->snd_conf.streams
> +            || !s->pcm->streams[hdr.stream_id]) {
> +            goto tx_err;
> +        }

... it is more useful displaying the callback stream index here?

           trace_virtio_snd_handle_xfer(hdr.stream_id);

> +
> +        stream = s->pcm->streams[hdr.stream_id];
> +        if (stream->direction != VIRTIO_SND_D_OUTPUT) {
> +            goto tx_err;
> +        }
> +
> +        WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
> +            virtio_snd_pcm_read_write(stream,
> +                    vq,
> +                    elem,
> +                    hdr.stream_id == VIRTIO_SND_D_INPUT);
> +
> +            resp.status = VIRTIO_SND_S_OK;
> +            iov_from_buf(elem->in_sg,
> +                         elem->in_num,
> +                         0,
> +                         &resp,
> +                         sizeof(resp));
> +        }
> +        continue;
> +
> +tx_err:
> +        WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
> +            resp.status = VIRTIO_SND_S_BAD_MSG;
> +            iov_from_buf(elem->in_sg,
> +                         elem->in_num,
> +                         0,
> +                         &resp,
> +                         sizeof(resp));
> +        }
> +    }
> +
> +    /*
> +     * Notify vq about virtio_snd_pcm_status responses.
> +     * Buffer responses must be notified separately later.
> +     */
> +    virtio_notify(VIRTIO_DEVICE(s), vq);
> +}