Skip to content

Commit

Permalink
Fixed bug 4642 - Rework SDL_netbsdaudio to improve performance
Browse files Browse the repository at this point in the history 
Nia Alarie

The NetBSD audio driver has a few problems. Lots of obsolete code, and extremely bad performance and stuttering.

I have a patch in NetBSD's package system to improve it. This is my attempt to upstream it.

The changes include:

* Removing references to defines which are never used.
* Using the correct structures for playback and recording, previously they were the wrong way around.
* Using the correct types ('struct audio_prinfo' in contrast to 'audio_prinfo')
* Removing the use of non-blocking I/O, as suggested in #3177.
* Removing workarounds for driver bugs on systems that don't exist or use this driver any more.
* Removing all usage of SDL_Delay(1)
* Removing pointless use of AUDIO_INITINFO and tests that expect AUDIO_SETINFO to fail when it can't.

These changes bring its performance in line with the DSP audio driver.
  • Loading branch information
@slouken
slouken committed Jun 8, 2019
1 parent f2c8d8e commit 15bae95
Showing 1 changed file with 47 additions and 126 deletions.
173 changes: 47 additions & 126 deletions src/audio/netbsd/SDL_netbsdaudio.c
View file
Expand Up @@ -43,12 +43,7 @@
#include "../SDL_audiodev_c.h"
#include "SDL_netbsdaudio.h"

/* Use timer for synchronization */
/* #define USE_TIMER_SYNC */

/* #define DEBUG_AUDIO */
/* #define DEBUG_AUDIO_STREAM */


static void
NETBSDAUDIO_DetectDevices(void)
Expand All @@ -63,14 +58,14 @@ NETBSDAUDIO_Status(_THIS)
#ifdef DEBUG_AUDIO
/* *INDENT-OFF* */
audio_info_t info;
const audio_prinfo *prinfo;
const struct audio_prinfo *prinfo;

if (ioctl(this->hidden->audio_fd, AUDIO_GETINFO, &info) < 0) {
fprintf(stderr, "AUDIO_GETINFO failed.\n");
return;
}

prinfo = this->iscapture ? &info.play : &info.record;
prinfo = this->iscapture ? &info.record : &info.play;

fprintf(stderr, "\n"
"[%s info]\n"
Expand Down Expand Up @@ -115,90 +110,37 @@ NETBSDAUDIO_Status(_THIS)
(info.mode == AUMODE_PLAY) ? "PLAY"
: (info.mode = AUMODE_RECORD) ? "RECORD"
: (info.mode == AUMODE_PLAY_ALL ? "PLAY_ALL" : "?"));

fprintf(stderr, "\n"
"[audio spec]\n"
"format : 0x%x\n"
"size : %u\n"
"",
this->spec.format,
this->spec.size);
/* *INDENT-ON* */
#endif /* DEBUG_AUDIO */
}


/* This function waits until it is possible to write a full sound buffer */
static void
NETBSDAUDIO_WaitDevice(_THIS)
{
#ifndef USE_BLOCKING_WRITES /* Not necessary when using blocking writes */
/* See if we need to use timed audio synchronization */
if (this->hidden->frame_ticks) {
/* Use timer for general audio synchronization */
Sint32 ticks;

ticks = ((Sint32) (this->hidden->next_frame - SDL_GetTicks())) - FUDGE_TICKS;
if (ticks > 0) {
SDL_Delay(ticks);
}
} else {
/* Use SDL_IOReady() for audio synchronization */
#ifdef DEBUG_AUDIO
fprintf(stderr, "Waiting for audio to get ready\n");
#endif
if (SDL_IOReady(this->hidden->audio_fd, SDL_TRUE, 10 * 1000)
<= 0) {
const char *message =
"Audio timeout - buggy audio driver? (disabled)";
/* In general we should never print to the screen,
but in this case we have no other way of letting
the user know what happened.
*/
fprintf(stderr, "SDL: %s\n", message);
SDL_OpenedAudioDeviceDisconnected(this);
/* Don't try to close - may hang */
this->hidden->audio_fd = -1;
#ifdef DEBUG_AUDIO
fprintf(stderr, "Done disabling audio\n");
#endif
}
#ifdef DEBUG_AUDIO
fprintf(stderr, "Ready!\n");
#endif
}
#endif /* !USE_BLOCKING_WRITES */
}

