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

Refactored ALSA backend code to prevent generating hardware parameters with an excessive number of periods #2888

Closed
SDLBugzilla opened this issue Feb 11, 2021 · 0 comments

Comments

@SDLBugzilla
Copy link
Collaborator

@SDLBugzilla SDLBugzilla commented Feb 11, 2021

This bug report was migrated from our old Bugzilla tracker.

These attachments are available in the static archive:

Reported in version: HG 2.1
Reported for operating system, platform: Linux, x86_64

Comments on the original bug report:

On 2018-05-05 01:48:27 +0000, Anthony Pesch wrote:

The previous code attempted to use set_buffer_size / set_period_size discretely,
favoring the parameters which generated a buffer size that was exactly 2x the
requested buffer size. This solution ultimately prioritizes only the buffer size,
which comes at a large performance cost on some machines where this results in
an excessive number of periods. In my case, for a 4096 sample buffer, this
configured the device to use 37 periods with a period size of 221 samples and a
buffer size of 8192 samples. With 37 periods, the SDL Audio thread was consuming
25% of the CPU.

This code has been refactored to use set_period_size and set_buffer_size together.
set_period_size is called first to attempt to set the period to exactly match the
requested buffer size, and set_buffer_size is called second to further refine the
parameters to attempt to use only 2 periods. The fundamental change here is that
the period size / count won't go to extreme values if the buffer size can't be
exactly matched, the buffer size should instead just increase to the next closest
multiple of the target period size that is supported. After changing this, for a
4096 sample buffer, the device is configured to use 3 periods with a period size
of 4096 samples and a buffer size of 12288 samples. With only 3 periods, the SDL
Audio thread doesn't even show up when profiling.

On 2018-05-05 01:49:56 +0000, Anthony Pesch wrote:

Created attachment 3237
patch

On 2018-05-05 01:57:22 +0000, Anthony Pesch wrote:

Sorry for the malformed comment, it was copied and pasted from the diff. Please just read the comment in the diff directly.

On 2018-05-05 17:38:48 +0000, Sam Lantinga wrote:

This seems reasonable. Ryan, can you review and apply this if it looks good?

On 2018-05-06 02:08:13 +0000, Ryan C. Gordon wrote:

Going to test this on a Raspberry Pi and a desktop Linux box first, but this looks like a good patch from here.

--ryan.

On 2018-06-25 05:50:48 +0000, Ryan C. Gordon wrote:

Okay, tried this on a desktop and a Raspberry Pi, and it's solid. This patch is now https://hg.libsdl.org/SDL/rev/b6c4568cc10b, thanks!

--ryan.

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

No branches or pull requests

1 participant