On Tue, Aug 6, 2024 at 7:25 PM Pierre-Louis Bossart
<pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote:
>
>
>
> On 8/6/24 12:26, Shengjiu Wang wrote:
> > Add Sample Rate Converter(SRC) codec support, define the output
> > format and rate for SRC.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@xxxxxxx>
> > ---
> > 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.
There is only decoder and encoder definition for compress, I know
it is difficult to add SRC to encoder or decoder classification,
SRC is a Post Processing. I hope you can have a recommandation.
Thanks.
Best regards
Shengjiu Wang
>
> 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.
>
> >
> > /**
> > 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?
> >
> > /*
> > * 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.
>
> > 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
>
