[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2be4303e-58e1-4ad7-92cf-f06fa6fa0f08@perex.cz>
Date: Mon, 12 Aug 2024 15:31:18 +0200
From: Jaroslav Kysela <perex@...ex.cz>
To: Shengjiu Wang <shengjiu.wang@...il.com>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
Shengjiu Wang <shengjiu.wang@....com>, vkoul@...nel.org, tiwai@...e.com,
alsa-devel@...a-project.org, linux-sound@...r.kernel.org,
linux-kernel@...r.kernel.org, Xiubo.Lee@...il.com, festevam@...il.com,
nicoleotsuka@...il.com, lgirdwood@...il.com, broonie@...nel.org,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [RFC PATCH 1/6] ALSA: compress: add Sample Rate Converter codec
support
On 12. 08. 24 12:24, Shengjiu Wang wrote:
> On Fri, Aug 9, 2024 at 10:01 PM Jaroslav Kysela <perex@...ex.cz> wrote:
>>
>> On 09. 08. 24 14:52, Pierre-Louis Bossart wrote:
>>
>>>> And metadata
>>>> ioctl can be called many times which can meet the ratio modifier
>>>> requirement (ratio may be drift on the fly)
>>>
>>> Interesting, that's yet another way of handling the drift with userspace
>>> modifying the ratio dynamically. That's different to what I've seen before.
>>
>> Note that the "timing" is managed by the user space with this scheme.
>>
>>>> And compress API uses codec as the unit for capability query and
>>>> parameter setting, So I think need to define "SND_AUDIOCODEC_SRC'
>>>> and 'struct snd_dec_src', for the 'snd_dec_src' just defined output
>>>> format and output rate, channels definition just reuse the snd_codec.ch_in.
>>>
>>> The capability query is an interesting point as well, it's not clear how
>>> to expose to userspace what this specific implementation can do, while
>>> at the same time *requiring* userpace to update the ratio dynamically.
>>> For something like this to work, userspace needs to have pre-existing
>>> information on how the SRC works.
>>
>> Yes, it's about abstraction. The user space wants to push data, read data back
>> converted to the target rate and eventually modify the drift using a control
>> managing clocks using own way. We can eventually assume, that if this control
>> does not exist, the drift cannot be controlled. Also, nice thing is that the
>> control has min and max values (range), so driver can specify the drift range,
>> too.
>>
>> And again, look to "PCM Rate Shift 100000" control implementation in
>> sound/drivers/aloop.c. It would be nice to have the base offset for the
>> shift/drift/pitch value standardized.
>
> Thanks.
>
> But the ASRC driver I implemented is different, I just register one sound
> card, one device/subdevice. but the ASRC hardware support 4 instances
> together, so user can open the card device 4 times to create 4 instances
> then the controls can only bind with compress streams.
It's just a reason to add the subdevice code for the compress offload layer
like we have in other APIs for overall consistency. I'll try to work on this.
> I think I can remove the 'SNDRV_COMPRESS_SRC_RATIO_MOD',
Yes.
> Only define a private type for driver, which means only the ASRC driver
> and its user application know the type.
The control API should be used for this IMHO.
> For the change in 'include/uapi/sound/compress_params.h", should I
> keep them, is there any other suggestion for them?
I don't see a problem there.
Jaroslav
--
Jaroslav Kysela <perex@...ex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
Powered by blists - more mailing lists