linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/2] Add raw OPUS codec support for compress offload
@ 2025-06-16 15:26 Alexey Klimov
  2025-06-16 15:26 ` [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus Alexey Klimov
  2025-06-16 15:26 ` [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback Alexey Klimov
  0 siblings, 2 replies; 9+ messages in thread
From: Alexey Klimov @ 2025-06-16 15:26 UTC (permalink / raw)
  To: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla,
	Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Alexey Klimov, Pierre-Louis Bossart

This series adds support in kernel to recognise raw (or plain) OPUS
codec playback for compress offloading. At this point this series
doesn't deal with OPUS packets packed in any kind of containers (OGG or
others) and focuses on adding missing bits for pure OPUS packets.

The second patch adds its usage in Qualcomm Audio DSP code. To correctly
recognise raw OPUS packets by qdsp6, each packets needs to be prepended
with 4-bytes field that contains length of a raw OPUS packet.
It is expected to be useful for usecases when OPUS packets are streamed
over network and they are not encapsulated in a container. Userspace
application that will use the compress API has to manually add such
4-bytes long field to each OPUS packet.

This is tested on sm8750-mtp. It is expected that next hardware revisions
will also support raw OPUS codec offloading.

Dependencies are:
-- hardware with DSP that supports decoding OPUS packets (>=sm8750);
-- adsp fastrpc for sm8750;
-- explicitly setting format in sm8750 soundcard driver;
-- running adsprpcd tool with support for Audio PD and DSP libraries
loading support (or its alternative);
-- tinycompress fcplay tool that will prepare raw opus packets and
do the required addition of length field.

The userspace tinycompress tool with support for raw OPUS compress
playback is located here:
https://github.com/laklimov/tinycompress_opus
branch: opus_v3_workinprogress

It is not expected that it is ready and still needs some work. More like PoC.

Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
Alexey Klimov (2):
      ALSA: compress: add raw opus codec define and struct snd_dec_opus
      ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback

 include/uapi/sound/compress_params.h | 21 ++++++++++++++++++++-
 sound/soc/qcom/qdsp6/audioreach.c    | 33 +++++++++++++++++++++++++++++++++
 sound/soc/qcom/qdsp6/audioreach.h    | 17 +++++++++++++++++
 sound/soc/qcom/qdsp6/q6apm-dai.c     |  3 ++-
 sound/soc/qcom/qdsp6/q6apm.c         |  3 +++
 5 files changed, 75 insertions(+), 2 deletions(-)
---
base-commit: 050f8ad7b58d9079455af171ac279c4b9b828c11
change-id: 20250616-opus_codec_rfc_v1-b60bd308893b

Best regards,
-- 
Alexey Klimov <alexey.klimov@linaro.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus
  2025-06-16 15:26 [PATCH RFC 0/2] Add raw OPUS codec support for compress offload Alexey Klimov
