Skip to content

Conversation

@sniperrifle2004
Copy link

@sniperrifle2004 sniperrifle2004 commented Jun 18, 2020

This will accomplish two things, in addition to what the buffer
time already provides:

  • With a latency of 100ms it doesn't make sense to wake up the
    cpu for new audio data more often than this. Maintaining a lower
    period time will create unnecessary load. This is an effect
    that stacks if the alsa device is virtual (Like the pulse plugin,
    which happens to have a low default period time, which stands
    in stark contrast to its reportedly high buffer). Most of the
    time 50ms would also work, but its better to have some fallout
    protection in the default scenario
  • It prevents errors trying to set the buffer time.

I removed the workaround introduced earlier for the second issue. While it's still not entirely clear why it should actually raise an error there if you do not first set the period time, there is something to be said for first choosing the period time in general, since it does restrict the possible buffer sizes.

Fixes #369, Fixes #391 (probably)

Now I also strongly suspect a good period time will have a positive effect for #322, but that should be verified. Since pulse appears involved I am optimistic.

EDIT: I now think that period size first is a requirement that comes with devices that require buffer sizes to be multiples of the period size. If that period size is not defined it is not yet possible to set a buffer size even though the minimum and maximum are well defined. That is probably why set_buffer_time_near fails.

This will accomplish two things, in addition to what the buffer
time already provides:
  - With a latency of 100ms it doesn't make sense to wake up the
    cpu for new audio data more often than this. Maintaining a lower
    period time will create unnecessary load. This is an effect
    that stacks if the alsa device is virtual (Like the pulse plugin,
    which happens to have a low default period time, which stands
    in stark contrast to its reportedly high buffer). Most of the
    time 50ms would also work, but its better to have some fallout
    protection in the default scenario
  - It prevents errors trying to set the buffer time.
@sniperrifle2004 sniperrifle2004 changed the title Add a default period time [Alsa Host] Add a default period time Jun 18, 2020
@richard-uk1
Copy link
Contributor

richard-uk1 commented Jun 19, 2020

Can confirm that it works for me! Awesome!

EDIT oops meant to comment on the issue.

@rfwatson
Copy link
Contributor

I tested this change locally after also experiencing high CPU load in a simple ALSA event loop.

Nothing scientific, but at a glance building CPAL from this branch reduced CPU usage for the process from around 10% to ~2%. 🚀

@richard-uk1
Copy link
Contributor

Really excited about this landing - I will be able to use cpal again! :)

@sniperrifle2004
Copy link
Author

Superseded by #520.

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.

Problem when setting buffer time (alsa backend) Problem with playback under arch linux.

3 participants