Re: [RFC PATCH v2 4/6] ASoC: fsl_asrc_m2m: Add memory to memory function

[Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>]

On 8/16/24 12:42, Shengjiu Wang wrote:
> Implement the ASRC memory to memory function using
> the compress framework, user can use this function with
> compress ioctl interface.
> 
> Define below private metadata key value for output
> format, output rate and ratio modifier configuration.
> ASRC_OUTPUT_FORMAT 0x80000001
> ASRC_OUTPUT_RATE   0x80000002
> ASRC_RATIO_MOD     0x80000003

Can the output format/rate change at run-time?

If no, then these parameters should be moved somewhere else - e.g.
hw_params or something.

I am still not very clear on the expanding the SET_METADATA ioctl to
deal with the ratio changes. This isn't linked to the control layer as
suggested before, and there's no precedent of calling it multiple times
during streaming.

I also wonder how it was tested since tinycompress does not support this?


> +static int fsl_asrc_m2m_fill_codec_caps(struct fsl_asrc *asrc,
> +					struct snd_compr_codec_caps *codec)
> +{
> +	struct fsl_asrc_m2m_cap cap;
> +	__u32 rates[MAX_NUM_BITRATES];
> +	snd_pcm_format_t k;
> +	int i = 0, j = 0;
> +	int ret;
> +
> +	ret = asrc->m2m_get_cap(&cap);
> +	if (ret)
> +		return -EINVAL;
> +
> +	if (cap.rate_in & SNDRV_PCM_RATE_5512)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_5512);

this doesn't sound compatible with the patch2 definitions?

cap->rate_in = SNDRV_PCM_RATE_8000_768000;

> +	if (cap.rate_in & SNDRV_PCM_RATE_8000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_8000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_11025)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_11025);
> +	if (cap.rate_in & SNDRV_PCM_RATE_16000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_16000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_22050)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_22050);

missing 24 kHz

> +	if (cap.rate_in & SNDRV_PCM_RATE_32000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_32000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_44100)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_44100);
> +	if (cap.rate_in & SNDRV_PCM_RATE_48000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_48000);

missing 64kHz

> +	if (cap.rate_in & SNDRV_PCM_RATE_88200)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_88200);
> +	if (cap.rate_in & SNDRV_PCM_RATE_96000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_96000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_176400)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_176400);
> +	if (cap.rate_in & SNDRV_PCM_RATE_192000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_192000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_352800)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_352800);
> +	if (cap.rate_in & SNDRV_PCM_RATE_384000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_384000);
> +	if (cap.rate_in & SNDRV_PCM_RATE_705600)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_705600);
> +	if (cap.rate_in & SNDRV_PCM_RATE_768000)
> +		rates[i++] = snd_pcm_rate_bit_to_rate(SNDRV_PCM_RATE_768000);
> +
> +	pcm_for_each_format(k) {
> +		if (pcm_format_to_bits(k) & cap.fmt_in) {
> +			codec->descriptor[j].max_ch = cap.chan_max;
> +			memcpy(codec->descriptor[j].sample_rates, rates, i * sizeof(__u32));
> +			codec->descriptor[j].num_sample_rates = i;
> +			codec->descriptor[j].formats = k;
> +			j++;
> +		}
> +	}
> +
> +	codec->codec = SND_AUDIOCODEC_PCM;
> +	codec->num_descriptors = j;
> +	return 0;