@ 2025-06-16 15:26 ` Alexey Klimov
  2025-06-18 12:33   ` Srinivas Kandagatla
  2025-06-16 15:26 ` [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback Alexey Klimov
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Klimov @ 2025-06-16 15:26 UTC (permalink / raw)
  To: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla,
	Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Alexey Klimov, Pierre-Louis Bossart

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@quicinc.com>

Cc: Annemarie Porter <annemari@quicinc.com>
Cc: Srinivas Kandagatla <srini@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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;
+	__u8 stream_count;	/* part of channel mapping */
+	__u8 coupled_count;	/* part of channel mapping */
+	__u8 channel_map;
+	__u8 reserved[7];	/* space for channel mapping */
+} __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;

-- 
2.47.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback
  2025-06-16 15:26 [PATCH RFC 0/2] Add raw OPUS codec support for compress offload Alexey Klimov
  2025-06-16 15:26 ` [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus Alexey Klimov
@ 2025-06-16 15:26 ` Alexey Klimov
  2025-06-18 12:34   ` Srinivas Kandagatla
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Klimov @ 2025-06-16 15:26 UTC (permalink / raw)
  To: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla,
	Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Alexey Klimov, Pierre-Louis Bossart

Add support for OPUS module, OPUS format ID, media format payload struct
and make it all recognizable by audioreach compress playback path.

At this moment this only supports raw or plain OPUS packets not
encapsulated in container (for instance OGG container). For this usecase
each OPUS packet needs to be prepended with 4-bytes long length field
which is expected to be done by userspace applications. This is
Qualcomm DSP specific requirement.

This patch is based on earlier work done by
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Cc: Annemarie Porter <annemari@quicinc.com>
Cc: Srinivas Kandagatla <srini@kernel.org>
Cc: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
---
 sound/soc/qcom/qdsp6/audioreach.c | 33 +++++++++++++++++++++++++++++++++
 sound/soc/qcom/qdsp6/audioreach.h | 17 +++++++++++++++++
 sound/soc/qcom/qdsp6/q6apm-dai.c  |  3 ++-
 sound/soc/qcom/qdsp6/q6apm.c      |  3 +++
 4 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
index 4ebaaf736fb98a5a8a58d06416b3ace2504856e1..09e3a4da945d61b6915bf8b6f382c25ae94c5888 100644
--- a/sound/soc/qcom/qdsp6/audioreach.c
+++ b/sound/soc/qcom/qdsp6/audioreach.c
@@ -859,6 +859,7 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
 	struct payload_media_fmt_aac_t *aac_cfg;
 	struct payload_media_fmt_pcm *mp3_cfg;
 	struct payload_media_fmt_flac_t *flac_cfg;
+	struct payload_media_fmt_opus_t *opus_cfg;
 
 	switch (mcfg->fmt) {
 	case SND_AUDIOCODEC_MP3:
@@ -901,6 +902,38 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
 		flac_cfg->min_frame_size = mcfg->codec.options.flac_d.min_frame_size;
 		flac_cfg->max_frame_size = mcfg->codec.options.flac_d.max_frame_size;
 		break;
+	case SND_AUDIOCODEC_OPUS_RAW:
+		media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
+		media_fmt_hdr->fmt_id = MEDIA_FMT_ID_OPUS;
+		media_fmt_hdr->payload_size = sizeof(struct payload_media_fmt_opus_t);
+		p = p + sizeof(*media_fmt_hdr);
+		opus_cfg = p;
+		/* raw opus packets prepended with 4 bytes of length */
+		opus_cfg->bitstream_format = 1;
+		/*
+		 * payload_type:
+		 * 0 -- read metadata from opus stream;
+		 * 1 -- metadata is provided by filling in the struct here.
+		 */
+		opus_cfg->payload_type = 1;
+		opus_cfg->version = mcfg->codec.options.opus_d.version;
+		opus_cfg->num_channels = mcfg->codec.options.opus_d.num_channels;
+		opus_cfg->pre_skip = mcfg->codec.options.opus_d.pre_skip;
+		opus_cfg->sample_rate = mcfg->codec.options.opus_d.sample_rate;
+		opus_cfg->output_gain = mcfg->codec.options.opus_d.output_gain;
+		opus_cfg->mapping_family = mcfg->codec.options.opus_d.mapping_family;
+		opus_cfg->stream_count = mcfg->codec.options.opus_d.stream_count;
+		opus_cfg->coupled_count = mcfg->codec.options.opus_d.coupled_count;
+
+		if (mcfg->codec.options.opus_d.num_channels == 1) {
+			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
+		} else if (mcfg->codec.options.opus_d.num_channels == 2) {
+			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
+			opus_cfg->channel_mapping[1] = PCM_CHANNEL_FR;
+		}
+
+		opus_cfg->reserved[0] = opus_cfg->reserved[1] = opus_cfg->reserved[2] = 0;
+		break;
 	default:
 		return -EINVAL;
 	}
diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
index 61a69df4f50f6cc90b56697c272ade6f1411bbf2..512ea24fd402c95f98db790313b756a5ba3dcca1 100644
--- a/sound/soc/qcom/qdsp6/audioreach.h
+++ b/sound/soc/qcom/qdsp6/audioreach.h
@@ -29,6 +29,7 @@ struct q6apm_graph;
 #define MODULE_ID_MP3_DECODE		0x0700103B
 #define MODULE_ID_GAPLESS		0x0700104D
 #define MODULE_ID_DISPLAY_PORT_SINK	0x07001069
+#define MODULE_ID_OPUS_DEC		0x07001174
 
 #define APM_CMD_GET_SPF_STATE		0x01001021
 #define APM_CMD_RSP_GET_SPF_STATE	0x02001007
@@ -255,6 +256,22 @@ struct payload_media_fmt_aac_t {
 	uint32_t sample_rate;
 } __packed;
 
+#define MEDIA_FMT_ID_OPUS	0x09001039
+struct payload_media_fmt_opus_t {
+	uint16_t bitstream_format;
+	uint16_t payload_type;
+	uint8_t version;
+	uint8_t num_channels;
+	uint16_t pre_skip;
+	uint32_t sample_rate;
+	uint16_t output_gain;
+	uint8_t mapping_family;
+	uint8_t stream_count;
+	uint8_t coupled_count;
+	uint8_t channel_mapping[8];
+	uint8_t reserved[3];
+} __packed;
+
 #define DATA_CMD_WR_SH_MEM_EP_EOS			0x04001002
 #define WR_SH_MEM_EP_EOS_POLICY_LAST	1
 #define WR_SH_MEM_EP_EOS_POLICY_EACH	2
diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
index 2cd522108221a2ec5c7bbbb63f7d4ae4f8001cf6..7da91ed297f4a5ed39ca0747804cabd579634b50 100644
--- a/sound/soc/qcom/qdsp6/q6apm-dai.c
+++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
@@ -550,10 +550,11 @@ static int q6apm_dai_compr_get_caps(struct snd_soc_component *component,
 	caps->max_fragment_size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE;
 	caps->min_fragments = COMPR_PLAYBACK_MIN_NUM_FRAGMENTS;
 	caps->max_fragments = COMPR_PLAYBACK_MAX_NUM_FRAGMENTS;
-	caps->num_codecs = 3;
+	caps->num_codecs = 4;
 	caps->codecs[0] = SND_AUDIOCODEC_MP3;
 	caps->codecs[1] = SND_AUDIOCODEC_AAC;
 	caps->codecs[2] = SND_AUDIOCODEC_FLAC;
+	caps->codecs[3] = SND_AUDIOCODEC_OPUS_RAW;
 
 	return 0;
 }
diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
index b4ffa0f0b188e2c32fdfb863b9130d1d11e578dd..0e667a7eb5467bdd65326099132e8ba9dfefa21e 100644
--- a/sound/soc/qcom/qdsp6/q6apm.c
+++ b/sound/soc/qcom/qdsp6/q6apm.c
@@ -354,6 +354,9 @@ int q6apm_set_real_module_id(struct device *dev, struct q6apm_graph *graph,
 	case SND_AUDIOCODEC_FLAC:
 		module_id = MODULE_ID_FLAC_DEC;
 		break;
+	case SND_AUDIOCODEC_OPUS_RAW:
+		module_id = MODULE_ID_OPUS_DEC;
+		break;
 	default:
 		return -EINVAL;
 	}

-- 
2.47.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus
  2025-06-16 15:26 ` [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus Alexey Klimov
@ 2025-06-18 12:33   ` Srinivas Kandagatla
  2025-08-20 17:59     ` Alexey Klimov
  0 siblings, 1 reply; 9+ messages in thread
From: Srinivas Kandagatla @ 2025-06-18 12:33 UTC (permalink / raw)
  To: Alexey Klimov, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
	Srinivas Kandagatla, Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Pierre-Louis Bossart



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@quicinc.com>
> 
May be co-dev by would be good option.

> Cc: Annemarie Porter <annemari@quicinc.com>
> Cc: Srinivas Kandagatla <srini@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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;
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback
  2025-06-16 15:26 ` [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback Alexey Klimov
@ 2025-06-18 12:34   ` Srinivas Kandagatla
  2025-07-03 14:33     ` Alexey Klimov
  2025-08-20 17:56     ` Alexey Klimov
  0 siblings, 2 replies; 9+ messages in thread
From: Srinivas Kandagatla @ 2025-06-18 12:34 UTC (permalink / raw)
  To: Alexey Klimov, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
	Srinivas Kandagatla, Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Pierre-Louis Bossart



On 6/16/25 4:26 PM, Alexey Klimov wrote:
> Add support for OPUS module, OPUS format ID, media format payload struct
> and make it all recognizable by audioreach compress playback path.
> 
> At this moment this only supports raw or plain OPUS packets not
> encapsulated in container (for instance OGG container). For this usecase
> each OPUS packet needs to be prepended with 4-bytes long length field
> which is expected to be done by userspace applications. This is
> Qualcomm DSP specific requirement.
> > This patch is based on earlier work done by
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org>

Thanks for picking this up Alexey,

Same, co-dev by should be good attribute for such things.


> 
> Cc: Annemarie Porter <annemari@quicinc.com>
> Cc: Srinivas Kandagatla <srini@kernel.org>
> Cc: Vinod Koul <vkoul@kernel.org>
> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
> ---
>  sound/soc/qcom/qdsp6/audioreach.c | 33 +++++++++++++++++++++++++++++++++
>  sound/soc/qcom/qdsp6/audioreach.h | 17 +++++++++++++++++
>  sound/soc/qcom/qdsp6/q6apm-dai.c  |  3 ++-
>  sound/soc/qcom/qdsp6/q6apm.c      |  3 +++
>  4 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
> index 4ebaaf736fb98a5a8a58d06416b3ace2504856e1..09e3a4da945d61b6915bf8b6f382c25ae94c5888 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.c
> +++ b/sound/soc/qcom/qdsp6/audioreach.c
> @@ -859,6 +859,7 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>  	struct payload_media_fmt_aac_t *aac_cfg;
>  	struct payload_media_fmt_pcm *mp3_cfg;
>  	struct payload_media_fmt_flac_t *flac_cfg;
> +	struct payload_media_fmt_opus_t *opus_cfg;
>  
>  	switch (mcfg->fmt) {
>  	case SND_AUDIOCODEC_MP3:
> @@ -901,6 +902,38 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>  		flac_cfg->min_frame_size = mcfg->codec.options.flac_d.min_frame_size;
>  		flac_cfg->max_frame_size = mcfg->codec.options.flac_d.max_frame_size;
>  		break;
> +	case SND_AUDIOCODEC_OPUS_RAW:
> +		media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
> +		media_fmt_hdr->fmt_id = MEDIA_FMT_ID_OPUS;
> +		media_fmt_hdr->payload_size = sizeof(struct payload_media_fmt_opus_t);

maybe sizeof(*opus_cfg)?

> +		p = p + sizeof(*media_fmt_hdr);
> +		opus_cfg = p;
> +		/* raw opus packets prepended with 4 bytes of length */
> +		opus_cfg->bitstream_format = 1;
> +		/*
> +		 * payload_type:
> +		 * 0 -- read metadata from opus stream;
> +		 * 1 -- metadata is provided by filling in the struct here.
> +		 */
> +		opus_cfg->payload_type = 1;
> +		opus_cfg->version = mcfg->codec.options.opus_d.version;
> +		opus_cfg->num_channels = mcfg->codec.options.opus_d.num_channels;
> +		opus_cfg->pre_skip = mcfg->codec.options.opus_d.pre_skip;
> +		opus_cfg->sample_rate = mcfg->codec.options.opus_d.sample_rate;
> +		opus_cfg->output_gain = mcfg->codec.options.opus_d.output_gain;
> +		opus_cfg->mapping_family = mcfg->codec.options.opus_d.mapping_family;
> +		opus_cfg->stream_count = mcfg->codec.options.opus_d.stream_count;
> +		opus_cfg->coupled_count = mcfg->codec.options.opus_d.coupled_count;
> +
> +		if (mcfg->codec.options.opus_d.num_channels == 1) {
> +			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
> +		} else if (mcfg->codec.options.opus_d.num_channels == 2) {
> +			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
> +			opus_cfg->channel_mapping[1] = PCM_CHANNEL_FR;
> +		}

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?


> +
> +		opus_cfg->reserved[0] = opus_cfg->reserved[1] = opus_cfg->reserved[2] = 0;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/sound/soc/qcom/qdsp6/audioreach.h b/sound/soc/qcom/qdsp6/audioreach.h
> index 61a69df4f50f6cc90b56697c272ade6f1411bbf2..512ea24fd402c95f98db790313b756a5ba3dcca1 100644
> --- a/sound/soc/qcom/qdsp6/audioreach.h
> +++ b/sound/soc/qcom/qdsp6/audioreach.h
> @@ -29,6 +29,7 @@ struct q6apm_graph;
>  #define MODULE_ID_MP3_DECODE		0x0700103B
>  #define MODULE_ID_GAPLESS		0x0700104D
>  #define MODULE_ID_DISPLAY_PORT_SINK	0x07001069
> +#define MODULE_ID_OPUS_DEC		0x07001174
>  
>  #define APM_CMD_GET_SPF_STATE		0x01001021
>  #define APM_CMD_RSP_GET_SPF_STATE	0x02001007
> @@ -255,6 +256,22 @@ struct payload_media_fmt_aac_t {
>  	uint32_t sample_rate;
>  } __packed;
>  
> +#define MEDIA_FMT_ID_OPUS	0x09001039
> +struct payload_media_fmt_opus_t {
> +	uint16_t bitstream_format;
> +	uint16_t payload_type;
> +	uint8_t version;
> +	uint8_t num_channels;
> +	uint16_t pre_skip;
> +	uint32_t sample_rate;
> +	uint16_t output_gain;
> +	uint8_t mapping_family;
> +	uint8_t stream_count;
> +	uint8_t coupled_count;
> +	uint8_t channel_mapping[8];
> +	uint8_t reserved[3];
> +} __packed;
> +
>  #define DATA_CMD_WR_SH_MEM_EP_EOS			0x04001002
>  #define WR_SH_MEM_EP_EOS_POLICY_LAST	1
>  #define WR_SH_MEM_EP_EOS_POLICY_EACH	2
> diff --git a/sound/soc/qcom/qdsp6/q6apm-dai.c b/sound/soc/qcom/qdsp6/q6apm-dai.c
> index 2cd522108221a2ec5c7bbbb63f7d4ae4f8001cf6..7da91ed297f4a5ed39ca0747804cabd579634b50 100644
> --- a/sound/soc/qcom/qdsp6/q6apm-dai.c
> +++ b/sound/soc/qcom/qdsp6/q6apm-dai.c
> @@ -550,10 +550,11 @@ static int q6apm_dai_compr_get_caps(struct snd_soc_component *component,
>  	caps->max_fragment_size = COMPR_PLAYBACK_MAX_FRAGMENT_SIZE;
>  	caps->min_fragments = COMPR_PLAYBACK_MIN_NUM_FRAGMENTS;
>  	caps->max_fragments = COMPR_PLAYBACK_MAX_NUM_FRAGMENTS;
> -	caps->num_codecs = 3;
> +	caps->num_codecs = 4;
>  	caps->codecs[0] = SND_AUDIOCODEC_MP3;
>  	caps->codecs[1] = SND_AUDIOCODEC_AAC;
>  	caps->codecs[2] = SND_AUDIOCODEC_FLAC;
> +	caps->codecs[3] = SND_AUDIOCODEC_OPUS_RAW;
>  
>  	return 0;
>  }
> diff --git a/sound/soc/qcom/qdsp6/q6apm.c b/sound/soc/qcom/qdsp6/q6apm.c
> index b4ffa0f0b188e2c32fdfb863b9130d1d11e578dd..0e667a7eb5467bdd65326099132e8ba9dfefa21e 100644
> --- a/sound/soc/qcom/qdsp6/q6apm.c
> +++ b/sound/soc/qcom/qdsp6/q6apm.c
> @@ -354,6 +354,9 @@ int q6apm_set_real_module_id(struct device *dev, struct q6apm_graph *graph,
>  	case SND_AUDIOCODEC_FLAC:
>  		module_id = MODULE_ID_FLAC_DEC;
>  		break;
> +	case SND_AUDIOCODEC_OPUS_RAW:
> +		module_id = MODULE_ID_OPUS_DEC;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback
  2025-06-18 12:34   ` Srinivas Kandagatla
@ 2025-07-03 14:33     ` Alexey Klimov
  2025-08-20 18:04       ` Alexey Klimov
  2025-08-20 17:56     ` Alexey Klimov
  1 sibling, 1 reply; 9+ messages in thread
From: Alexey Klimov @ 2025-07-03 14:33 UTC (permalink / raw)
  To: Srinivas Kandagatla, Vinod Koul, Jaroslav Kysela, Takashi Iwai,
	Srinivas Kandagatla, Liam Girdwood, Mark Brown
  Cc: Patrick Lai, Annemarie Porter, linux-sound, linux-kernel,
	linux-arm-msm, Krzysztof Kozlowski, kernel, Ekansh Gupta,
	Pierre-Louis Bossart

On Wed Jun 18, 2025 at 1:34 PM BST, Srinivas Kandagatla wrote:
>
>
> On 6/16/25 4:26 PM, Alexey Klimov wrote:
>> Add support for OPUS module, OPUS format ID, media format payload struct
>> and make it all recognizable by audioreach compress playback path.
>> 
>> At this moment this only supports raw or plain OPUS packets not
>> encapsulated in container (for instance OGG container). For this usecase
>> each OPUS packet needs to be prepended with 4-bytes long length field
>> which is expected to be done by userspace applications. This is
>> Qualcomm DSP specific requirement.
>> > This patch is based on earlier work done by
>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Thanks for picking this up Alexey,
>
> Same, co-dev by should be good attribute for such things.

I need your Signed-off-by then and/or permission to use your Sign off.

>> Cc: Annemarie Porter <annemari@quicinc.com>
>> Cc: Srinivas Kandagatla <srini@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>>  sound/soc/qcom/qdsp6/audioreach.c | 33 +++++++++++++++++++++++++++++++++
>>  sound/soc/qcom/qdsp6/audioreach.h | 17 +++++++++++++++++
>>  sound/soc/qcom/qdsp6/q6apm-dai.c  |  3 ++-
>>  sound/soc/qcom/qdsp6/q6apm.c      |  3 +++
>>  4 files changed, 55 insertions(+), 1 deletion(-)
>> 
>> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
>> index 4ebaaf736fb98a5a8a58d06416b3ace2504856e1..09e3a4da945d61b6915bf8b6f382c25ae94c5888 100644
>> --- a/sound/soc/qcom/qdsp6/audioreach.c
>> +++ b/sound/soc/qcom/qdsp6/audioreach.c
>> @@ -859,6 +859,7 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>>  	struct payload_media_fmt_aac_t *aac_cfg;
>>  	struct payload_media_fmt_pcm *mp3_cfg;
>>  	struct payload_media_fmt_flac_t *flac_cfg;
>> +	struct payload_media_fmt_opus_t *opus_cfg;
>>  
>>  	switch (mcfg->fmt) {
>>  	case SND_AUDIOCODEC_MP3:
>> @@ -901,6 +902,38 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>>  		flac_cfg->min_frame_size = mcfg->codec.options.flac_d.min_frame_size;
>>  		flac_cfg->max_frame_size = mcfg->codec.options.flac_d.max_frame_size;
>>  		break;
>> +	case SND_AUDIOCODEC_OPUS_RAW:
>> +		media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
>> +		media_fmt_hdr->fmt_id = MEDIA_FMT_ID_OPUS;
>> +		media_fmt_hdr->payload_size = sizeof(struct payload_media_fmt_opus_t);
>
> maybe sizeof(*opus_cfg)?

Yes, I can update that.

>> +		p = p + sizeof(*media_fmt_hdr);
>> +		opus_cfg = p;
>> +		/* raw opus packets prepended with 4 bytes of length */
>> +		opus_cfg->bitstream_format = 1;
>> +		/*
>> +		 * payload_type:
>> +		 * 0 -- read metadata from opus stream;
>> +		 * 1 -- metadata is provided by filling in the struct here.
>> +		 */
>> +		opus_cfg->payload_type = 1;
>> +		opus_cfg->version = mcfg->codec.options.opus_d.version;
>> +		opus_cfg->num_channels = mcfg->codec.options.opus_d.num_channels;
>> +		opus_cfg->pre_skip = mcfg->codec.options.opus_d.pre_skip;
>> +		opus_cfg->sample_rate = mcfg->codec.options.opus_d.sample_rate;
>> +		opus_cfg->output_gain = mcfg->codec.options.opus_d.output_gain;
>> +		opus_cfg->mapping_family = mcfg->codec.options.opus_d.mapping_family;
>> +		opus_cfg->stream_count = mcfg->codec.options.opus_d.stream_count;
>> +		opus_cfg->coupled_count = mcfg->codec.options.opus_d.coupled_count;
>> +
>> +		if (mcfg->codec.options.opus_d.num_channels == 1) {
>> +			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
>> +		} else if (mcfg->codec.options.opus_d.num_channels == 2) {
>> +			opus_cfg->channel_mapping[0] = PCM_CHANNEL_FL;
>> +			opus_cfg->channel_mapping[1] = PCM_CHANNEL_FR;
>> +		}
>
> 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.

Also, I don't have any test files to test mapping_family 1 and some tricky
cases here. As far as I understand, it works just fine right now with
mapping_family 0.

Best regards,
Alexey


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback
  2025-06-18 12:34   ` Srinivas Kandagatla
  2025-07-03 14:33     ` Alexey Klimov
@ 2025-08-20 17:56     ` Alexey Klimov
  1 sibling, 0 replies; 9+ messages in thread
From: Alexey Klimov @ 2025-08-20 17:56 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla,
	Liam Girdwood, Mark Brown, Patrick Lai, Annemarie Porter,
	linux-sound, linux-kernel, linux-arm-msm, Krzysztof Kozlowski,
	kernel, Ekansh Gupta, Pierre-Louis Bossart

On Wed Jun 18, 2025 at 1:34 PM BST, Srinivas Kandagatla wrote:
>
>
> On 6/16/25 4:26 PM, Alexey Klimov wrote:
>> Add support for OPUS module, OPUS format ID, media format payload struct
>> and make it all recognizable by audioreach compress playback path.
>> 
>> At this moment this only supports raw or plain OPUS packets not
>> encapsulated in container (for instance OGG container). For this usecase
>> each OPUS packet needs to be prepended with 4-bytes long length field
>> which is expected to be done by userspace applications. This is
>> Qualcomm DSP specific requirement.
>> > This patch is based on earlier work done by
>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>
> Thanks for picking this up Alexey,
>
> Same, co-dev by should be good attribute for such things.

Thanks. I'll update it for the next version.

>> Cc: Annemarie Porter <annemari@quicinc.com>
>> Cc: Srinivas Kandagatla <srini@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.org>
>> ---
>>  sound/soc/qcom/qdsp6/audioreach.c | 33 +++++++++++++++++++++++++++++++++
>>  sound/soc/qcom/qdsp6/audioreach.h | 17 +++++++++++++++++
>>  sound/soc/qcom/qdsp6/q6apm-dai.c  |  3 ++-
>>  sound/soc/qcom/qdsp6/q6apm.c      |  3 +++
>>  4 files changed, 55 insertions(+), 1 deletion(-)
>> 
>> diff --git a/sound/soc/qcom/qdsp6/audioreach.c b/sound/soc/qcom/qdsp6/audioreach.c
>> index 4ebaaf736fb98a5a8a58d06416b3ace2504856e1..09e3a4da945d61b6915bf8b6f382c25ae94c5888 100644
>> --- a/sound/soc/qcom/qdsp6/audioreach.c
>> +++ b/sound/soc/qcom/qdsp6/audioreach.c
>> @@ -859,6 +859,7 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>>  	struct payload_media_fmt_aac_t *aac_cfg;
>>  	struct payload_media_fmt_pcm *mp3_cfg;
>>  	struct payload_media_fmt_flac_t *flac_cfg;
>> +	struct payload_media_fmt_opus_t *opus_cfg;
>>  
>>  	switch (mcfg->fmt) {
>>  	case SND_AUDIOCODEC_MP3:
>> @@ -901,6 +902,38 @@ static int audioreach_set_compr_media_format(struct media_format *media_fmt_hdr,
>>  		flac_cfg->min_frame_size = mcfg->codec.options.flac_d.min_frame_size;
>>  		flac_cfg->max_frame_size = mcfg->codec.options.flac_d.max_frame_size;
>>  		break;
>> +	case SND_AUDIOCODEC_OPUS_RAW:
>> +		media_fmt_hdr->data_format = DATA_FORMAT_RAW_COMPRESSED;
>> +		media_fmt_hdr->fmt_id = MEDIA_FMT_ID_OPUS;
>> +		media_fmt_hdr->payload_size = sizeof(struct payload_media_fmt_opus_t);
>
> maybe sizeof(*opus_cfg)?

Ack.

Best regards,
Alexey

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus
  2025-06-18 12:33   ` Srinivas Kandagatla
@ 2025-08-20 17:59     ` Alexey Klimov
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Klimov @ 2025-08-20 17:59 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Srinivas Kandagatla,
	Liam Girdwood, Mark Brown, Patrick Lai, Annemarie Porter,
	linux-sound, linux-kernel, linux-arm-msm, Krzysztof Kozlowski,
	kernel, Ekansh Gupta, Pierre-Louis Bossart

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@quicinc.com>
>> 
> May be co-dev by would be good option.

Ack.

>> Cc: Annemarie Porter <annemari@quicinc.com>
>> Cc: Srinivas Kandagatla <srini@kernel.org>
>> Cc: Vinod Koul <vkoul@kernel.org>
>> Signed-off-by: Alexey Klimov <alexey.klimov@linaro.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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback
  2025-07-03 14:33     ` Alexey Klimov
@ 2025-08-20 18:04       ` Alexey Klimov
  0 siblings, 0 replies; 9+ messages in thread
From: Alexey Klimov @ 2025-08-20 18:04 UTC (permalink / raw)
  To: Srinivas Kandagatla, Srinivas Kandagatla
  Cc: Vinod Koul, Jaroslav Kysela, Takashi Iwai, Liam Girdwood,
	Mark Brown, Patrick Lai, Annemarie Porter, linux-sound,
	linux-kernel, linux-arm-msm, Krzysztof Kozlowski, kernel,
	Ekansh Gupta, Pierre-Louis Bossart

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-08-20 18:04 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-16 15:26 [PATCH RFC 0/2] Add raw OPUS codec support for compress offload Alexey Klimov
2025-06-16 15:26 ` [PATCH RFC 1/2] ALSA: compress: add raw opus codec define and struct snd_dec_opus Alexey Klimov
2025-06-18 12:33   ` Srinivas Kandagatla
2025-08-20 17:59     ` Alexey Klimov
2025-06-16 15:26 ` [PATCH RFC 2/2] ASoC: qcom: qdsp6/audioreach: add support for offloading raw opus playback Alexey Klimov
2025-06-18 12:34   ` Srinivas Kandagatla
2025-07-03 14:33     ` Alexey Klimov
2025-08-20 18:04       ` Alexey Klimov
2025-08-20 17:56     ` Alexey Klimov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).