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

Fix/alsa wait #1770

Merged
merged 4 commits into from
Jul 5, 2023
Merged

Fix/alsa wait #1770

merged 4 commits into from
Jul 5, 2023

Conversation

cmhulbert
Copy link
Contributor

The previous commit did fix ALSA not being interruptible, but I notice that it also doesn't cleanly end on it own (i.e. without being interrupted). Specifically, availableSamples was not implemented, so wait() always returned immediately. This was not noticed, since stop() called drain() which waits for the written audio to end. This seeks to solve that by:

  • properly updating availableSamples so wait() works properly
  • change stop() to call snd_pcm_drop() instead of snd_pcm_drain() to immediately drop all remaining frames, instead of waiting for them to finish
  • calling wait() then stop() when a sound is done
  • calling only stop() when a sound is cancelled
  • implement snd_pcm_delay() to get the number of frames remaining to be heard after snd_pcm_writei to determine how long to wait before writing the next sound (or returning, if done).

previously, `wait()` was returnign immediately, but `stop()` was not
stopping properly (drain, instead of drop), so it had the impression of working.
…iod worth of frames remaining

Fixes the issue where sounds where not properly waiting to play before
finishing. Would cause only a very small portion of the start of the
sound to play.
@codecov-commenter
Copy link

codecov-commenter commented Jul 5, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.04 ⚠️

Comparison is base (b70656a) 51.19% compared to head (bf0ca15) 51.16%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1770      +/-   ##
==========================================
- Coverage   51.19%   51.16%   -0.04%     
==========================================
  Files        1740     1740              
  Lines      102140   102163      +23     
  Branches    14453    14456       +3     
==========================================
- Hits        52294    52270      -24     
- Misses      45905    45935      +30     
- Partials     3941     3958      +17     
Flag Coverage Δ
unittests 51.16% <0.00%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ain/kotlin/korlibs/audio/sound/SoundAudioStream.kt 68.35% <0.00%> (-0.88%) ⬇️
...onMain/kotlin/korlibs/audio/sound/backends/ALSA.kt 5.04% <0.00%> (-1.15%) ⬇️

... and 26 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@cmhulbert
Copy link
Contributor Author

All modification of availableSamples are now locked

@soywiz soywiz enabled auto-merge (squash) July 5, 2023 20:22
@soywiz
Copy link
Member

soywiz commented Jul 5, 2023

Thanks!

@soywiz soywiz merged commit eb62d05 into korlibs:main Jul 5, 2023
10 checks passed
@cmhulbert cmhulbert deleted the fix/ALSAWait branch July 5, 2023 21:39
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.

None yet

3 participants