On Mon, 04 Sep 2023 13:26, Philippe Mathieu-Daudé <phi...@linaro.org> wrote:
/*
- * Handles VIRTIO_SND_R_PCM_RELEASE. Releases the buffer resources allocated to
- * a stream.
+ * Returns the number of I/O messages that are being processed.
+ *
+ * @stream: VirtIOSoundPCMStream
+ */
+static size_t virtio_snd_pcm_get_pending_io_msgs(VirtIOSoundPCMStream *stream)
+{
+ VirtIOSoundPCMBlock *block;
+ VirtIOSoundPCMBlock *next;
+ size_t size = 0;
+
+ WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
+ QSIMPLEQ_FOREACH_SAFE(block, &stream->queue, entry, next) {
+ size += 1;
Can you add a comment explaining this magic size?
It's not magic, it's simply how many messages there are as explained in
the function doc comment. This was previously bytes hence `size`. I will
change the variable name to `count`.
+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 };
virtio_snd_pcm_status has multiple fields, so better zero-initialize
all of them with '{ }'.
I don't understand why, virtio_snd_pcm_status has two int fields hence {
0 } zero-initializes all of them.
+/*
+ * 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.
+ 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);
Is it OK to ignore AUD_write() returning < block->size?
If so, can you add a comment please?
This is a flush event with a timeout so it should complete asap. As
mentioned in another reply it might be better to copy the data to a
buffer in order not to lose any audio bytes.
Thank you for the feedback,
Manos