Skip to content

Switch to PortAudio #76

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Cuda-Chen
Copy link
Collaborator

@Cuda-Chen Cuda-Chen commented Feb 23, 2025

Switch to PortAudio for cross-platform and cross backend compatibility.

Yet, there are two known limitations for current sound playback implementation:

  1. The sound may produce stutter effect.
  2. You cannot play sound twice or the device will crash thus
    whole emulator crashes.

Testing

Preparation

  1. Run make build-image to build Linux Kernel image and root filesystem.

Test Cases

play white noise

test procedures
  1. Execute speaker-test in emulator shell.
  2. Check the host speaker.
expected results
  1. A white noise should be played by host speaker in the first round while speaker-test is executing.
  2. The emulator should crash after the second round of playing white noise.

play sequence sound

test procedures
  1. Execute aplay /usr/share/sounds/alsa/Front_Center.wav in emulator.
  2. Check the host speaker.
  3. Execute aplay /usr/share/sounds/alsa/Front_Center.wav in emulator.
expected results
  1. A sound with "Front Center" speaking with stutter effect should be played in step 2.
  2. The emulator should crash after step 1.

Summary by Bito

This pull request integrates PortAudio to enhance cross-platform audio playback, replacing the previous CNFA submodule. It updates the Makefile for better build configurations, addresses issues like sound loss and system crashes, and improves PCM stream handling and compatibility, while also updating documentation.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 2

@jserv
Copy link
Collaborator

jserv commented Feb 23, 2025

You should explain the motivation and benefit for the migration of PortAudio.

@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch 4 times, most recently from 6d42303 to 6e00d27 Compare February 24, 2025 09:33
@jserv
Copy link
Collaborator

jserv commented Feb 24, 2025

Build failure:

  LD	semu
/usr/bin/ld: cannot find -lpulse: No such file or directory
collect2: error: ld returned 1 exit status

@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Feb 24, 2025
@Cuda-Chen
Copy link
Collaborator Author

Build failure:

  LD	semu
/usr/bin/ld: cannot find -lpulse: No such file or directory
collect2: error: ld returned 1 exit status

Thanks for the reminder!
I am still working on this issue.

@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from f26f174 to e331ed7 Compare March 1, 2025 08:47
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from e331ed7 to e7184f0 Compare March 1, 2025 11:56
@jserv jserv changed the title [WIP] Switch to PortAudio Switch to PortAudio Mar 1, 2025
@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from e7184f0 to a73d91d Compare March 1, 2025 12:14
@Cuda-Chen Cuda-Chen marked this pull request as ready for review March 1, 2025 12:14
@jserv jserv marked this pull request as draft March 1, 2025 14:12
@sysprog21 sysprog21 deleted a comment from bito-code-review bot Mar 1, 2025
@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch 2 times, most recently from c270822 to 856807d Compare March 4, 2025 11:43
@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from 04ea592 to 9023def Compare March 26, 2025 04:03
@Cuda-Chen Cuda-Chen requested a review from jserv March 26, 2025 04:32
@jserv
Copy link
Collaborator

jserv commented Mar 26, 2025

Prepare the instructions l for testing the PortAudio-based virtio-snd backend, including preparation of the Linux kernel image and root filesystem. The goal is to deliver the preliminary work that can be shared with and potentially expanded by additional contributors.

@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from d989a67 to 18178b5 Compare March 27, 2025 00:32
@Cuda-Chen
Copy link
Collaborator Author

Prepare the instructions l for testing the PortAudio-based virtio-snd backend, including preparation of the Linux kernel image and root filesystem. The goal is to deliver the preliminary work that can be shared with and potentially expanded by additional contributors.

I have added the test procedures in "Testing" section located in the first comment.

@Cuda-Chen Cuda-Chen requested a review from jserv March 27, 2025 00:38
@Cuda-Chen Cuda-Chen marked this pull request as ready for review March 28, 2025 13:19
Switch to PortAudio for cross-platform and cross backend compatibility.

Yet, there are two known limitations for current sound playback implementation:
1. The sound may produce stutter effect.
2. You cannot play sound twice or the device will crash thus
whole emulator crashes.
@Cuda-Chen Cuda-Chen force-pushed the switch-to-portaudio branch from 18178b5 to 8451a5c Compare April 1, 2025 11:56
@jserv
Copy link
Collaborator

jserv commented Apr 2, 2025

You cannot play sound twice or the device will crash thusnwhole emulator crashes.

Have you resolved this issue?

@Cuda-Chen
Copy link
Collaborator Author

You cannot play sound twice or the device will crash thusnwhole emulator crashes.

Have you resolved this issue?

I haven't resolve this issue, yet.

Should we resolve the issue in this PR?

@jserv
Copy link
Collaborator

jserv commented Apr 2, 2025

Should we resolve the issue in this PR?

Yes, let's determine the root cause and resolve the issue causing crash.

Get re-sending frames effect with the following errors:
[   35.875955] virtio_snd virtio3: virtsnd-tx:id 0 is not a head!
[   36.892059] virtio_snd virtio3: SID 0: failed to flush I/O queue
#define CNFA_IMPLEMENTATION
#include "CNFA_sf.h"

#include "portaudio.h"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use <portaudio.h> instead.


#define VSND_GEN_TX_QUEUE_HANDLER(NAME_SUFFIX, WRITE) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move the common part into an inline function?

@jserv
Copy link
Collaborator

jserv commented Apr 17, 2025

@Cuda-Chen, Why did you enable Enable hrtimer in ALSA configurations?

@Cuda-Chen
Copy link
Collaborator Author

@Cuda-Chen, Why did you enable Enable hrtimer in ALSA configurations?

To make sure whether the hrtimer resolve the stuttering and sending invalid stream ID issues.

@Cuda-Chen
Copy link
Collaborator Author

Hi @jserv , I would like to summarize the current progress:

  1. For stuttering effect, the root case remains unknown. It seems that the sound driver just sends the sound with stuttering effect.
  2. For crash issue, it is confirmed that the sound driver sends the PCM frames with invalid stream ID. I need to come up with a solution (the current attempt needs more work). As a consequence, you may mark this PR into a draft PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants