Implement VirtIO sound device capture #115

Cuda-Chen · 2025-11-17T06:48:30Z

Summary by cubic

Implements VirtIO sound capture (RX) with PortAudio input, RX virtqueue handling, and circular buffering so input audio can be delivered to the guest. Also exposes separate input/output streams with accurate device info.

New Features
- Added RX virtqueue handlers (normal/flush) with default flush stream_id = 1.
- Opened PortAudio input stream and RX callback to capture frames and enqueue to the driver.
- Implemented circular intermediate buffer (buf_sz, buf_idx) and RX enqueue/dequeue helpers.
- Exposed two streams (output + input), mono S16, with per-stream jack/pcm/chmap info.
- Added RX worker thread, mutex/cond vars, and VSND_QUEUE_RX notifications.
- Applied direction-aware flush in pcm_release and set RX response length/status.
- Added CI sound tests for playback and capture and wired them into GitHub Actions.

^{Written for commit fd9ed70. Summary will update automatically on new commits.}

jserv

Rebase to avoid unnecessary merge commits.

jserv · 2025-12-17T03:56:30Z

virtio-snd.c

+static void __virtio_snd_rx_frame_enqueue(void *payload,
+                                          uint32_t n,
+                                          uint32_t stream_id)


__virtio_snd_rx_frame_enqueue() never increments buf_ev_notity or signals the condition variable, but __virtio_snd_rx_frame_dequeue() waits on it:

// In __virtio_snd_rx_frame_dequeue: while (props->lock.buf_ev_notity < 1) pthread_cond_wait(&props->lock.readable, ...); // BLOCKS FOREVER

The TX path correctly does:

props->lock.buf_ev_notity++; pthread_cond_signal(&props->lock.readable);

Fix: Add after list_push() in __virtio_snd_rx_frame_enqueue:

props->lock.buf_ev_notity++; pthread_cond_signal(&props->lock.readable);

The buf_ev_notity will be increased and then signals the update of CV after consuming all the virtq elements when calling virtio_snd_rx_desc_normal_handler():

// In VSND_GEN_RX_QUEUE_HANDLER macro virtio_snd_prop_t *props = &vsnd_props[stream_id]; props->lock.buf_ev_notity--; pthread_cond_signal(&props->lock.writable);

The virtio_snd_tx_desc_normal_handler() also has the same design.

The reason is that the driver should populate a buffer size of PCM frames (except the end of stream) as mentioned by VirtIO standard:

5.14.6.8.2.2 Driver Requirements: Input Stream

The driver SHOULD populate the rx queue with period_bytes sized empty buffers before starting the stream.

The driver MUST NOT place device-readable buffers into the rx queue.

jserv · 2025-12-17T03:57:20Z

virtio-snd.c

+static void __virtio_snd_rx_frame_dequeue(void *out,
+                                          uint32_t n,
+                                          uint32_t stream_id)


__virtio_snd_rx_frame_dequeue() does list_del(&node->q) but never free(node):

if (node->pos >= node->len) list_del(&node->q); // Missing: free(node);

virtio-snd.c

jserv · 2025-12-17T03:59:22Z

Debug Printf Pollution
Multiple fprintf(stderr, ...) debug statements throughout the RX path should be removed before merge.

Makefile

virtio-snd.c

jserv · 2025-12-17T06:51:03Z

Can we perform loopback tests with virtio sound device?

Cuda-Chen · 2025-12-17T07:09:51Z

Can we perform loopback tests with virtio sound device?

You mean aloop or a device loopback testing command like arecord -C | aplay -?

jserv · 2025-12-17T07:11:38Z

You mean aloop of a device loopback testing command like arecord -C | aplay -?

The headless verification is crucial for CI/CD integration.

.ci/test-sound.sh

Implement VirtIO sound device capture, with adding notice that the capture usually doesn't work for the reason of 'semu' emulation part.

cubic-dev-ai

3 issues found across 4 files

