On 08. 08. 24 13:27, Pierre-Louis Bossart wrote:
On 8/8/24 11:17, Shengjiu Wang wrote:
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
I don't. I think we're blurring layers in a really odd way.
The main reason why the compress API was added is to remove the
byte-to-time conversions. But that's clearly not useful for a
post-processing of PCM data, where the bitrate is constant. It really
feels like we're adding this memory-to-memory API to the compress
framework because we don't have anything else available, not because it
makes sense to do so.
It makes sense to offload decoder/encoder tasks as batch processing for
standard compress stream and return back result (PCM stream or encoded stream)
to user space. So it makes sense to use the compress interface (parameters
handshake) for it. Let's talk about the proper SRC extension.
For SRC and dynamic rate modification. I would just create an ALSA control for
this. We are already using the "PCM Rate Shift 100000" control in the
sound/drivers/aloop.c for this purpose (something like pitch in MIDI) for
example. There is no requirement to add this function through metadata ioctls.
As bonus, this control can be monitored with multiple tasks.
Then there's the issue of parameters, we chose to only add parameters
for standard encoders/decoders. Post-processing is highly specific and
the parameter definitions varies from one implementation to another -
and usually parameters are handled in an opaque way with binary
controls. This is best handled with a UUID that needs to be known only
to applications and low-level firmware/hardware, the kernel code should
not have to be modified for each and every processing and to add new
parameters. It just does not scale and it's unmaintainable.
At the very least if you really want to use this compress API, extend it
to use a non-descript "UUID-defined" type and an opaque set of
parameters with this UUID passed in a header.
We don't need to use UUID-defined scheme for simple (A)SRC implementation. As
I noted, the specific runtime controls may use existing ALSA control API.
Jaroslav
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
