From aa0272f6f357706307ec4cab7296fd4f691db2de Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Fri, 24 Oct 2025 10:28:38 +0200 Subject: [PATCH] treewide: remove some obsolete channel checks The spa_audio_info can not be parsed with too many channels so there is always enough space for the positions. --- pipewire-alsa/alsa-plugins/pcm_pipewire.c | 31 ++++++++----------- spa/include/spa/param/audio/raw-utils.h | 4 ++- spa/plugins/audioconvert/audioconvert.c | 36 ++++++++++------------- spa/plugins/bluez5/a2dp-codec-opus.c | 15 +++++----- spa/plugins/bluez5/bluez5-device.c | 5 ++-- src/modules/module-ffado-driver.c | 6 ++-- src/modules/module-filter-chain.c | 3 +- src/modules/module-vban/stream.c | 14 ++++----- 8 files changed, 50 insertions(+), 64 deletions(-) diff --git a/pipewire-alsa/alsa-plugins/pcm_pipewire.c b/pipewire-alsa/alsa-plugins/pcm_pipewire.c index bf7196898..24a66eabc 100644 --- a/pipewire-alsa/alsa-plugins/pcm_pipewire.c +++ b/pipewire-alsa/alsa-plugins/pcm_pipewire.c @@ -643,11 +643,8 @@ static int snd_pcm_pipewire_pause(snd_pcm_ioplug_t * io, int enable) #define _FORMAT_BE(p, fmt) p ? SPA_AUDIO_FORMAT_UNKNOWN : SPA_AUDIO_FORMAT_ ## fmt ## _OE #endif -static int set_default_channels(uint32_t channels, uint32_t *position, uint32_t max_position) +static int set_default_channels(uint32_t channels, uint32_t position[MAX_CHANNELS]) { - if (max_position < 8) - return -ENOSPC; - switch (channels) { case 8: position[6] = SPA_AUDIO_CHANNEL_SL; @@ -775,8 +772,7 @@ static int snd_pcm_pipewire_hw_params(snd_pcm_ioplug_t * io, case SPA_MEDIA_SUBTYPE_raw: pw->requested.info.raw.channels = io->channels; pw->requested.info.raw.rate = io->rate; - set_default_channels(io->channels, pw->requested.info.raw.position, - SPA_N_ELEMENTS(pw->requested.info.raw.position)); + set_default_channels(io->channels, pw->requested.info.raw.position); fmt_str = spa_type_audio_format_to_short_name(pw->requested.info.raw.format); pw->format = pw->requested; break; @@ -784,8 +780,7 @@ static int snd_pcm_pipewire_hw_params(snd_pcm_ioplug_t * io, pw->requested.info.dsd.bitorder = SPA_PARAM_BITORDER_msb; pw->requested.info.dsd.channels = io->channels; pw->requested.info.dsd.rate = io->rate * SPA_ABS(pw->requested.info.dsd.interleave); - set_default_channels(io->channels, pw->requested.info.dsd.position, - SPA_N_ELEMENTS(pw->requested.info.dsd.position)); + set_default_channels(io->channels, pw->requested.info.dsd.position); pw->format = pw->requested; /* we need to let the server decide these values */ pw->format.info.dsd.bitorder = 0; @@ -907,30 +902,30 @@ static int snd_pcm_pipewire_set_chmap(snd_pcm_ioplug_t * io, { snd_pcm_pipewire_t *pw = io->private_data; unsigned int i; - uint32_t *position, max_position; + uint32_t *position; switch (pw->requested.media_subtype) { case SPA_MEDIA_SUBTYPE_raw: pw->requested.info.raw.channels = map->channels; position = pw->requested.info.raw.position; - max_position = SPA_N_ELEMENTS(pw->requested.info.raw.position); break; case SPA_MEDIA_SUBTYPE_dsd: pw->requested.info.dsd.channels = map->channels; position = pw->requested.info.dsd.position; - max_position = SPA_N_ELEMENTS(pw->requested.info.dsd.position); break; default: return -EINVAL; } + if (map->channels > MAX_CHANNELS) + return -ENOTSUP; + for (i = 0; i < map->channels; i++) { - uint32_t pos = chmap_to_channel(map->pos[i]); char buf[8]; - if (i < max_position) - position[i] = pos; + position[i] = chmap_to_channel(map->pos[i]); pw_log_debug("map %d: %s / %s", i, snd_pcm_chmap_name(map->pos[i]), - spa_type_audio_channel_make_short_name(pos, buf, sizeof(buf), "UNK")); + spa_type_audio_channel_make_short_name(position[i], + buf, sizeof(buf), "UNK")); } return 1; } @@ -939,18 +934,16 @@ static snd_pcm_chmap_t * snd_pcm_pipewire_get_chmap(snd_pcm_ioplug_t * io) { snd_pcm_pipewire_t *pw = io->private_data; snd_pcm_chmap_t *map; - uint32_t i, channels, *position, max_position; + uint32_t i, channels, *position; switch (pw->requested.media_subtype) { case SPA_MEDIA_SUBTYPE_raw: channels = pw->requested.info.raw.channels; position = pw->requested.info.raw.position; - max_position = SPA_N_ELEMENTS(pw->requested.info.raw.position); break; case SPA_MEDIA_SUBTYPE_dsd: channels = pw->requested.info.dsd.channels; position = pw->requested.info.dsd.position; - max_position = SPA_N_ELEMENTS(pw->requested.info.dsd.position); break; default: return NULL; @@ -960,7 +953,7 @@ static snd_pcm_chmap_t * snd_pcm_pipewire_get_chmap(snd_pcm_ioplug_t * io) channels * sizeof(unsigned int)); map->channels = channels; for (i = 0; i < channels; i++) - map->pos[i] = channel_to_chmap(position[i % max_position]); + map->pos[i] = channel_to_chmap(position[i]); return map; } diff --git a/spa/include/spa/param/audio/raw-utils.h b/spa/include/spa/param/audio/raw-utils.h index ea7097a69..ce5e61e67 100644 --- a/spa/include/spa/param/audio/raw-utils.h +++ b/spa/include/spa/param/audio/raw-utils.h @@ -43,7 +43,7 @@ spa_format_audio_raw_ext_parse(const struct spa_pod *format, struct spa_audio_in SPA_FORMAT_AUDIO_channels, SPA_POD_OPT_Int(&info->channels), SPA_FORMAT_AUDIO_position, SPA_POD_OPT_Pod(&position)); if (info->channels > max_position) - return -ENOTSUP; + return -ECHRNG; if (position == NULL || spa_pod_copy_array(position, SPA_TYPE_Id, info->position, max_position) != info->channels) SPA_FLAG_SET(info->flags, SPA_AUDIO_FLAG_UNPOSITIONED); @@ -78,6 +78,8 @@ spa_format_audio_raw_ext_build(struct spa_pod_builder *builder, uint32_t id, if (info->channels != 0) { spa_pod_builder_add(builder, SPA_FORMAT_AUDIO_channels, SPA_POD_Int(info->channels), 0); + /* we drop the positions here when we can't read all of them. This is + * really a malformed spa_audio_info structure. */ if (!SPA_FLAG_IS_SET(info->flags, SPA_AUDIO_FLAG_UNPOSITIONED) && max_position > info->channels) { spa_pod_builder_add(builder, SPA_FORMAT_AUDIO_position, diff --git a/spa/plugins/audioconvert/audioconvert.c b/spa/plugins/audioconvert/audioconvert.c index e40c4f2ab..e8c95b44a 100644 --- a/spa/plugins/audioconvert/audioconvert.c +++ b/spa/plugins/audioconvert/audioconvert.c @@ -1148,7 +1148,7 @@ struct spa_filter_graph_events graph_events = { }; static int setup_filter_graph(struct impl *this, struct filter_graph *g, - uint32_t channels, uint32_t *position, uint32_t max_position) + uint32_t channels, uint32_t *position) { int res; char rate_str[64], in_ports[64]; @@ -1163,9 +1163,8 @@ static int setup_filter_graph(struct impl *this, struct filter_graph *g, snprintf(in_ports, sizeof(in_ports), "%d", channels); g->n_inputs = channels; if (position) { - uint32_t n_pos = SPA_MIN(channels, max_position); - memcpy(g->inputs_position, position, sizeof(uint32_t) * n_pos); - memcpy(g->outputs_position, position, sizeof(uint32_t) * n_pos); + memcpy(g->inputs_position, position, sizeof(uint32_t) * channels); + memcpy(g->outputs_position, position, sizeof(uint32_t) * channels); } } @@ -1180,7 +1179,7 @@ static int setup_filter_graph(struct impl *this, struct filter_graph *g, return res; } -static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *position, uint32_t max_position); +static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *position); static void free_tmp(struct impl *this) { @@ -1262,7 +1261,7 @@ static int ensure_tmp(struct impl *this) static int setup_filter_graphs(struct impl *impl, bool force) { int res; - uint32_t channels, *position, max_position; + uint32_t channels, *position; struct dir *in, *out; struct filter_graph *g, *t; @@ -1271,7 +1270,6 @@ static int setup_filter_graphs(struct impl *impl, bool force) channels = in->format.info.raw.channels; position = in->format.info.raw.position; - max_position = SPA_N_ELEMENTS(in->format.info.raw.position); impl->maxports = SPA_MAX(in->format.info.raw.channels, out->format.info.raw.channels); spa_list_for_each_safe(g, t, &impl->active_graphs, link) { @@ -1279,20 +1277,19 @@ static int setup_filter_graphs(struct impl *impl, bool force) continue; if (force) g->setup = false; - if ((res = setup_filter_graph(impl, g, channels, position, max_position)) < 0) { + if ((res = setup_filter_graph(impl, g, channels, position)) < 0) { g->removing = true; spa_log_warn(impl->log, "failed to activate graph %d: %s", g->order, spa_strerror(res)); } else { channels = g->n_outputs; position = g->outputs_position; - max_position = SPA_N_ELEMENTS(g->outputs_position); impl->maxports = SPA_MAX(impl->maxports, channels); } } if ((res = ensure_tmp(impl)) < 0) return res; - if ((res = setup_channelmix(impl, channels, position, max_position)) < 0) + if ((res = setup_channelmix(impl, channels, position)) < 0) return res; return 0; @@ -2058,8 +2055,7 @@ static int setup_in_convert(struct impl *this) dst_info.info.raw.channels, dst_info.info.raw.rate); - qsort(dst_info.info.raw.position, SPA_MIN(dst_info.info.raw.channels, - SPA_N_ELEMENTS(dst_info.info.raw.position)), + qsort(dst_info.info.raw.position, dst_info.info.raw.channels, sizeof(uint32_t), int32_cmp); for (i = 0; i < src_info.info.raw.channels; i++) { @@ -2188,18 +2184,18 @@ static void set_volume(struct impl *this) this->params[IDX_Props].user++; } -static char *format_position(char *str, size_t len, uint32_t channels, uint32_t *position, uint32_t max_position) +static char *format_position(char *str, size_t len, uint32_t channels, uint32_t *position) { uint32_t i, idx = 0; char buf[8]; for (i = 0; i < channels; i++) idx += snprintf(str + idx, len - idx, "%s%s", i == 0 ? "" : " ", - spa_type_audio_channel_make_short_name(position[i % max_position], + spa_type_audio_channel_make_short_name(position[i], buf, sizeof(buf), "UNK")); return str; } -static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *position, uint32_t max_position) +static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *position) { struct dir *in = &this->dir[SPA_DIRECTION_INPUT]; struct dir *out = &this->dir[SPA_DIRECTION_OUTPUT]; @@ -2212,7 +2208,7 @@ static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *posi dst_chan = out->format.info.raw.channels; for (i = 0, src_mask = 0; i < src_chan; i++) { - p = position[i % max_position]; + p = position[i]; src_mask |= 1ULL << (p < 64 ? p : 0); } for (i = 0, dst_mask = 0; i < dst_chan; i++) { @@ -2221,10 +2217,9 @@ static int setup_channelmix(struct impl *this, uint32_t channels, uint32_t *posi } spa_log_info(this->log, "in %s (%016"PRIx64")", format_position(str, sizeof(str), - src_chan, position, max_position), src_mask); + src_chan, position), src_mask); spa_log_info(this->log, "out %s (%016"PRIx64")", format_position(str, sizeof(str), - dst_chan, out->format.info.raw.position, - SPA_N_ELEMENTS(out->format.info.raw.position)), dst_mask); + dst_chan, out->format.info.raw.position), dst_mask); spa_log_info(this->log, "%p: %s/%d@%d->%s/%d@%d %08"PRIx64":%08"PRIx64, this, spa_debug_type_find_name(spa_type_audio_format, SPA_AUDIO_FORMAT_DSP_F32), @@ -2352,8 +2347,7 @@ static int setup_out_convert(struct impl *this) dst_info.info.raw.channels, dst_info.info.raw.rate); - qsort(src_info.info.raw.position, SPA_MIN(src_info.info.raw.channels, - SPA_N_ELEMENTS(src_info.info.raw.position)), + qsort(src_info.info.raw.position, src_info.info.raw.channels, sizeof(uint32_t), int32_cmp); for (i = 0; i < src_info.info.raw.channels; i++) { diff --git a/spa/plugins/bluez5/a2dp-codec-opus.c b/spa/plugins/bluez5/a2dp-codec-opus.c index 0881c0255..c5b042b20 100644 --- a/spa/plugins/bluez5/a2dp-codec-opus.c +++ b/spa/plugins/bluez5/a2dp-codec-opus.c @@ -491,7 +491,7 @@ static void get_default_bitrates(const struct media_codec *codec, bool bidi, int static int get_mapping(const struct media_codec *codec, const a2dp_opus_05_direction_t *conf, bool use_surround_encoder, uint8_t *streams_ret, uint8_t *coupled_streams_ret, - const uint8_t **surround_mapping, uint32_t *positions, uint32_t max_positions) + const uint8_t **surround_mapping, uint32_t *positions) { const uint32_t channels = conf->channels; const uint32_t location = OPUS_05_GET_LOCATION(*conf); @@ -539,12 +539,11 @@ static int get_mapping(const struct media_codec *codec, const a2dp_opus_05_direc if (location & loc.mask) { uint32_t idx = permutation ? permutation[j] : j; - if (idx < max_positions) - positions[idx] = loc.position; + positions[idx] = loc.position; j++; } } - for (i = SPA_AUDIO_CHANNEL_START_Aux; j < channels && j < max_positions; ++i, ++j) + for (i = SPA_AUDIO_CHANNEL_START_Aux; j < channels; ++i, ++j) positions[j] = i; } @@ -779,7 +778,7 @@ static int codec_enum_config(const struct media_codec *codec, uint32_t flags, dir = !is_duplex_codec(codec) ? &conf.main : &conf.bidi; - if (get_mapping(codec, dir, surround_encoder, NULL, NULL, NULL, position, MAX_CHANNELS) < 0) + if (get_mapping(codec, dir, surround_encoder, NULL, NULL, NULL, position) < 0) return -EINVAL; spa_pod_builder_push_object(b, &f[0], SPA_TYPE_OBJECT_Format, id); @@ -832,9 +831,9 @@ static int codec_validate_config(const struct media_codec *codec, uint32_t flags info->info.raw.channels = dir1->channels; if (get_mapping(codec, dir1, surround_encoder, NULL, NULL, NULL, - info->info.raw.position, SPA_N_ELEMENTS(info->info.raw.position)) < 0) + info->info.raw.position) < 0) return -EINVAL; - if (get_mapping(codec, dir2, surround_encoder, NULL, NULL, NULL, NULL, 0) < 0) + if (get_mapping(codec, dir2, surround_encoder, NULL, NULL, NULL, NULL) < 0) return -EINVAL; return 0; @@ -925,7 +924,7 @@ static void *codec_init(const struct media_codec *codec, uint32_t flags, if ((res = codec_validate_config(codec, flags, config, config_len, &config_info)) < 0) goto error; if ((res = get_mapping(codec, dir, surround_encoder, &this->streams, &this->coupled_streams, - &enc_mapping, NULL, 0)) < 0) + &enc_mapping, NULL)) < 0) goto error; if (config_info.info.raw.channels != info->info.raw.channels) { res = -EINVAL; diff --git a/spa/plugins/bluez5/bluez5-device.c b/spa/plugins/bluez5/bluez5-device.c index 796c73c1c..6546abf4f 100644 --- a/spa/plugins/bluez5/bluez5-device.c +++ b/spa/plugins/bluez5/bluez5-device.c @@ -452,8 +452,7 @@ static int node_offload_set_active(struct node *node, bool active) return res; } -static void get_channels(struct spa_bt_transport *t, bool a2dp_duplex, uint32_t *n_channels, uint32_t *channels, - uint32_t max_channels) +static void get_channels(struct spa_bt_transport *t, bool a2dp_duplex, uint32_t *n_channels, uint32_t *channels) { const struct media_codec *codec; struct spa_audio_info info = { 0 }; @@ -689,7 +688,7 @@ static void emit_node(struct impl *this, struct spa_bt_transport *t, this->nodes[id].active = true; this->nodes[id].offload_acquired = false; this->nodes[id].a2dp_duplex = a2dp_duplex; - get_channels(t, a2dp_duplex, &this->nodes[id].n_channels, this->nodes[id].channels, MAX_CHANNELS); + get_channels(t, a2dp_duplex, &this->nodes[id].n_channels, this->nodes[id].channels); if (this->nodes[id].transport) spa_hook_remove(&this->nodes[id].transport_listener); this->nodes[id].transport = t; diff --git a/src/modules/module-ffado-driver.c b/src/modules/module-ffado-driver.c index de5ed0ff0..7094feaf4 100644 --- a/src/modules/module-ffado-driver.c +++ b/src/modules/module-ffado-driver.c @@ -761,7 +761,7 @@ static int make_stream_ports(struct stream *s) struct port *port = s->ports[i]; char channel[32]; - snprintf(channel, sizeof(channel), "AUX%u", n_channels % MAX_CHANNELS); + snprintf(channel, sizeof(channel), "AUX%u", n_channels); switch (port->stream_type) { case ffado_stream_type_audio: @@ -1229,7 +1229,7 @@ static int probe_ffado_device(struct impl *impl) } if (impl->source.info.channels != n_channels) { uint32_t n_pos = SPA_MIN(n_channels, SPA_N_ELEMENTS(impl->source.info.position)); - impl->source.info.channels = n_channels; + impl->source.info.channels = n_pos; for (i = 0; i < n_pos; i++) impl->source.info.position[i] = SPA_AUDIO_CHANNEL_AUX0 + i; } @@ -1256,7 +1256,7 @@ static int probe_ffado_device(struct impl *impl) } if (impl->sink.info.channels != n_channels) { uint32_t n_pos = SPA_MIN(n_channels, SPA_N_ELEMENTS(impl->sink.info.position)); - impl->sink.info.channels = n_channels; + impl->sink.info.channels = n_pos; for (i = 0; i < n_pos; i++) impl->sink.info.position[i] = SPA_AUDIO_CHANNEL_AUX0 + i; } diff --git a/src/modules/module-filter-chain.c b/src/modules/module-filter-chain.c index 6b759434b..a2d1f5361 100644 --- a/src/modules/module-filter-chain.c +++ b/src/modules/module-filter-chain.c @@ -1691,8 +1691,7 @@ static void copy_position(struct spa_audio_info_raw *dst, const struct spa_audio { if (SPA_FLAG_IS_SET(dst->flags, SPA_AUDIO_FLAG_UNPOSITIONED) && !SPA_FLAG_IS_SET(src->flags, SPA_AUDIO_FLAG_UNPOSITIONED)) { - uint32_t i, n_pos = SPA_MIN(src->channels, SPA_N_ELEMENTS(dst->position)); - for (i = 0; i < n_pos; i++) + for (uint32_t i = 0; i < src->channels; i++) dst->position[i] = src->position[i]; SPA_FLAG_CLEAR(dst->flags, SPA_AUDIO_FLAG_UNPOSITIONED); } diff --git a/src/modules/module-vban/stream.c b/src/modules/module-vban/stream.c index 22ef8bea5..941fa1acd 100644 --- a/src/modules/module-vban/stream.c +++ b/src/modules/module-vban/stream.c @@ -215,16 +215,16 @@ static const struct spa_audio_layout_info layouts[] = { { SPA_AUDIO_LAYOUT_7_1 }, }; -static void default_layout(uint32_t channels, uint32_t *position, uint32_t max_position) +static void default_layout(uint32_t channels, uint32_t *position) { SPA_FOR_EACH_ELEMENT_VAR(layouts, l) { if (l->n_channels == channels) { - for (uint32_t i = 0; i < l->n_channels && i < max_position; i++) + for (uint32_t i = 0; i < l->n_channels; i++) position[i] = l->position[i]; return; } } - for (uint32_t i = 0; i < channels && i < max_position; i++) + for (uint32_t i = 0; i < channels; i++) position[i] = SPA_AUDIO_CHANNEL_AUX0 + i; } @@ -278,10 +278,10 @@ struct vban_stream *vban_stream_new(struct pw_core *core, pw_log_error("can't parse format: %s", spa_strerror(res)); goto out; } - if (SPA_FLAG_IS_SET(impl->info.info.raw.flags, SPA_AUDIO_FLAG_UNPOSITIONED)) - default_layout(impl->info.info.raw.channels, - impl->info.info.raw.position, - SPA_N_ELEMENTS(impl->info.info.raw.position)); + if (SPA_FLAG_IS_SET(impl->info.info.raw.flags, SPA_AUDIO_FLAG_UNPOSITIONED)) { + default_layout(impl->info.info.raw.channels, impl->info.info.raw.position); + SPA_FLAG_CLEAR(impl->info.info.raw.flags, SPA_AUDIO_FLAG_UNPOSITIONED); + } impl->stream_info = impl->info; impl->format_info = find_audio_format_info(&impl->info); if (impl->format_info == NULL) {