alsa: avoid hardware parameters with an excessive number of periods.
authorAnthony Pesch <inolen@gmail.com>
Fri, 04 May 2018 21:21:32 -0400
changeset 12031b6c4568cc10b
parent 12030 c0c4ba22a685
child 12032 a1fde7f4230f
alsa: avoid hardware parameters with an excessive number of periods.

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.

Fixes Bugzilla #4156.
src/audio/alsa/SDL_alsa_audio.c
     1.1 --- a/src/audio/alsa/SDL_alsa_audio.c	Sun Jun 24 15:12:18 2018 -0400
     1.2 +++ b/src/audio/alsa/SDL_alsa_audio.c	Fri May 04 21:21:32 2018 -0400
     1.3 @@ -435,10 +435,32 @@
     1.4  }
     1.5  
     1.6  static int
     1.7 -ALSA_finalize_hardware(_THIS, snd_pcm_hw_params_t *hwparams, int override)
     1.8 +ALSA_set_buffer_size(_THIS, snd_pcm_hw_params_t *params)
     1.9  {
    1.10      int status;
    1.11 +    snd_pcm_hw_params_t *hwparams;
    1.12      snd_pcm_uframes_t bufsize;
    1.13 +    snd_pcm_uframes_t persize;
    1.14 +
    1.15 +    /* Copy the hardware parameters for this setup */
    1.16 +    snd_pcm_hw_params_alloca(&hwparams);
    1.17 +    ALSA_snd_pcm_hw_params_copy(hwparams, params);
    1.18 +
    1.19 +    /* Prioritize matching the period size to the requested buffer size */
    1.20 +    persize = this->spec.samples;
    1.21 +    status = ALSA_snd_pcm_hw_params_set_period_size_near(
    1.22 +                this->hidden->pcm_handle, hwparams, &persize, NULL);
    1.23 +    if ( status < 0 ) {
    1.24 +        return(-1);
    1.25 +    }
    1.26 +
    1.27 +    /* Next try to restrict the parameters to having only two periods */
    1.28 +    bufsize = this->spec.samples * 2;
    1.29 +    status = ALSA_snd_pcm_hw_params_set_buffer_size_near(
    1.30 +                    this->hidden->pcm_handle, hwparams, &bufsize);
    1.31 +    if ( status < 0 ) {
    1.32 +        return(-1);
    1.33 +    }
    1.34  
    1.35      /* "set" the hardware with the desired parameters */
    1.36      status = ALSA_snd_pcm_hw_params(this->hidden->pcm_handle, hwparams);
    1.37 @@ -446,24 +468,12 @@
    1.38          return(-1);
    1.39      }
    1.40  
    1.41 -    /* Get samples for the actual buffer size */
    1.42 -    status = ALSA_snd_pcm_hw_params_get_buffer_size(hwparams, &bufsize);
    1.43 -    if ( status < 0 ) {
    1.44 -        return(-1);
    1.45 -    }
    1.46 -    if ( !override && bufsize != this->spec.samples * 2 ) {
    1.47 -        return(-1);
    1.48 -    }
    1.49 -
    1.50 -    /* !!! FIXME: Is this safe to do? */
    1.51 -    this->spec.samples = bufsize / 2;
    1.52 +    this->spec.samples = persize;
    1.53  
    1.54      /* This is useful for debugging */
    1.55      if ( SDL_getenv("SDL_AUDIO_ALSA_DEBUG") ) {
    1.56 -        snd_pcm_uframes_t persize = 0;
    1.57          unsigned int periods = 0;
    1.58  
    1.59 -        ALSA_snd_pcm_hw_params_get_period_size(hwparams, &persize, NULL);
    1.60          ALSA_snd_pcm_hw_params_get_periods(hwparams, &periods, NULL);
    1.61  
    1.62          fprintf(stderr,
    1.63 @@ -475,78 +485,6 @@
    1.64  }
    1.65  
    1.66  static int
    1.67 -ALSA_set_period_size(_THIS, snd_pcm_hw_params_t *params, int override)
    1.68 -{
    1.69 -    const char *env;
    1.70 -    int status;
    1.71 -    snd_pcm_hw_params_t *hwparams;
    1.72 -    snd_pcm_uframes_t frames;
    1.73 -    unsigned int periods;
    1.74 -
    1.75 -    /* Copy the hardware parameters for this setup */
    1.76 -    snd_pcm_hw_params_alloca(&hwparams);
    1.77 -    ALSA_snd_pcm_hw_params_copy(hwparams, params);
    1.78 -
    1.79 -    if ( !override ) {
    1.80 -        env = SDL_getenv("SDL_AUDIO_ALSA_SET_PERIOD_SIZE");
    1.81 -        if ( env ) {
    1.82 -            override = SDL_atoi(env);
    1.83 -            if ( override == 0 ) {
    1.84 -                return(-1);
    1.85 -            }
    1.86 -        }
    1.87 -    }
    1.88 -
    1.89 -    frames = this->spec.samples;
    1.90 -    status = ALSA_snd_pcm_hw_params_set_period_size_near(
    1.91 -                this->hidden->pcm_handle, hwparams, &frames, NULL);
    1.92 -    if ( status < 0 ) {
    1.93 -        return(-1);
    1.94 -    }
    1.95 -
    1.96 -    periods = 2;
    1.97 -    status = ALSA_snd_pcm_hw_params_set_periods_near(
    1.98 -                this->hidden->pcm_handle, hwparams, &periods, NULL);
    1.99 -    if ( status < 0 ) {
   1.100 -        return(-1);
   1.101 -    }
   1.102 -
   1.103 -    return ALSA_finalize_hardware(this, hwparams, override);
   1.104 -}
   1.105 -
   1.106 -static int
   1.107 -ALSA_set_buffer_size(_THIS, snd_pcm_hw_params_t *params, int override)
   1.108 -{
   1.109 -    const char *env;
   1.110 -    int status;
   1.111 -    snd_pcm_hw_params_t *hwparams;
   1.112 -    snd_pcm_uframes_t frames;
   1.113 -
   1.114 -    /* Copy the hardware parameters for this setup */
   1.115 -    snd_pcm_hw_params_alloca(&hwparams);
   1.116 -    ALSA_snd_pcm_hw_params_copy(hwparams, params);
   1.117 -
   1.118 -    if ( !override ) {
   1.119 -        env = SDL_getenv("SDL_AUDIO_ALSA_SET_BUFFER_SIZE");
   1.120 -        if ( env ) {
   1.121 -            override = SDL_atoi(env);
   1.122 -            if ( override == 0 ) {
   1.123 -                return(-1);
   1.124 -            }
   1.125 -        }
   1.126 -    }
   1.127 -
   1.128 -    frames = this->spec.samples * 2;
   1.129 -    status = ALSA_snd_pcm_hw_params_set_buffer_size_near(
   1.130 -                    this->hidden->pcm_handle, hwparams, &frames);
   1.131 -    if ( status < 0 ) {
   1.132 -        return(-1);
   1.133 -    }
   1.134 -
   1.135 -    return ALSA_finalize_hardware(this, hwparams, override);
   1.136 -}
   1.137 -
   1.138 -static int
   1.139  ALSA_OpenDevice(_THIS, void *handle, const char *devname, int iscapture)
   1.140  {
   1.141      int status = 0;
   1.142 @@ -692,14 +630,11 @@
   1.143      this->spec.freq = rate;
   1.144  
   1.145      /* Set the buffer size, in samples */
   1.146 -    if ( ALSA_set_period_size(this, hwparams, 0) < 0 &&
   1.147 -         ALSA_set_buffer_size(this, hwparams, 0) < 0 ) {
   1.148 -        /* Failed to set desired buffer size, do the best you can... */
   1.149 -        status = ALSA_set_period_size(this, hwparams, 1);
   1.150 -        if (status < 0) {
   1.151 -            return SDL_SetError("Couldn't set hardware audio parameters: %s", ALSA_snd_strerror(status));
   1.152 -        }
   1.153 +    status = ALSA_set_buffer_size(this, hwparams);
   1.154 +    if (status < 0) {
   1.155 +        return SDL_SetError("Couldn't set hardware audio parameters: %s", ALSA_snd_strerror(status));
   1.156      }
   1.157 +
   1.158      /* Set the software parameters */
   1.159      snd_pcm_sw_params_alloca(&swparams);
   1.160      status = ALSA_snd_pcm_sw_params_current(pcm_handle, swparams);