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 for Android: free password hash cracker in your pocket

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

Message-ID: <1317c3c5-967d-4013-9c0b-f68844c63908@oss.qualcomm.com>
Date: Wed, 18 Jun 2025 13:33:52 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@....qualcomm.com>
To: Alexey Klimov <alexey.klimov@...aro.org>, 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>
Cc: 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 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.

> 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?

> +	__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.

> +	__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?

> +} __attribute__((packed, aligned(4)));
> +
>  union snd_codec_options {
>  	struct snd_enc_wma wma;
>  	struct snd_enc_vorbis vorbis;
> @@ -334,6 +352,7 @@ union snd_codec_options {
>  	struct snd_dec_wma wma_d;
>  	struct snd_dec_alac alac_d;
>  	struct snd_dec_ape ape_d;
> +	struct snd_dec_opus opus_d;
>  	struct {
>  		__u32 out_sample_rate;
>  	} src_d;
>