linux-kernel - Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <DC7GC0UEKZDM.2C8KIM6TFLMHK@linaro.org>
Date: Wed, 20 Aug 2025 19:04:10 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "Srinivas Kandagatla" <srinivas.kandagatla@....qualcomm.com>, "Srinivas
 Kandagatla" <srini@...nel.org>
Cc: "Vinod Koul" <vkoul@...nel.org>, "Jaroslav Kysela" <perex@...ex.cz>,
 "Takashi Iwai" <tiwai@...e.com>, "Liam Girdwood" <lgirdwood@...il.com>,
 "Mark Brown" <broonie@...nel.org>, "Patrick Lai" <plai@....qualcomm.com>,
 "Annemarie Porter" <annemari@...cinc.com>, <linux-sound@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>, "Krzysztof
 Kozlowski" <krzysztof.kozlowski@...aro.org>, <kernel@....qualcomm.com>,
 "Ekansh Gupta" <ekansh.gupta@....qualcomm.com>, "Pierre-Louis Bossart"
 <pierre-louis.bossart@...ux.dev>
Subject: Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for
 offloading raw opus playback

On Thu Jul 3, 2025 at 3:33 PM BST, Alexey Klimov wrote:
> On Wed Jun 18, 2025 at 1:34 PM BST, Srinivas Kandagatla wrote:

[...]

>> Pl use audioreach_set_default_channel_mapping() to fill in the channel
>> mapping data.
>>
>> Why are we not using channel mapping info from the snd_dec_opus struct here?
>
> Okay, I was re-reading RFC and can't really get my head around this.
>
> So first I came up with something like this:
>
> 	switch (opus_cfg->mapping_family) {
> 	case 0:
> 		if (num_chan == 1 || num_chan == 2)
> 			audioreach_set_default_channel_mapping(ch_map, num_chan);
> 		else
> 			/* mapping family 0 allows only 1 or 2 channels */
> 			return -EINVAL;
> 			break;
> 		case 1:
> 			if (num_chan > 8)
> 				return -EINVAL;
> 			if (mcfg->codec.options.opus_d.coupled_count > mcfg->codec.options.opus_d.stream_count)
> 				return -EINVAL;
>
> 			memcpy(ch_map, mcfg->codec.options.opus_d.channel_map, sizeof(mcfg->codec.options.opus_d.channel_map));
> 			break;
> 		default:
> 			/* mapping family 2..255 shouldn't be allowed to playback */
> 			return -EOPNOTSUPP;
> 		}
>
> but I don't think above is correct at all.
>
> After re-reading the RFC few more times. It looks that channel_mapping in
> opus struct has nothing to do with channel mapping that we need to provide
> to DSP here. The channel mapping maps "decoded" channels to output channels
> and seems to be needed by opus decoder logic and in some sense is internal
> thingy to correctly construct sound for output channel from opus stream(s).
> In other words if output channel is present and valid then channel_mapping
> describes how and which decoded stream or streams (coupled or uncoupled)
> to use for producing sound output for that output channel.
> This is described in https://www.rfc-editor.org/rfc/rfc7845#section-5.1.1
>
> The number of output channels here actually matters for us. We can construct
> mapping for channels that we pass to DSP based just only on the number of
> output channels here and let DSP to figure out how to scatter or downmix or
> upmix them to its own output channels.
>
> Conclusion from my understanding:
> -- we shouldn't mess with opus_cfg->channel_mapping here, it is needed for
> the correct operation of decoder, we shouldn't call
> audioreach_set_default_channel_mapping() on it;
> -- mapping output channels to provide the mapping to DSP might require some
> rework or I need to look into this.
>
> Or something else that didn't came up in my mind yet.

As discussed during out-of-list chats I'll update it to include mfc module in
compress-playback path that should handle the mapping to output channels.
I already have a change for that locally and it seems to work, at least it
doesn't break playback.

Thanks,
Alexey