On 8/14/24 13:12, Shengjiu Wang wrote: > On Wed, Aug 14, 2024 at 5:40 PM Pierre-Louis Bossart > <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote: >> >> >>> Yes, to go further, I think we can use SND_AUDIOCODEC_PCM, then >>> the SRC type will be dropped. >> >> sounds good. >> >>> But my understanding of the control means the .set_metadata() API, right? >>> As I said, the output rate, output format, and ratio modifier are applied to >>> the instances of ASRC, which is the snd_compr_stream in driver. >>> so only the .set_metadata() API can be used for these purposes. >> >> Humm, this is more controversial. >> >> The term 'metadata' really referred to known information present in >> headers or additional ID3 tags and not in the compressed file itself. >> The .set_metadata was assumed to be called ONCE before decoding. >> >> But here you have a need to update the ratio modifier on a regular basis >> to compensate for the drift. This isn't what this specific callback was >> designed for. We could change and allow this callback to be used >> multiple times, but then this could create problems for existing >> implementations which cannot deal with modified metadata on the fly. > > .set_metadata can be called multi times now, no need to change currently. Not really, this set_metadata() callback is used only for gapless transitions between tracks, see fcplay.c in tinycompress. And now I am really confused because tinycompress uses an IOCTL directly: metadata.key = SNDRV_COMPRESS_ENCODER_PADDING; metadata.value[0] = mdata->encoder_padding; if (ioctl(compress->fd, SNDRV_COMPRESS_SET_METADATA, &metadata)) Whereas you want to use the ops callback directly from the control layer? What would present a userspace program from using the ioctl directly then? In that case, why do we need the control? I must be missing something. >> And then there's the problem of defining a 'key' for the metadata. the >> definition of the key is a u32, so there's plenty of space for different >> implementations, but a collision is possible. We'd need an agreement on >> how to allocate keys to different solutions without changing the header >> file for every implementation. > > Can we define a private space for each case? For example the key larger > than 0x80000000 is private, each driver can define it by themself? that would be a possibility indeed - provided that the opens above are straightened out. >> It sounds like we'd need a 'runtime params' callback - unless there's a >> better trick to tie the control and compress layers?