Skip to content
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

Aplay resampler #750

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

Aplay resampler #750

wants to merge 9 commits into from

Conversation

borine
Copy link
Collaborator

@borine borine commented Feb 2, 2025

This is my attempt to address the topics previously discussed in PR #459

The io_worker_routine() function is already quite complex and can be difficult to read, so before adding more complexity, the first three commits simply factor out some of the code into new files.

The commit "aplay: improve overrun/underrun handling" uses a different approach, but addresses the same problem, as #459 and therefore if merged it closes #459

The libsamplerate resampler causes high CPU usage. Its not so bad on x86_64, but really very high on armhf. I have not tried with arm64. Having said that, I have tested on an old pi zero W (32bit armv6) with the "SINC_FASTEST" converter and got very stable audio with accurate delay reporting watching a 4 hour video stream. top showed bluealsa-aplay consuming 28% of the (single core) CPU but the pi continued to run smoothly. So anyway this is left as an option and not included in the build by default.

Do not allow audio frames to accumulate in the FIFO, and do not
block when writing to the ALSA PCM. By this means we prevent the
delay from increasing much beyond our chosen buffer sizes, and
ensure that if it becomes necessary to drop audio frames then we
always drop only the oldest ones.

Avoid underruns on the ALSA PCM as far as possible by inserting
silence when there are not enough frames available to maintain the
ALSA buffer fill level above the period size.

Use the BlueALSA PCM "running" property to detect when the
transport becomes idle. This avoids closing the ALSA PCM whenever
an unstable Bluetooth link causes a short break in the stream,
allowing **bluealsa-aplay** to play silence to keep the ALSA
stream running. When the transport becomes idle, drain the ALSA
PCM before closing it to ensure that all audio frames are played
out.
Copy link

codecov bot commented Feb 2, 2025

Codecov Report

Attention: Patch coverage is 80.42169% with 65 lines in your changes missing coverage. Please review.

Project coverage is 70.55%. Comparing base (4a661e8) to head (9aeb573).

Files with missing lines Patch % Lines
utils/aplay/alsa-pcm.c 72.64% 32 Missing ⚠️
utils/aplay/aplay.c 78.94% 20 Missing ⚠️
utils/aplay/alsa-mixer.c 81.81% 12 Missing ⚠️
utils/aplay/delay-report.c 97.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #750      +/-   ##
==========================================
+ Coverage   70.39%   70.55%   +0.16%     
==========================================
  Files          96       99       +3     
  Lines       16103    16230     +127     
  Branches     2520     2535      +15     
==========================================
+ Hits        11335    11451     +116     
- Misses       4649     4660      +11     
  Partials      119      119              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

utils/aplay/alsa-pcm.c Fixed Show fixed Hide fixed
utils/aplay/aplay.c Dismissed Show dismissed Hide dismissed
utils/aplay/aplay.c Fixed Show fixed Hide fixed
@arkq
Copy link
Owner

arkq commented Feb 5, 2025

I'm currently in the review process. If you agree, I will push the refactoring bits to the master branch and keep only the resampling-related changes here. Please give me a day or two to examine the refactoring, then I will look at the resampler. Thank you very very much for this piece of work!

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