| Message ID | 1723804959-31921-1-git-send-email-shengjiu.wang@nxp.com |
|---|---|
| Headers | show |
| Series | ASoC: fsl: add memory to memory function for ASRC | expand |
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;
On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > > > > 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? Seldom I think. > > If no, then these parameters should be moved somewhere else - e.g. > hw_params or something. This means I will do some changes in compress_params.h, add output format and output rate definition, follow Jaroslav's example right? > > 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. Which control layer? if you means the snd_kcontrol_new? it is bound with sound card, but in my case, I need to the control bind with the snd_compr_stream to support multi streams/instances. > > I also wonder how it was tested since tinycompress does not support this? I wrote a unit test to test these ASRC M2M functions. > > > > +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; This ASRC M2M driver is designed for two kinds of hw ASRC modules. one cap is : cap->rate_in = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_5512; another is : cap->rate_in = SNDRV_PCM_RATE_8000_768000; they are in patch2 and patch3 > > > + 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 There is no SNDRV_PCM_RATE_24000 in ALSA. > > > + 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 Yes, will add it. Best regards Shengjiu Wang > > > + 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; > >
On 8/20/24 04:53, Shengjiu Wang wrote: > On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart > <pierre-louis.bossart@linux.intel.com> wrote: >> >> >> >> 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? > > Seldom I think. > >> >> If no, then these parameters should be moved somewhere else - e.g. >> hw_params or something. > > This means I will do some changes in compress_params.h, add > output format and output rate definition, follow Jaroslav's example > right? yes, having parameters for the PCM case would make sense. >> 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. > > Which control layer? if you means the snd_kcontrol_new? it is bound > with sound card, but in my case, I need to the control bind with > the snd_compr_stream to support multi streams/instances. I can only quote Jaroslav's previous answer: " This argument is not valid. The controls are bound to the card, but the element identifiers have already iface (interface), device and subdevice numbers. We are using controls for PCM devices for example. The binding is straight. Just add SNDRV_CTL_ELEM_IFACE_COMPRESS define and specify the compress device number in the 'struct snd_ctl_elem_id'. " >> I also wonder how it was tested since tinycompress does not support this? > > I wrote a unit test to test these ASRC M2M functions. This should be shared IMHO, usually when we add/extend a new interface it's best to have a userspace test program that can be used by others. >>> +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; > > This ASRC M2M driver is designed for two kinds of hw ASRC modules. > > one cap is : cap->rate_in = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_5512; > another is : cap->rate_in = SNDRV_PCM_RATE_8000_768000; > they are in patch2 and patch3 > > >> >>> + 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 > > There is no SNDRV_PCM_RATE_24000 in ALSA. Right, but that doesn't mean 24kHz cannot be supported. We use constraints in those cases. see quote from Takashi found with a 2s Google search https://mailman.alsa-project.org/pipermail/alsa-devel/2013-November/069356.html " CONTINUOUS means that any rate between the specified min and max is fine, if no min or max is specified any rate is fine. KNOT means there are rates supported other than the standard rates defines by ALSA, but the other rates are enumerable. You'd typically specify them by explicitly listing them all and use a list constraint or you'd use one of the ratio constraints. " >>> + 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 > > Yes, will add it.
On Tue, Aug 20, 2024 at 2:59 PM Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com> wrote: > > > > On 8/20/24 04:53, Shengjiu Wang wrote: > > On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart > > <pierre-louis.bossart@linux.intel.com> wrote: > >> > >> > >> > >> 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? > > > > Seldom I think. > > > >> > >> If no, then these parameters should be moved somewhere else - e.g. > >> hw_params or something. > > > > This means I will do some changes in compress_params.h, add > > output format and output rate definition, follow Jaroslav's example > > right? > > yes, having parameters for the PCM case would make sense. > > >> 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. > > > > Which control layer? if you means the snd_kcontrol_new? it is bound > > with sound card, but in my case, I need to the control bind with > > the snd_compr_stream to support multi streams/instances. > > I can only quote Jaroslav's previous answer: > > " > This argument is not valid. The controls are bound to the card, but the > element identifiers have already iface (interface), device and subdevice > numbers. We are using controls for PCM devices for example. The binding > is straight. > > Just add SNDRV_CTL_ELEM_IFACE_COMPRESS define and specify the compress > device number in the 'struct snd_ctl_elem_id'. > " I don't think it is doable, or at least I don't know how to do it. My case is just one card/one device/one subdevice. can't use it to distinguish multi streams. > > >> I also wonder how it was tested since tinycompress does not support this? > > > > I wrote a unit test to test these ASRC M2M functions. > > This should be shared IMHO, usually when we add/extend a new interface > it's best to have a userspace test program that can be used by others. After Jaroslav updates the tinycompress, I can update this example to it. > > >>> +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; > > > > This ASRC M2M driver is designed for two kinds of hw ASRC modules. > > > > one cap is : cap->rate_in = SNDRV_PCM_RATE_8000_192000 | SNDRV_PCM_RATE_5512; > > another is : cap->rate_in = SNDRV_PCM_RATE_8000_768000; > > they are in patch2 and patch3 > > > > > >> > >>> + 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 > > > > There is no SNDRV_PCM_RATE_24000 in ALSA. > > Right, but that doesn't mean 24kHz cannot be supported. We use > constraints in those cases. see quote from Takashi found with a 2s > Google search > > https://mailman.alsa-project.org/pipermail/alsa-devel/2013-November/069356.html > > " > CONTINUOUS means that any rate between the specified min and max is > fine, if no min or max is specified any rate is fine. KNOT means there > are rates supported other than the standard rates defines by ALSA, but > the other rates are enumerable. You'd typically specify them by > explicitly listing them all and use a list constraint or you'd use one > of the ratio constraints. > " > > >>> + 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 > > > > Yes, will add it. >
On 20. 08. 24 9:37, Shengjiu Wang wrote: > On Tue, Aug 20, 2024 at 2:59 PM Pierre-Louis Bossart > <pierre-louis.bossart@linux.intel.com> wrote: >> >> >> >> On 8/20/24 04:53, Shengjiu Wang wrote: >>> On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart >>> <pierre-louis.bossart@linux.intel.com> wrote: >>>> >>>> >>>> >>>> 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? >>> >>> Seldom I think. >>> >>>> >>>> If no, then these parameters should be moved somewhere else - e.g. >>>> hw_params or something. >>> >>> This means I will do some changes in compress_params.h, add >>> output format and output rate definition, follow Jaroslav's example >>> right? >> >> yes, having parameters for the PCM case would make sense. >> >>>> 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. >>> >>> Which control layer? if you means the snd_kcontrol_new? it is bound >>> with sound card, but in my case, I need to the control bind with >>> the snd_compr_stream to support multi streams/instances. >> >> I can only quote Jaroslav's previous answer: >> >> " >> This argument is not valid. The controls are bound to the card, but the >> element identifiers have already iface (interface), device and subdevice >> numbers. We are using controls for PCM devices for example. The binding >> is straight. >> >> Just add SNDRV_CTL_ELEM_IFACE_COMPRESS define and specify the compress >> device number in the 'struct snd_ctl_elem_id'. >> " > > I don't think it is doable, or at least I don't know how to do it. > > My case is just one card/one device/one subdevice. can't use it to > distinguish multi streams. I already wrote that the compress core code should be extended to support subdevices like other ALSA APIs. I'll work on it. For now, just add support for one converter. Jaroslav
On Tue, Aug 20, 2024 at 3:42 PM Jaroslav Kysela <perex@perex.cz> wrote: > > On 20. 08. 24 9:37, Shengjiu Wang wrote: > > On Tue, Aug 20, 2024 at 2:59 PM Pierre-Louis Bossart > > <pierre-louis.bossart@linux.intel.com> wrote: > >> > >> > >> > >> On 8/20/24 04:53, Shengjiu Wang wrote: > >>> On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart > >>> <pierre-louis.bossart@linux.intel.com> wrote: > >>>> > >>>> > >>>> > >>>> 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? > >>> > >>> Seldom I think. > >>> > >>>> > >>>> If no, then these parameters should be moved somewhere else - e.g. > >>>> hw_params or something. > >>> > >>> This means I will do some changes in compress_params.h, add > >>> output format and output rate definition, follow Jaroslav's example > >>> right? > >> > >> yes, having parameters for the PCM case would make sense. > >> > >>>> 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. > >>> > >>> Which control layer? if you means the snd_kcontrol_new? it is bound > >>> with sound card, but in my case, I need to the control bind with > >>> the snd_compr_stream to support multi streams/instances. > >> > >> I can only quote Jaroslav's previous answer: > >> > >> " > >> This argument is not valid. The controls are bound to the card, but the > >> element identifiers have already iface (interface), device and subdevice > >> numbers. We are using controls for PCM devices for example. The binding > >> is straight. > >> > >> Just add SNDRV_CTL_ELEM_IFACE_COMPRESS define and specify the compress > >> device number in the 'struct snd_ctl_elem_id'. > >> " > > > > I don't think it is doable, or at least I don't know how to do it. > > > > My case is just one card/one device/one subdevice. can't use it to > > distinguish multi streams. > > I already wrote that the compress core code should be extended to support > subdevices like other ALSA APIs. I'll work on it. For now, just add support > for one converter. Thanks. What does this subdevices mean? Is it equal to the compress streams? When I call snd_compr_ops.open(), it means to create an instance, the instance is created at runtime (call open()), not created when the sound card is created. Best regards Shengjiu Wang > > Jaroslav > > -- > Jaroslav Kysela <perex@perex.cz> > Linux Sound Maintainer; ALSA Project; Red Hat, Inc. >
On 8/20/24 09:42, Jaroslav Kysela wrote: > On 20. 08. 24 9:37, Shengjiu Wang wrote: >> On Tue, Aug 20, 2024 at 2:59 PM Pierre-Louis Bossart >> <pierre-louis.bossart@linux.intel.com> wrote: >>> >>> >>> >>> On 8/20/24 04:53, Shengjiu Wang wrote: >>>> On Mon, Aug 19, 2024 at 3:42 PM Pierre-Louis Bossart >>>> <pierre-louis.bossart@linux.intel.com> wrote: >>>>> >>>>> >>>>> >>>>> 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? >>>> >>>> Seldom I think. >>>> >>>>> >>>>> If no, then these parameters should be moved somewhere else - e.g. >>>>> hw_params or something. >>>> >>>> This means I will do some changes in compress_params.h, add >>>> output format and output rate definition, follow Jaroslav's example >>>> right? >>> >>> yes, having parameters for the PCM case would make sense. >>> >>>>> 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. >>>> >>>> Which control layer? if you means the snd_kcontrol_new? it is bound >>>> with sound card, but in my case, I need to the control bind with >>>> the snd_compr_stream to support multi streams/instances. >>> >>> I can only quote Jaroslav's previous answer: >>> >>> " >>> This argument is not valid. The controls are bound to the card, but the >>> element identifiers have already iface (interface), device and subdevice >>> numbers. We are using controls for PCM devices for example. The binding >>> is straight. >>> >>> Just add SNDRV_CTL_ELEM_IFACE_COMPRESS define and specify the compress >>> device number in the 'struct snd_ctl_elem_id'. >>> " >> >> I don't think it is doable, or at least I don't know how to do it. >> >> My case is just one card/one device/one subdevice. can't use it to >> distinguish multi streams. > > I already wrote that the compress core code should be extended to > support subdevices like other ALSA APIs. I'll work on it. For now, just > add support for one converter. I am not sure I get the benefits of subdevices in this context. Can we not use different devices already, one per hardware ASRC instance? Put differently, what would be the difference between a card with N compressed devices or a card with 1 compressed device and N subdevices?
This function is base on the accelerator implementation for compress API: https://patchwork.kernel.org/project/alsa-devel/patch/20240731083843.59911-1-perex@perex.cz/ Audio signal processing also has the requirement for memory to memory similar as Video. This asrc memory to memory (memory ->asrc->memory) case is a non real time use case. User fills the input buffer to the asrc module, after conversion, then asrc sends back the output buffer to user. So it is not a traditional ALSA playback and capture case. Because we had implemented the "memory -> asrc ->i2s device-> codec" use case in ALSA. Now the "memory->asrc->memory" needs to reuse the code in asrc driver, so the patch 1 and patch 2 is for refining the code to make it can be shared by the "memory->asrc->memory" driver. Other change is to add memory to memory support for two kinds of i.MX ASRC modules. changes in v2: - Remove the changes in compress API - drop the SNDRV_COMPRESS_SRC_RATIO_MOD - drop the SND_AUDIOCODEC_SRC and struct snd_dec_src - define private metadata key value ASRC_OUTPUT_FORMAT/ASRC_OUTPUT_RATE/ASRC_RATIO_MOD Shengjiu Wang (6): ALSA: compress: reserve space in snd_compr_metadata.key for private usage ASoC: fsl_asrc: define functions for memory to memory usage ASoC: fsl_easrc: define functions for memory to memory usage ASoC: fsl_asrc_m2m: Add memory to memory function ASoC: fsl_asrc: register m2m platform device ASoC: fsl_easrc: register m2m platform device include/uapi/sound/compress_offload.h | 2 +- sound/soc/fsl/Kconfig | 1 + sound/soc/fsl/Makefile | 2 +- sound/soc/fsl/fsl_asrc.c | 176 +++++- sound/soc/fsl/fsl_asrc.h | 2 + sound/soc/fsl/fsl_asrc_common.h | 68 +++ sound/soc/fsl/fsl_asrc_m2m.c | 791 ++++++++++++++++++++++++++ sound/soc/fsl/fsl_easrc.c | 259 ++++++++- sound/soc/fsl/fsl_easrc.h | 4 + 9 files changed, 1297 insertions(+), 8 deletions(-) create mode 100644 sound/soc/fsl/fsl_asrc_m2m.c