We are currently migrating Bugzilla to GitHub issues.
Any changes made to the bug tracker now will be lost, so please do not post new bugs or make changes to them.
When we're done, all bug URLs will redirect to their equivalent location on the new bug tracker.

Bug 5080 - [PATCH] SDL_netbsdaudio: Always use the device's preferred frequency
Summary: [PATCH] SDL_netbsdaudio: Always use the device's preferred frequency
Status: RESOLVED FIXED
Alias: None
Product: SDL
Classification: Unclassified
Component: audio (show other bugs)
Version: HG 2.0
Hardware: x86 NetBSD
: P2 enhancement
Assignee: Ryan C. Gordon
QA Contact: Sam Lantinga
URL:
Keywords: target-2.0.16
Depends on:
Blocks:
 
Reported: 2020-04-08 14:19 UTC by Nia Alarie
Modified: 2021-01-08 18:10 UTC (History)
1 user (show)

See Also:


Attachments
Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl. (1.91 KB, patch)
2020-04-08 14:19 UTC, Nia Alarie
Details | Diff
[v2] Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl. (1.91 KB, patch)
2020-04-08 14:30 UTC, Nia Alarie
Details | Diff
[v3] Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl (2.11 KB, patch)
2021-01-08 12:46 UTC, Nia Alarie
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nia Alarie 2020-04-08 14:19:52 UTC
Created attachment 4298 [details]
Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl.

The NetBSD kernel's audio resampling code is much simpler and lower quality than libsamplerate.

Presumably, if SDL always performs I/O on the audio device in its native frequency, we can avoid resampling audio in the kernel and let SDL do it with libsamplerate instead.
Comment 1 Nia Alarie 2020-04-08 14:30:46 UTC
Created attachment 4299 [details]
[v2] Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl.

Actually, we shouldn't set the device "blocksize" at all (the kernel will ignore it and calculate its own), but especially don't set it to the value of spec->size before spec->size is calculated.
Comment 2 Nia Alarie 2020-12-27 09:24:22 UTC
The last version of this patch I uploaded doesn't work with older NetBSD releases so shouldn't be applied.
Comment 3 Sam Lantinga 2020-12-27 21:11:58 UTC
These changes sound useful. Is there a way to make this work with both old and new versions of NetBSD?
Comment 4 Sam Lantinga 2020-12-27 21:12:25 UTC
Waiting for confirmation before resolving this bug.
Comment 5 Nia Alarie 2021-01-06 13:46:09 UTC
Yes, we can easily make this work on older versions of NetBSD by #ifdefing around the AUDIO_GETFORMAT ioctl usage.

I wonder if this is actually how SDL is supposed to work, though. I just assumed that SDL now does the conversions automatically, so to revisit this patch I'd need reassurance about the internal behaviour.
Comment 6 Sam Lantinga 2021-01-06 17:29:29 UTC
SDL will automatically convert sample rate if the application requests it, so returning the native sample rate is completely reasonable here.
Comment 7 Sam Lantinga 2021-01-06 17:30:33 UTC
Basically the audio driver should change to the requested frequency if the audio hardware can run at that frequency, and return the actual frequency if not.
Comment 8 Nia Alarie 2021-01-08 12:46:29 UTC
Created attachment 4643 [details]
[v3] Patch to get and use the current hardware frequency with the AUDIO_GETFORMAT ioctl

Attempt to work with older NetBSD releases.
Comment 9 Nia Alarie 2021-01-08 12:50:42 UTC
OK, new patch uploaded.

Background: The audio device supports any arbitrary sample rate from 1-192kHz. The user can configure a native frequency for the device, which is nearly always 48kHz by default. Audio at a different frequency will play without issue, but will be resampled in the kernel, and we definitely want libsamplerate to be used instead.

v3 of the patch should _always_ set this->spec.freq to the native rate (usually 48kHz) on NetBSD 9.x and newer, but will leave it unchanged on older releases and rely on the in-kernel resampler. Does that sound right?
Comment 10 Sam Lantinga 2021-01-08 18:10:30 UTC
Yup, looks good, thanks!
https://hg.libsdl.org/SDL/rev/eec6a0d937e2