Re: [RFC PATCH 1/6] ALSA: compress: add Sample Rate Converter codec support

All-

"The sample rate converter is not an encoder ..."

Indeed, an encoder creates a compressed bitstream from audio data  
(typically linear PCM samples), normally for transmission of some  
type. A sample rate converter generates audio data from audio data,  
and is normally applied prior to an encoder because it can only accept  
a limited range of sample rates.

-Jeff

Quoting Pierre-Louis Bossart [off-list ref]:

On 8/6/24 12:26, Shengjiu Wang wrote:

quoted

Add Sample Rate Converter(SRC) codec support, define the output
format and rate for SRC.

Signed-off-by: Shengjiu Wang <redacted>
---
 include/uapi/sound/compress_offload.h | 2 ++
 include/uapi/sound/compress_params.h  | 9 ++++++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/uapi/sound/compress_offload.h  

b/include/uapi/sound/compress_offload.h
index 98772b0cbcb7..8b2b72f94e26 100644

--- a/include/uapi/sound/compress_offload.h
+++ b/include/uapi/sound/compress_offload.h

@@ -112,10 +112,12 @@ struct snd_compr_codec_caps {
  * end of the track
  * @SNDRV_COMPRESS_ENCODER_DELAY: no of samples inserted by the  

encoder at the
  * beginning of the track
+ * @SNDRV_COMPRESS_SRC_RATIO_MOD: Resampling Ratio Modifier for  
sample rate converter
  */
 enum sndrv_compress_encoder {
 	SNDRV_COMPRESS_ENCODER_PADDING = 1,
 	SNDRV_COMPRESS_ENCODER_DELAY = 2,
+	SNDRV_COMPRESS_SRC_RATIO_MOD = 3,
 };

this sounds wrong to me. The sample rate converter is not an "encoder",
and the properties for padding/delay are totally specific to an encoder
function.

The other point is that I am not sure how standard this ratio_mod
parameter is. This could be totally specific to a specific
implementation, and another ASRC might have different parameters.

quoted

 /**

diff --git a/include/uapi/sound/compress_params.h  

b/include/uapi/sound/compress_params.h
index ddc77322d571..0843773ea6b4 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_SRC                   ((__u32) 0x00000011)
+#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_SRC

I am not sure this is wise to change such definitions?

quoted

 /*
  * Profile and modes are listed with bit masks. This allows for a

@@ -324,6 +325,11 @@ struct snd_dec_ape {
 	__u32 seek_table_present;
 } __attribute__((packed, aligned(4)));

+struct snd_dec_src {
+	__u32 format_out;
+	__u32 rate_out;
+} __attribute__((packed, aligned(4)));

Again I am not sure how standard those parameters are, and even if they
were if their representation is reusable.

And the compressed API has a good split between encoders and decoders, I
am not sure how an SRC can be classified as either.

quoted

 union snd_codec_options {
 	struct snd_enc_wma wma;
 	struct snd_enc_vorbis vorbis;

@@ -334,6 +340,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_src src;
 } __attribute__((packed, aligned(4)));

 /** struct snd_codec_desc - description of codec capabilities