linux-kernel - Re: [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec

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] [thread-next>] [day] [month] [year] [list]

Message-Id: <DC7G8QJ0XVJX.BBZHVVZO6OEN@linaro.org>
Date: Wed, 20 Aug 2025 18:59:53 +0100
From: "Alexey Klimov" <alexey.klimov@...aro.org>
To: "Srinivas Kandagatla" <srinivas.kandagatla@....qualcomm.com>
Cc: "Vinod Koul" <vkoul@...nel.org>, "Jaroslav Kysela" <perex@...ex.cz>,
 "Takashi Iwai" <tiwai@...e.com>, "Srinivas Kandagatla" <srini@...nel.org>,
 "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 1/2] ALSA: compress: add raw opus codec define and
 struct snd_dec_opus

On Wed Jun 18, 2025 at 1:33 PM BST, Srinivas Kandagatla wrote:
>
>
> On 6/16/25 4:26 PM, Alexey Klimov wrote:
>> Adds a raw opus codec define and raw opus decoder struct.
>> This is for raw OPUS packets not packed in any type of container
>> (for instance OGG container). The decoder struct fields
>> are taken from corresponding RFC document.
>> 
>> This is based on earlier work done by
>> Annemarie Porter <annemari@...cinc.com>
>> 
> May be co-dev by would be good option.

Ack.

>> Cc: Annemarie Porter <annemari@...cinc.com>
>> Cc: Srinivas Kandagatla <srini@...nel.org>
>> Cc: Vinod Koul <vkoul@...nel.org>
>> Signed-off-by: Alexey Klimov <alexey.klimov@...aro.org>
>> ---
>>  include/uapi/sound/compress_params.h | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>> 
>> diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
>> index bc7648a30746f4632ecf6695868e79550a431dfa..f80989f7bdd2f1bfad843b1dc30fa263e083d17a 100644
>> --- a/include/uapi/sound/compress_params.h
>> +++ b/include/uapi/sound/compress_params.h
>> @@ -43,7 +43,8 @@
>>  #define SND_AUDIOCODEC_BESPOKE               ((__u32) 0x0000000E)
>>  #define SND_AUDIOCODEC_ALAC                  ((__u32) 0x0000000F)
>>  #define SND_AUDIOCODEC_APE                   ((__u32) 0x00000010)
>> -#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_APE
>> +#define SND_AUDIOCODEC_OPUS_RAW              ((__u32) 0x00000011)
>> +#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_OPUS_RAW
>>  
>>  /*
>>   * Profile and modes are listed with bit masks. This allows for a
>> @@ -324,6 +325,23 @@ struct snd_dec_ape {
>>  	__u32 seek_table_present;
>>  } __attribute__((packed, aligned(4)));
>>  
>> +/*
>> + * RFC with info on below OPUS decoder fields:
>> + * https://www.rfc-editor.org/rfc/rfc7845#section-5
>> + */
>> +struct snd_dec_opus {
>> +	__u8 version;		/* must be 1 */
>> +	__u8 num_channels;
>> +	__u16 pre_skip;
>> +	__u32 sample_rate;
>> +	__u16 output_gain;	/* in Q7.8 format */
>> +	__u8 mapping_family;
>
> This is where optional Channel Mapping Table starts in the structure.
>
> Should this all these channel mapping memnbers go into a dedicated
> struct snd_dec_opus_ch_map?

Ok.

>> +	__u8 stream_count;	/* part of channel mapping */
>> +	__u8 coupled_count;	/* part of channel mapping */
> Comments are bit misleading. Either we document them in detail or point to
> the rfc which has this documented in more detail.

Ok.

>> +	__u8 channel_map;
>
> Channel Mapping is (8*C bits), one octet per output channel.
>
> The way this is represented/split in this struct is confusing should it
> be just channel_map[8]
>
>
>> +	__u8 reserved[7];	/* space for channel mapping */
> Any reason only 7?

It was 7 because 1+7=8. For RFC 7845 the mappin family 0 and 1 only
actually make sense for playback hence max number of channels seems to
be 8.
I'll update it in the next version.

Thank you,
Alexey Klimov