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

[Alexey Klimov]

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