Fixed bug 4642 - Rework SDL_netbsdaudio to improve performance
authorSam Lantinga <slouken@libsdl.org>
Sat, 08 Jun 2019 13:03:36 -0700
changeset 12771f26b341b14b4
parent 12770 74ba5a85d682
child 12776 8eb9c7128f76
Fixed bug 4642 - Rework SDL_netbsdaudio to improve performance

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.
src/audio/netbsd/SDL_netbsdaudio.c
     1.1 --- a/src/audio/netbsd/SDL_netbsdaudio.c	Sat Jun 08 10:47:43 2019 -0700
     1.2 +++ b/src/audio/netbsd/SDL_netbsdaudio.c	Sat Jun 08 13:03:36 2019 -0700
     1.3 @@ -43,12 +43,7 @@
     1.4  #include "../SDL_audiodev_c.h"
     1.5  #include "SDL_netbsdaudio.h"
     1.6  
     1.7 -/* Use timer for synchronization */
     1.8 -/* #define USE_TIMER_SYNC */
     1.9 -
    1.10  /* #define DEBUG_AUDIO */
    1.11 -/* #define DEBUG_AUDIO_STREAM */
    1.12 -
    1.13  
    1.14  static void
    1.15  NETBSDAUDIO_DetectDevices(void)
    1.16 @@ -63,14 +58,14 @@
    1.17  #ifdef DEBUG_AUDIO
    1.18      /* *INDENT-OFF* */
    1.19      audio_info_t info;
    1.20 -    const audio_prinfo *prinfo;
    1.21 +    const struct audio_prinfo *prinfo;
    1.22  
    1.23      if (ioctl(this->hidden->audio_fd, AUDIO_GETINFO, &info) < 0) {
    1.24          fprintf(stderr, "AUDIO_GETINFO failed.\n");
    1.25          return;
    1.26      }
    1.27  
    1.28 -    prinfo = this->iscapture ? &info.play : &info.record;
    1.29 +    prinfo = this->iscapture ? &info.record : &info.play;
    1.30  
    1.31      fprintf(stderr, "\n"
    1.32              "[%s info]\n"
    1.33 @@ -115,90 +110,37 @@
    1.34              (info.mode == AUMODE_PLAY) ? "PLAY"
    1.35              : (info.mode = AUMODE_RECORD) ? "RECORD"
    1.36              : (info.mode == AUMODE_PLAY_ALL ? "PLAY_ALL" : "?"));
    1.37 +
    1.38 +    fprintf(stderr, "\n"
    1.39 +            "[audio spec]\n"
    1.40 +            "format		:   0x%x\n"
    1.41 +            "size		:   %u\n"
    1.42 +            "",
    1.43 +            this->spec.format,
    1.44 +            this->spec.size);
    1.45      /* *INDENT-ON* */
    1.46  #endif /* DEBUG_AUDIO */
    1.47  }
    1.48  
    1.49  
    1.50 -/* This function waits until it is possible to write a full sound buffer */
    1.51 -static void
    1.52 -NETBSDAUDIO_WaitDevice(_THIS)
    1.53 -{
    1.54 -#ifndef USE_BLOCKING_WRITES     /* Not necessary when using blocking writes */
    1.55 -    /* See if we need to use timed audio synchronization */
    1.56 -    if (this->hidden->frame_ticks) {
    1.57 -        /* Use timer for general audio synchronization */
    1.58 -        Sint32 ticks;
    1.59 -
    1.60 -        ticks = ((Sint32) (this->hidden->next_frame - SDL_GetTicks())) - FUDGE_TICKS;
    1.61 -        if (ticks > 0) {
    1.62 -            SDL_Delay(ticks);
    1.63 -        }
    1.64 -    } else {
    1.65 -        /* Use SDL_IOReady() for audio synchronization */
    1.66 -#ifdef DEBUG_AUDIO
    1.67 -        fprintf(stderr, "Waiting for audio to get ready\n");
    1.68 -#endif
    1.69 -        if (SDL_IOReady(this->hidden->audio_fd, SDL_TRUE, 10 * 1000)
    1.70 -            <= 0) {
    1.71 -            const char *message =
    1.72 -                "Audio timeout - buggy audio driver? (disabled)";
    1.73 -            /* In general we should never print to the screen,
    1.74 -               but in this case we have no other way of letting
    1.75 -               the user know what happened.
    1.76 -             */
    1.77 -            fprintf(stderr, "SDL: %s\n", message);
    1.78 -            SDL_OpenedAudioDeviceDisconnected(this);
    1.79 -            /* Don't try to close - may hang */
    1.80 -            this->hidden->audio_fd = -1;
    1.81 -#ifdef DEBUG_AUDIO
    1.82 -            fprintf(stderr, "Done disabling audio\n");
    1.83 -#endif
    1.84 -        }
    1.85 -#ifdef DEBUG_AUDIO
    1.86 -        fprintf(stderr, "Ready!\n");
    1.87 -#endif
    1.88 -    }
    1.89 -#endif /* !USE_BLOCKING_WRITES */
    1.90 -}
    1.91 -
    1.92  static void
    1.93  NETBSDAUDIO_PlayDevice(_THIS)
    1.94  {
    1.95 -    int written, p = 0;
    1.96 +    struct SDL_PrivateAudioData *h = this->hidden;
    1.97 +    int written;
    1.98  
    1.99 -    /* Write the audio data, checking for EAGAIN on broken audio drivers */
   1.100 -    do {
   1.101 -        written = write(this->hidden->audio_fd,
   1.102 -                        &this->hidden->mixbuf[p], this->hidden->mixlen - p);
   1.103 -
   1.104 -        if (written > 0)
   1.105 -            p += written;
   1.106 -        if (written == -1 && errno != 0 && errno != EAGAIN && errno != EINTR) {
   1.107 -            /* Non recoverable error has occurred. It should be reported!!! */
   1.108 -            perror("audio");
   1.109 -            break;
   1.110 -        }
   1.111 +    /* Write the audio data */
   1.112 +    written = write(h->audio_fd, h->mixbuf, h->mixlen);
   1.113 +    if (written == -1) {
   1.114 +        /* Non recoverable error has occurred. It should be reported!!! */
   1.115 +        SDL_OpenedAudioDeviceDisconnected(this);
   1.116 +        perror("audio");
   1.117 +        return;
   1.118 +    }
   1.119  
   1.120  #ifdef DEBUG_AUDIO
   1.121 -        fprintf(stderr, "Wrote %d bytes of audio data\n", written);
   1.122 +    fprintf(stderr, "Wrote %d bytes of audio data\n", written);
   1.123  #endif
   1.124 -
   1.125 -        if (p < this->hidden->mixlen
   1.126 -            || ((written < 0) && ((errno == 0) || (errno == EAGAIN)))) {
   1.127 -            SDL_Delay(1);       /* Let a little CPU time go by */
   1.128 -        }
   1.129 -    } while (p < this->hidden->mixlen);
   1.130 -
   1.131 -    /* If timer synchronization is enabled, set the next write frame */
   1.132 -    if (this->hidden->frame_ticks) {
   1.133 -        this->hidden->next_frame += this->hidden->frame_ticks;
   1.134 -    }
   1.135 -
   1.136 -    /* If we couldn't write, assume fatal error for now */
   1.137 -    if (written < 0) {
   1.138 -        SDL_OpenedAudioDeviceDisconnected(this);
   1.139 -    }
   1.140  }
   1.141  
   1.142  static Uint8 *
   1.143 @@ -212,28 +154,19 @@
   1.144  NETBSDAUDIO_CaptureFromDevice(_THIS, void *_buffer, int buflen)
   1.145  {
   1.146      Uint8 *buffer = (Uint8 *) _buffer;
   1.147 -    int br, p = 0;
   1.148 +    int br;
   1.149  
   1.150 -    /* Capture the audio data, checking for EAGAIN on broken audio drivers */
   1.151 -    do {
   1.152 -        br = read(this->hidden->audio_fd, buffer + p, buflen - p);
   1.153 -        if (br > 0)
   1.154 -            p += br;
   1.155 -        if (br == -1 && errno != 0 && errno != EAGAIN && errno != EINTR) {
   1.156 -            /* Non recoverable error has occurred. It should be reported!!! */
   1.157 -            perror("audio");
   1.158 -            return p ? p : -1;
   1.159 -        }
   1.160 +    br = read(this->hidden->audio_fd, buffer, buflen);
   1.161 +    if (br == -1) {
   1.162 +        /* Non recoverable error has occurred. It should be reported!!! */
   1.163 +        perror("audio");
   1.164 +        return -1;
   1.165 +    }
   1.166  
   1.167  #ifdef DEBUG_AUDIO
   1.168 -        fprintf(stderr, "Captured %d bytes of audio data\n", br);
   1.169 +    fprintf(stderr, "Captured %d bytes of audio data\n", br);
   1.170  #endif
   1.171 -
   1.172 -        if (p < buflen
   1.173 -            || ((br < 0) && ((errno == 0) || (errno == EAGAIN)))) {
   1.174 -            SDL_Delay(1);       /* Let a little CPU time go by */
   1.175 -        }
   1.176 -    } while (p < buflen);
   1.177 +    return 0;
   1.178  }
   1.179  
   1.180  static void
   1.181 @@ -271,10 +204,9 @@
   1.182  static int
   1.183  NETBSDAUDIO_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
   1.184  {
   1.185 -    const int flags = iscapture ? OPEN_FLAGS_INPUT : OPEN_FLAGS_OUTPUT;
   1.186      SDL_AudioFormat format = 0;
   1.187      audio_info_t info;
   1.188 -    audio_prinfo *prinfo = iscapture ? &info.play : &info.record;
   1.189 +    struct audio_prinfo *prinfo = iscapture ? &info.record : &info.play;
   1.190  
   1.191      /* We don't care what the devname is...we'll try to open anything. */
   1.192      /*  ...but default to first name in the list... */
   1.193 @@ -294,25 +226,16 @@
   1.194      SDL_zerop(this->hidden);
   1.195  
   1.196      /* Open the audio device */
   1.197 -    this->hidden->audio_fd = open(devname, flags, 0);
   1.198 +    this->hidden->audio_fd = open(devname, iscapture ? O_RDONLY : O_WRONLY);
   1.199      if (this->hidden->audio_fd < 0) {
   1.200          return SDL_SetError("Couldn't open %s: %s", devname, strerror(errno));
   1.201      }
   1.202  
   1.203      AUDIO_INITINFO(&info);
   1.204  
   1.205 -    /* Calculate the final parameters for this audio specification */
   1.206 -    SDL_CalculateAudioSpec(&this->spec);
   1.207 +    prinfo->encoding = AUDIO_ENCODING_NONE;
   1.208  
   1.209 -    /* Set to play mode */
   1.210 -    info.mode = iscapture ? AUMODE_RECORD : AUMODE_PLAY;
   1.211 -    if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) < 0) {
   1.212 -        return SDL_SetError("Couldn't put device into play mode");
   1.213 -    }
   1.214 -
   1.215 -    AUDIO_INITINFO(&info);
   1.216 -    for (format = SDL_FirstAudioFormat(this->spec.format);
   1.217 -         format; format = SDL_NextAudioFormat()) {
   1.218 +    for (format = SDL_FirstAudioFormat(this->spec.format); format;) {
   1.219          switch (format) {
   1.220          case AUDIO_U8:
   1.221              prinfo->encoding = AUDIO_ENCODING_ULINEAR;
   1.222 @@ -338,34 +261,33 @@
   1.223              prinfo->encoding = AUDIO_ENCODING_ULINEAR_BE;
   1.224              prinfo->precision = 16;
   1.225              break;
   1.226 -        default:
   1.227 -            continue;
   1.228          }
   1.229 -
   1.230 -        if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) == 0) {
   1.231 +        if (prinfo->encoding != AUDIO_ENCODING_NONE) {
   1.232              break;
   1.233          }
   1.234 +        format = SDL_NextAudioFormat();
   1.235      }
   1.236  
   1.237 -    if (!format) {
   1.238 +    if (prinfo->encoding == AUDIO_ENCODING_NONE) {
   1.239          return SDL_SetError("No supported encoding for 0x%x", this->spec.format);
   1.240      }
   1.241  
   1.242      this->spec.format = format;
   1.243  
   1.244 -    AUDIO_INITINFO(&info);
   1.245 -    prinfo->channels = this->spec.channels;
   1.246 -    if (ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info) == -1) {
   1.247 -        this->spec.channels = 1;
   1.248 -    }
   1.249 -    AUDIO_INITINFO(&info);
   1.250 -    prinfo->sample_rate = this->spec.freq;
   1.251 +    /* Calculate spec parameters based on our chosen format */
   1.252 +    SDL_CalculateAudioSpec(&this->spec);
   1.253 +
   1.254 +    info.mode = iscapture ? AUMODE_RECORD : AUMODE_PLAY;
   1.255      info.blocksize = this->spec.size;
   1.256      info.hiwat = 5;
   1.257      info.lowat = 3;
   1.258 +    prinfo->sample_rate = this->spec.freq;
   1.259 +    prinfo->channels = this->spec.channels;
   1.260      (void) ioctl(this->hidden->audio_fd, AUDIO_SETINFO, &info);
   1.261 +
   1.262      (void) ioctl(this->hidden->audio_fd, AUDIO_GETINFO, &info);
   1.263      this->spec.freq = prinfo->sample_rate;
   1.264 +    this->spec.channels = prinfo->channels;
   1.265  
   1.266      if (!iscapture) {
   1.267          /* Allocate mixing buffer */
   1.268 @@ -390,7 +312,6 @@
   1.269      impl->DetectDevices = NETBSDAUDIO_DetectDevices;
   1.270      impl->OpenDevice = NETBSDAUDIO_OpenDevice;
   1.271      impl->PlayDevice = NETBSDAUDIO_PlayDevice;
   1.272 -    impl->WaitDevice = NETBSDAUDIO_WaitDevice;
   1.273      impl->GetDeviceBuf = NETBSDAUDIO_GetDeviceBuf;
   1.274      impl->CloseDevice = NETBSDAUDIO_CloseDevice;
   1.275      impl->CaptureFromDevice = NETBSDAUDIO_CaptureFromDevice;