Prompt for AI agents (all 3 issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="virtio-snd.c">

<violation number="1" location="virtio-snd.c:373">
P2: The global `rx_ev_start` has a data race - it&#39;s read in the RX thread (in `__virtio_snd_rx_frame_dequeue`) while being written from the control path without synchronization. Consider using atomic operations or protecting access with `props-&gt;lock.lock` mutex.</violation>
</file>

<file name="README.md">

<violation number="1" location="README.md:17">
P3: Grammar error: &quot;until `semu` being rebooted&quot; should be &quot;until `semu` is rebooted&quot;.</violation>

<violation number="2" location="README.md:21">
P3: Grammar error: subject-verb agreement. &quot;ALSA usually get stuck&quot; should be &quot;ALSA usually gets stuck&quot;.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

cubic-dev-ai · 2025-12-19T10:02:39Z

virtio-snd.c

+static int rx_ev_notify;
+
+// FIXME: set these variables into each capture stream;s structure
+static int rx_ev_start;


P2: The global rx_ev_start has a data race - it's read in the RX thread (in __virtio_snd_rx_frame_dequeue) while being written from the control path without synchronization. Consider using atomic operations or protecting access with props->lock.lock mutex.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At virtio-snd.c, line 373: <comment>The global `rx_ev_start` has a data race - it's read in the RX thread (in `__virtio_snd_rx_frame_dequeue`) while being written from the control path without synchronization. Consider using atomic operations or protecting access with `props->lock.lock` mutex.</comment> <file context> @@ -316,9 +362,15 @@ static virtio_snd_prop_t vsnd_props[VSND_DEV_CNT_MAX] = { +static int rx_ev_notify; + +// FIXME: set these variables into each capture stream;s structure +static int rx_ev_start; /* vsnd virtq callback type */ </file context>

README.md

cubic-dev-ai · 2025-12-19T10:02:39Z

README.md

-          the program cannot write the PCM frames into guest OS ALSA stack.
+    - virtio-snd uses [PortAudio](https://github.com/PortAudio/portaudio) for sound playback and capture on the host with one limitation:
+        - As a confirmed issue that `semu` cannot send/receive PCM frames in time, causing
+          the ALSA stack stuck in XRUN state until `semu` being rebooted.


P3: Grammar error: "until semu being rebooted" should be "until semu is rebooted".

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At README.md, line 17: <comment>Grammar error: "until `semu` being rebooted" should be "until `semu` is rebooted".</comment> <file context> @@ -12,12 +12,13 @@ A minimalist RISC-V system emulator capable of running Linux the kernel and corr - the program cannot write the PCM frames into guest OS ALSA stack. + - virtio-snd uses [PortAudio](https://github.com/PortAudio/portaudio) for sound playback and capture on the host with one limitation: + - As a confirmed issue that `semu` cannot send/receive PCM frames in time, causing + the ALSA stack stuck in XRUN state until `semu` being rebooted. + - For playback, you can try to adjust the buffer size to more than four times of period size. - For instance, the following buffer/period size settings on `aplay` has been tested </file context>

Suggested change

the ALSA stack stuck in XRUN state until `semu` being rebooted.

the ALSA stack to get stuck in XRUN state until `semu` is rebooted.

✅ Addressed in 60ab79a

cubic-dev-ai

5 issues found across 4 files

Prompt for AI agents (all 5 issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="README.md">

<violation number="1" location="README.md:17">
P3: Grammar issue: &#39;stuck&#39; should be &#39;to get stuck&#39; and &#39;being rebooted&#39; should be &#39;is rebooted&#39;.</violation>

<violation number="2" location="README.md:21">
P3: Subject-verb agreement error: &quot;ALSA usually get stuck&quot; should be &quot;ALSA usually gets stuck&quot;.</violation>
</file>

<file name=".github/workflows/main.yml">

<violation number="1" location=".github/workflows/main.yml:72">
P1: The sound test runs unconditionally for all matrix variants, including when `matrix.dependency == &#39;none&#39;` where no sound library is installed. This will likely cause the test to fail. Consider adding a condition similar to the install step.</violation>
</file>

<file name="virtio-snd.c">

<violation number="1" location="virtio-snd.c:857">
P1: Potential double-free: `props-&gt;intermediate` is freed in error path but not set to NULL. If `virtio_snd_read_pcm_release` is called later (state is already PREPARE), it will free the same pointer again.</violation>

<violation number="2" location="virtio-snd.c:1225">
P1: Missing NULL checks after malloc. If memory allocation fails, dereferencing `node` or `node-&gt;addr` will cause a crash.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

README.md

cubic-dev-ai · 2025-12-19T10:12:16Z

.github/workflows/main.yml

      run: sudo .ci/test-netdev.sh
      shell: bash
      timeout-minutes: 10
+    - name: sound test


P1: The sound test runs unconditionally for all matrix variants, including when matrix.dependency == 'none' where no sound library is installed. This will likely cause the test to fail. Consider adding a condition similar to the install step.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At .github/workflows/main.yml, line 72: <comment>The sound test runs unconditionally for all matrix variants, including when `matrix.dependency == 'none'` where no sound library is installed. This will likely cause the test to fail. Consider adding a condition similar to the install step.</comment> <file context> @@ -69,6 +69,10 @@ jobs: run: sudo .ci/test-netdev.sh shell: bash timeout-minutes: 10 + - name: sound test + run: .ci/test-sound.sh + shell: bash </file context>

Suggested change

- name: sound test

- name: sound test

if: matrix.dependency != 'none'

virtio-snd.c

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="virtio-snd.c">

<violation number="1" location="virtio-snd.c:1239">
P1: When malloc fails, the code still increments `buf_ev_notity` and signals the condition variable, falsely indicating data is available. This will cause the consumer to wake up expecting data that was never enqueued, leading to incorrect behavior. The notification logic should only execute on successful enqueue.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

virtio-snd.c

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="README.md">

<violation number="1" location="README.md:17">
P3: Grammar error: &quot;stucked&quot; is not a valid word. Use &quot;stuck&quot; instead (it&#39;s already the past participle of &quot;stick&quot;).</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

README.md

jserv

Refine git commit messages with explicit limitations.

cubic-dev-ai

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name=".ci/test-sound.sh">

<violation number="1" location=".ci/test-sound.sh:21">
P1: The expect pattern `${PLAYBACK_EXPECTED}` contains spaces but is not quoted. In Tcl/Expect, unquoted strings with spaces are parsed as separate arguments, so only `Playing` would be matched as the pattern. This will cause the test to fail or behave incorrectly. The pattern should be wrapped in double quotes.</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

.ci/test-sound.sh

jserv requested changes Nov 17, 2025

View reviewed changes

This comment was marked as resolved.

Sign in to view

Cuda-Chen force-pushed the add-virtio-snd-rx branch 2 times, most recently from 54a45b5 to 05b22f9 Compare November 17, 2025 10:31

Cuda-Chen force-pushed the add-virtio-snd-rx branch from 2db638a to 781ef0a Compare December 17, 2025 03:40

jserv reviewed Dec 17, 2025

View reviewed changes

virtio-snd.c Outdated Show resolved Hide resolved

jserv reviewed Dec 17, 2025

View reviewed changes

virtio-snd.c Outdated Show resolved Hide resolved

Cuda-Chen force-pushed the add-virtio-snd-rx branch 2 times, most recently from be341b2 to 1a14913 Compare December 17, 2025 06:16

jserv reviewed Dec 17, 2025

View reviewed changes

Makefile Outdated Show resolved Hide resolved

jserv reviewed Dec 17, 2025

View reviewed changes

virtio-snd.c Outdated Show resolved Hide resolved

Cuda-Chen force-pushed the add-virtio-snd-rx branch 3 times, most recently from fd4f538 to a2348ec Compare December 18, 2025 11:02

Cuda-Chen marked this pull request as ready for review December 18, 2025 11:46

Cuda-Chen requested a review from jserv December 18, 2025 11:46

This comment was marked as resolved.

Sign in to view

jserv reviewed Dec 18, 2025

View reviewed changes

.ci/test-sound.sh Outdated Show resolved Hide resolved

Implement VirtIO sound device capture

41fe790

Implement VirtIO sound device capture, with adding notice that the capture usually doesn't work for the reason of 'semu' emulation part.

Cuda-Chen force-pushed the add-virtio-snd-rx branch from 30d1ccd to 41fe790 Compare December 18, 2025 13:04

cubic-dev-ai bot reviewed Dec 19, 2025

View reviewed changes

sysprog21 deleted a comment from cubic-dev-ai bot Dec 19, 2025

cubic-dev-ai bot reviewed Dec 19, 2025

View reviewed changes

Apply suggestions

8b5d0cb

cubic-dev-ai bot reviewed Dec 21, 2025

View reviewed changes

virtio-snd.c Outdated Show resolved Hide resolved

Apply suggestion

60ab79a

cubic-dev-ai bot reviewed Dec 21, 2025

View reviewed changes

README.md Outdated Show resolved Hide resolved

Cuda-Chen added 2 commits December 21, 2025 12:24

Apply suggestion

5bca780

Apply suggestion

a8c5d37

jserv requested changes Dec 21, 2025

View reviewed changes

Test playback

7d09cf1

cubic-dev-ai bot reviewed Dec 21, 2025

View reviewed changes

.ci/test-sound.sh Outdated Show resolved Hide resolved

Cuda-Chen added 2 commits December 21, 2025 21:04

Test playback

46bbb67

Add test status

fd9ed70

sysprog21 deleted a comment from cubic-dev-ai bot Dec 22, 2025

	the ALSA stack stuck in XRUN state until `semu` being rebooted.
	the ALSA stack to get stuck in XRUN state until `semu` is rebooted.

	- name: sound test
	- name: sound test
	if: matrix.dependency != 'none'

Implement VirtIO sound device capture #115

Are you sure you want to change the base?

Implement VirtIO sound device capture #115

Conversation

Cuda-Chen commented Nov 17, 2025 • edited by cubic-dev-ai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

jserv Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Cuda-Chen Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jserv Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jserv commented Dec 17, 2025

Uh oh!

Uh oh!

Uh oh!

jserv commented Dec 17, 2025

Uh oh!

Cuda-Chen commented Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jserv commented Dec 17, 2025

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai bot Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jserv left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Cuda-Chen commented Nov 17, 2025 •

edited by cubic-dev-ai bot

Loading

Cuda-Chen Dec 17, 2025 •

edited

Loading

Cuda-Chen commented Dec 17, 2025 •

edited

Loading

cubic-dev-ai bot Dec 19, 2025 •

edited

Loading

cubic-dev-ai bot Dec 19, 2025 •

edited

Loading

cubic-dev-ai bot Dec 19, 2025 •

edited

Loading