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

[Volker Rümelin]

Am 04.09.23 um 23:34 schrieb Volker Rümelin:
> Am 04.09.23 um 12:34 schrieb Manos Pitsidianakis:
> > On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <philmd@linaro.org> 
> > wrote:
> >
> > > > +/*
> > > > + * AUD_* output callback.
> > > > + *
> > > > + * @data: VirtIOSoundPCMStream stream
> > > > + * @available: number of bytes that can be written with AUD_write()
> > > > + */
> > > > +static void virtio_snd_pcm_out_cb(void *data, int available)
> > > > +{
> > > > +    VirtIOSoundPCMStream *stream = data;
> > > > +    VirtIOSoundPCMBlock *block;
> > > > +    VirtIOSoundPCMBlock *next;
> > > > +    size_t size;
> > > > +
> > > > +    WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
> > > > +        QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) {
> > > > +            for (;;) {
> > > > +                size = MIN(block->size, available);
> > > > +                size = AUD_write(stream->voice.out,
> > > > +                        block->data + block->offset,
> > > > +                        size);
> > >
> > > If AUD_write() returns 0, is this an infinite loop?
> >
> > Hm since we have available > 0 bytes this wouldn't theoretically 
> > happen, but I see there are code paths that return 0 on 
> > bugs/failures, I will add the check.
>
> Before QEMU 8.0.0 it was possible that AUD_write() couldn't write the 
> last audio frame and sometimes 'available' was just miscalculated. 
> Since commit e1e6a6fcc9 ("audio: handle leftover audio frame from 
> upsampling") AUD_write() writes all 'available' bytes.

I thought about this again. The error check is necessary.

> With best regards,
> Volker
>
> > > > +                block->size -= size;
> > > > +                block->offset += size;
> > > > +                if (!block->size) {
> > > > +                    virtqueue_push(block->vq,
> > > > +                            block->elem,
> > > > +                            sizeof(block->elem));
> > > > + virtio_notify(VIRTIO_DEVICE(stream->s),
> > > > +                            block->vq);
> > > > + QSIMPLEQ_REMOVE_HEAD(&stream->queue, entry);
> > > > +                    g_free(block);
> > > > +                    available -= size;
> > > > +                    break;
> > > > +                }
> > > > +
> > > > +                available -= size;
> > > > +                if (!available) {
> > > > +                    break;
> > > > +                }
> > > > +            }
> > > > +            if (!available) {
> > > > +                break;
> > > > +            }
> > > > +        }
> > > > +    }
> > > > +}
> > > > +
> > > > +/*
> > > > + * Flush all buffer data from this stream's queue into the 
> > > > driver's virtual
> > > > + * queue.
> > > > + *
> > > > + * @stream: VirtIOSoundPCMStream *stream
> > > > + */
> > > > +static void virtio_snd_pcm_flush(VirtIOSoundPCMStream *stream)
> > > > +{
> > > > +    VirtIOSoundPCMBlock *block;
> > > > +    VirtIOSoundPCMBlock *next;
> > > > +
> > > > +    WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
> > > > +        QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) {
> > > > +            AUD_write(stream->voice.out, block->data + 
> > > > block->offset, block->size);