[2/2] alsa: Add support for channel reconfiguration

Submitted by Arun Raghavan on May 20, 2018, 6:28 a.m.

Details

Message ID 20180520062859.30207-3-arun@arunraghavan.net
State New
Series "Series without cover letter"
Headers show

Commit Message

Arun Raghavan May 20, 2018, 6:28 a.m.
This is needed for supporting high-bitrate formats in passthrough mode.
The alsa-source reconfiguration path is untested at the moment as I do
not have hardware that can support multiple channel configurations for
capture.

This patch is not complete, because setting a channel count that results
in a frame size that does not exactly divide the h/w buffer size results
in a failure during unsuspend() -- the buffer and period count that we
get are necessarily lower than the total buffer / period size.
---
 src/modules/alsa/alsa-sink.c   | 43 ++++++++++++++++++++++++++++------
 src/modules/alsa/alsa-source.c | 43 ++++++++++++++++++++++++++++------
 src/modules/alsa/alsa-util.c   | 38 ++++++++++++++++++++++++++++++
 src/modules/alsa/alsa-util.h   |  1 +
 4 files changed, 111 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index ed9e0a51c..56d4feafd 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -112,6 +112,7 @@  struct userdata {
     pa_cvolume hardware_volume;
 
     unsigned int *rates;
+    unsigned int *channels;
 
     size_t
         frame_size,
@@ -1670,30 +1671,49 @@  static bool sink_set_formats(pa_sink *s, pa_idxset *formats) {
     return true;
 }
 
-static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
+static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) {
     struct userdata *u = s->userdata;
     int i;
-    bool supported = false;
-
-    /* FIXME: we only update rate for now */
+    bool supported_rate = false, supported_channels = false;
 
     pa_assert(u);
 
     for (i = 0; u->rates[i]; i++) {
         if (u->rates[i] == spec->rate) {
-            supported = true;
+            supported_rate = true;
             break;
         }
     }
 
-    if (!supported) {
+    if (!supported_rate) {
         pa_log_info("Sink does not support sample rate of %d Hz", spec->rate);
         return -1;
     }
 
+    for (i = 0; u->channels[i]; i++) {
+        if (u->channels[i] == spec->channels) {
+            supported_channels = true;
+            break;
+        }
+    }
+
+    if (!supported_channels) {
+        pa_log_info("Sink does not support channels: %d", spec->channels);
+        return -1;
+    }
+
     if (!PA_SINK_IS_OPENED(s->state)) {
-        pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
+        pa_log_info("Updating rate and channels for device %s, %d Hz, %d channels", u->device_name, spec->rate, spec->channels);
+
         u->sink->sample_spec.rate = spec->rate;
+        u->sink->sample_spec.channels = spec->channels;
+        if (map)
+            u->sink->channel_map = *map;
+        else
+            pa_channel_map_init_auto(&u->sink->channel_map, u->sink->sample_spec.channels, PA_CHANNEL_MAP_ALSA);
+
+        u->frame_size = pa_frame_size(&u->sink->sample_spec);
+
         return 0;
     }
 
@@ -2345,6 +2365,12 @@  pa_sink *pa_alsa_sink_new(pa_module *m, pa_modargs *ma, const char*driver, pa_ca
         goto fail;
     }
 
+    u->channels = pa_alsa_get_supported_channels(u->pcm_handle, ss.channels);
+    if (!u->channels) {
+        pa_log_error("Failed to find any supported sample rates.");
+        goto fail;
+    }
+
     /* ALSA might tweak the sample spec, so recalculate the frame size */
     frame_size = pa_frame_size(&ss);
 
@@ -2616,6 +2642,9 @@  static void userdata_free(struct userdata *u) {
     if (u->rates)
         pa_xfree(u->rates);
 
+    if (u->channels)
+        pa_xfree(u->channels);
+
     reserve_done(u);
     monitor_done(u);
 
diff --git a/src/modules/alsa/alsa-source.c b/src/modules/alsa/alsa-source.c
index 31d5bb321..fc8ddaf07 100644
--- a/src/modules/alsa/alsa-source.c
+++ b/src/modules/alsa/alsa-source.c
@@ -100,6 +100,7 @@  struct userdata {
     pa_cvolume hardware_volume;
 
     unsigned int *rates;
+    unsigned int *channels;
 
     size_t
         frame_size,
@@ -1466,30 +1467,49 @@  static void source_update_requested_latency_cb(pa_source *s) {
     update_sw_params(u);
 }
 
-static int source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, bool passthrough) {
+static int source_reconfigure_cb(pa_source *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) {
     struct userdata *u = s->userdata;
     int i;
-    bool supported = false;
-
-    /* FIXME: we only update rate for now */
+    bool supported_rate = false, supported_channels = false;
 
     pa_assert(u);
 
     for (i = 0; u->rates[i]; i++) {
         if (u->rates[i] == spec->rate) {
-            supported = true;
+            supported_rate = true;
             break;
         }
     }
 
-    if (!supported) {
+    if (!supported_rate) {
         pa_log_info("Source does not support sample rate of %d Hz", spec->rate);
         return -1;
     }
 
+    for (i = 0; u->channels[i]; i++) {
+        if (u->channels[i] == spec->channels) {
+            supported_channels = true;
+            break;
+        }
+    }
+
+    if (!supported_channels) {
+        pa_log_info("Source does not support channels: %d", spec->channels);
+        return -1;
+    }
+
     if (!PA_SOURCE_IS_OPENED(s->state)) {
-        pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
+        pa_log_info("Updating rate and channels for device %s, %d Hz, %d channels", u->device_name, spec->rate, spec->channels);
+
         u->source->sample_spec.rate = spec->rate;
+        u->source->sample_spec.channels = spec->channels;
+        if (map)
+            u->source->channel_map = *map;
+        else
+            pa_channel_map_init_auto(&u->source->channel_map, u->source->sample_spec.channels, PA_CHANNEL_MAP_ALSA);
+
+        u->frame_size = pa_frame_size(&u->source->sample_spec);
+
         return 0;
     }
 
@@ -2022,6 +2042,12 @@  pa_source *pa_alsa_source_new(pa_module *m, pa_modargs *ma, const char*driver, p
         goto fail;
     }
 
+    u->channels = pa_alsa_get_supported_channels(u->pcm_handle, ss.channels);
+    if (!u->channels) {
+        pa_log_error("Failed to find any supported sample rates.");
+        goto fail;
+    }
+
     /* ALSA might tweak the sample spec, so recalculate the frame size */
     frame_size = pa_frame_size(&ss);
 
@@ -2247,6 +2273,9 @@  static void userdata_free(struct userdata *u) {
     if (u->rates)
         pa_xfree(u->rates);
 
+    if (u->channels)
+        pa_xfree(u->channels);
+
     reserve_done(u);
     monitor_done(u);
 
diff --git a/src/modules/alsa/alsa-util.c b/src/modules/alsa/alsa-util.c
index 61fb4903c..ea5743046 100644
--- a/src/modules/alsa/alsa-util.c
+++ b/src/modules/alsa/alsa-util.c
@@ -1417,6 +1417,44 @@  unsigned int *pa_alsa_get_supported_rates(snd_pcm_t *pcm, unsigned int fallback_
     return rates;
 }
 
+unsigned int *pa_alsa_get_supported_channels(snd_pcm_t *pcm, unsigned int fallback_channels) {
+    snd_pcm_hw_params_t *hwparams;
+    unsigned int min, max;
+    unsigned int *channels;
+    unsigned int n, i;
+    int ret;
+
+    snd_pcm_hw_params_alloca(&hwparams);
+
+    if ((ret = snd_pcm_hw_params_any(pcm, hwparams)) < 0) {
+        pa_log_debug("snd_pcm_hw_params_any() failed: %s", pa_alsa_strerror(ret));
+        return NULL;
+    }
+
+    if (snd_pcm_hw_params_get_channels_min(hwparams, &min) < 0 ||
+        snd_pcm_hw_params_get_channels_max(hwparams, &max) < 0 ||
+        min - max + 1 <= 0) {
+
+        pa_log_debug("Could not probe channel range");
+        return NULL;
+    }
+
+    channels = pa_xnew0(unsigned int, max - min + 1);
+
+    for (i = min, n = 0; i <= max; i++) {
+        if (snd_pcm_hw_params_test_channels(pcm, hwparams, i) == 0) {
+            pa_log_debug("Supported channel count: %d", i);
+            channels[n] = i;
+            n++;
+        }
+    }
+
+    if (n == 0)
+        channels[0] = fallback_channels;
+
+    return channels;
+}
+
 bool pa_alsa_pcm_is_hw(snd_pcm_t *pcm) {
     snd_pcm_info_t* info;
     snd_pcm_info_alloca(&info);
diff --git a/src/modules/alsa/alsa-util.h b/src/modules/alsa/alsa-util.h
index 8345a0ba5..1fa2e3766 100644
--- a/src/modules/alsa/alsa-util.h
+++ b/src/modules/alsa/alsa-util.h
@@ -132,6 +132,7 @@  char *pa_alsa_get_driver_name_by_pcm(snd_pcm_t *pcm);
 char *pa_alsa_get_reserve_name(const char *device);
 
 unsigned int *pa_alsa_get_supported_rates(snd_pcm_t *pcm, unsigned int fallback_rate);
+unsigned int *pa_alsa_get_supported_channels(snd_pcm_t *pcm, unsigned int fallback_channels);
 
 bool pa_alsa_pcm_is_hw(snd_pcm_t *pcm);
 bool pa_alsa_pcm_is_modem(snd_pcm_t *pcm);

Comments

Tanu Kaskinen May 31, 2018, 11:44 a.m.
On Sun, 2018-05-20 at 11:58 +0530, Arun Raghavan wrote:
> This is needed for supporting high-bitrate formats in passthrough mode.
> The alsa-source reconfiguration path is untested at the moment as I do
> not have hardware that can support multiple channel configurations for
> capture.
> 
> This patch is not complete, because setting a channel count that results
> in a frame size that does not exactly divide the h/w buffer size results
> in a failure during unsuspend() -- the buffer and period count that we
> get are necessarily lower than the total buffer / period size.

Is there a reason why you don't reconfigure the buffer size?

> ---
>  src/modules/alsa/alsa-sink.c   | 43 ++++++++++++++++++++++++++++------
>  src/modules/alsa/alsa-source.c | 43 ++++++++++++++++++++++++++++------
>  src/modules/alsa/alsa-util.c   | 38 ++++++++++++++++++++++++++++++
>  src/modules/alsa/alsa-util.h   |  1 +
>  4 files changed, 111 insertions(+), 14 deletions(-)
> 
> diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
> index ed9e0a51c..56d4feafd 100644
> --- a/src/modules/alsa/alsa-sink.c
> +++ b/src/modules/alsa/alsa-sink.c
> @@ -112,6 +112,7 @@ struct userdata {
>      pa_cvolume hardware_volume;
>  
>      unsigned int *rates;
> +    unsigned int *channels;
>  
>      size_t
>          frame_size,
> @@ -1670,30 +1671,49 @@ static bool sink_set_formats(pa_sink *s, pa_idxset *formats) {
>      return true;
>  }
>  
> -static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, bool passthrough) {
> +static int sink_reconfigure_cb(pa_sink *s, pa_sample_spec *spec, pa_channel_map *map, bool passthrough) {
>      struct userdata *u = s->userdata;
>      int i;
> -    bool supported = false;
> -
> -    /* FIXME: we only update rate for now */
> +    bool supported_rate = false, supported_channels = false;
>  
>      pa_assert(u);
>  
>      for (i = 0; u->rates[i]; i++) {
>          if (u->rates[i] == spec->rate) {
> -            supported = true;
> +            supported_rate = true;
>              break;
>          }
>      }
>  
> -    if (!supported) {
> +    if (!supported_rate) {
>          pa_log_info("Sink does not support sample rate of %d Hz", spec->rate);
>          return -1;
>      }
>  
> +    for (i = 0; u->channels[i]; i++) {
> +        if (u->channels[i] == spec->channels) {
> +            supported_channels = true;
> +            break;
> +        }
> +    }
> +
> +    if (!supported_channels) {
> +        pa_log_info("Sink does not support channels: %d", spec->channels);
> +        return -1;
> +    }
> +
>      if (!PA_SINK_IS_OPENED(s->state)) {
> -        pa_log_info("Updating rate for device %s, new rate is %d", u->device_name, spec->rate);
> +        pa_log_info("Updating rate and channels for device %s, %d Hz, %d channels", u->device_name, spec->rate, spec->channels);
> +
>          u->sink->sample_spec.rate = spec->rate;
> +        u->sink->sample_spec.channels = spec->channels;
> +        if (map)
> +            u->sink->channel_map = *map;
> +        else
> +            pa_channel_map_init_auto(&u->sink->channel_map, u->sink->sample_spec.channels, PA_CHANNEL_MAP_ALSA);

You should use pa_channel_map_init_extend(), because
pa_channel_map_init_auto() can fail. (Same for the source.)

> +unsigned int *pa_alsa_get_supported_channels(snd_pcm_t *pcm, unsigned int fallback_channels) {
> +    snd_pcm_hw_params_t *hwparams;
> +    unsigned int min, max;
> +    unsigned int *channels;
> +    unsigned int n, i;
> +    int ret;
> +
> +    snd_pcm_hw_params_alloca(&hwparams);
> +
> +    if ((ret = snd_pcm_hw_params_any(pcm, hwparams)) < 0) {
> +        pa_log_debug("snd_pcm_hw_params_any() failed: %s", pa_alsa_strerror(ret));
> +        return NULL;
> +    }
> +
> +    if (snd_pcm_hw_params_get_channels_min(hwparams, &min) < 0 ||
> +        snd_pcm_hw_params_get_channels_max(hwparams, &max) < 0 ||
> +        min - max + 1 <= 0) {
> +
> +        pa_log_debug("Could not probe channel range");
> +        return NULL;
> +    }
> +
> +    channels = pa_xnew0(unsigned int, max - min + 1);

The intention seems to be that the last array element is always 0 (it
would be good to document this, by the way), but here you don't reserve
space for it. If max and min are the same, then the array has length of
1, which isn't enough for the terminating 0.