static void
NETBSDAUDIO_PlayDevice(_THIS)
{
int written, p = 0;

/* Write the audio data, checking for EAGAIN on broken audio drivers */
do {
written = write(this->hidden->audio_fd,
&this->hidden->mixbuf[p], this->hidden->mixlen - p);

if (written > 0)
p += written;
if (written == -1 && errno != 0 && errno != EAGAIN && errno != EINTR) {
/* Non recoverable error has occurred. It should be reported!!! */
perror("audio");
break;
}

#ifdef DEBUG_AUDIO
fprintf(stderr, "Wrote %d bytes of audio data\n", written);
#endif

if (p < this->hidden->mixlen
|| ((written < 0) && ((errno == 0) || (errno == EAGAIN)))) {
SDL_Delay(1); /* Let a little CPU time go by */
}
} while (p < this->hidden->mixlen);

/* If timer synchronization is enabled, set the next write frame */
if (this->hidden->frame_ticks) {
this->hidden->next_frame += this->hidden->frame_ticks;
}
struct SDL_PrivateAudioData *h = this->hidden;
int written;

/* If we couldn't write, assume fatal error for now */
if (written < 0) {
/* Write the audio data */
written = write(h->audio_fd, h->mixbuf, h->mixlen);
if (written == -1) {
/* Non recoverable error has occurred. It should be reported!!! */
SDL_OpenedAudioDeviceDisconnected(this);
perror("audio");
return;
}

#ifdef DEBUG_AUDIO
fprintf(stderr, "Wrote %d bytes of audio data\n", written);
#endif
}

static Uint8 *
Expand All @@ -212,28 +154,19 @@ static int
NETBSDAUDIO_CaptureFromDevice(_THIS, void *_buffer, int buflen)
{
Uint8 *buffer = (Uint8 *) _buffer;
int br, p = 0;

/* Capture the audio data, checking for EAGAIN on broken audio drivers */
do {
br = read(this->hidden->audio_fd, buffer + p, buflen - p);
if (br > 0)
p += br;
if (br == -1 && errno != 0 && errno != EAGAIN && errno != EINTR) {
/* Non recoverable error has occurred. It should be reported!!! */
perror("audio");
return p ? p : -1;
}
int br;

br = read(this->hidden->audio_fd, buffer, buflen);
if (br == -1) {
/* Non recoverable error has occurred. It should be reported!!! */
perror("audio");
return -1;
}

#ifdef DEBUG_AUDIO
fprintf(stderr, "Captured %d bytes of audio data\n", br);
fprintf(stderr, "Captured %d bytes of audio data\n", br);
#endif

if (p < buflen
|| ((br < 0) && ((errno == 0) || (errno == EAGAIN)))) {
SDL_Delay(1); /* Let a little CPU time go by */
}
} while (p < buflen);
return 0;
}

static void
Expand Down Expand Up @@ -271,10 +204,9 @@ NETBSDAUDIO_CloseDevice(_THIS)
static int
NETBSDAUDIO_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
{
const int flags = iscapture ? OPEN_FLAGS_INPUT : OPEN_FLAGS_OUTPUT;
SDL_AudioFormat format = 0;
audio_info_t info;
audio_prinfo *prinfo = iscapture ? &info.play : &info.record;
struct audio_prinfo *prinfo = iscapture ? &info.record : &info.play;

/* We don't care what the devname is...we'll try to open anything. */
/* ...but default to first name in the list... */
Expand All @@ -294,25 +226,16 @@ NETBSDAUDIO_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
SDL_zerop(this->hidden);

/* Open the audio device */
this->hidden->audio_fd = open(devname, flags, 0);
this->hidden->audio_fd = open(devname, iscapture ? O_RDONLY : O_WRONLY);
if (this->hidden->audio_fd < 0) {
return SDL_SetError("Couldn't open %s: %s", devname, strerror(errno));
}

AUDIO_INITINFO(&info);

/* Calculate the final parameters for this audio specification */
SDL_CalculateAudioSpec(&this->spec);

/* Set to play mode */
info.mode = iscapture ? AUMODE_RECORD : AUMODE_PLAY;
if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) < 0) {
return SDL_SetError("Couldn't put device into play mode");
}
prinfo->encoding = AUDIO_ENCODING_NONE;

AUDIO_INITINFO(&info);
for (format = SDL_FirstAudioFormat(this->spec.format);
format; format = SDL_NextAudioFormat()) {
for (format = SDL_FirstAudioFormat(this->spec.format); format;) {
switch (format) {
case AUDIO_U8:
prinfo->encoding = AUDIO_ENCODING_ULINEAR;
Expand All @@ -338,34 +261,33 @@ NETBSDAUDIO_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
prinfo->encoding = AUDIO_ENCODING_ULINEAR_BE;
prinfo->precision = 16;
break;
default:
continue;
}

if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) == 0) {
if (prinfo->encoding != AUDIO_ENCODING_NONE) {
break;
}
format = SDL_NextAudioFormat();
}

if (!format) {
if (prinfo->encoding == AUDIO_ENCODING_NONE) {
return SDL_SetError("No supported encoding for 0x%x", this->spec.format);
}

this->spec.format = format;

AUDIO_INITINFO(&info);
prinfo->channels = this->spec.channels;
if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) == -1) {
this->spec.channels = 1;
}
AUDIO_INITINFO(&info);
prinfo->sample_rate = this->spec.freq;
/* Calculate spec parameters based on our chosen format */
SDL_CalculateAudioSpec(&this->spec);

info.mode = iscapture ? AUMODE_RECORD : AUMODE_PLAY;
info.blocksize = this->spec.size;
info.hiwat = 5;
info.lowat = 3;
prinfo->sample_rate = this->spec.freq;
prinfo->channels = this->spec.channels;
(void) ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info);

(void) ioctl(this->hidden->audio_fd, AUDIO_GETINFO, &info);
this->spec.freq = prinfo->sample_rate;
this->spec.channels = prinfo->channels;

if (!iscapture) {
/* Allocate mixing buffer */
Expand All @@ -390,7 +312,6 @@ NETBSDAUDIO_Init(SDL_AudioDriverImpl * impl)
impl->DetectDevices = NETBSDAUDIO_DetectDevices;
impl->OpenDevice = NETBSDAUDIO_OpenDevice;
impl->PlayDevice = NETBSDAUDIO_PlayDevice;
impl->WaitDevice = NETBSDAUDIO_WaitDevice;
impl->GetDeviceBuf = NETBSDAUDIO_GetDeviceBuf;
impl->CloseDevice = NETBSDAUDIO_CloseDevice;
impl->CaptureFromDevice = NETBSDAUDIO_CaptureFromDevice;
Expand Down

0 comments on commit 15bae95

Please sign in to comment.