Re: [RFC PATCH] ALSA: compress_offload: introduce passthrough operation mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On 27. 05. 24 13:23, Takashi Iwai wrote:
On Mon, 27 May 2024 09:11:33 +0200,
Jaroslav Kysela wrote:

There is a requirement to expose the audio hardware that accelerates various
tasks for user space such as sample rate converters, compressed
stream decoders, etc.

This is description for the API extension for the compress ALSA API which
is able to handle "tasks" that are not bound to real-time operations
and allows for the serialization of operations.

For details, refer to "compress-passthrough.rst" document.

Note: This code is RFC (not tested, just to clearify the API requirements).
My goal is to add a test (loopback) driver and add a support to tinycompress
library in the next step.

Cc: Mark Brown <broonie@xxxxxxxxxx>
Cc: Shengjiu Wang <shengjiu.wang@xxxxxxxxx>
Cc: Nicolas Dufresne <nicolas@xxxxxxxxxxxx>
Cc: Amadeusz Sławiński <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
Cc: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
Cc: Vinod Koul <vkoul@xxxxxxxxxx>
Signed-off-by: Jaroslav Kysela <perex@xxxxxxxx>
---
(snip)

Through a quick glance, the general idea looks simple enough and
straightforward.  If this satisfies the needs, we can go in this way.

A few subtle things:

- We might want to have a safe limit for the task creations.
   An evil user-space can fill up too easily.

The limit was proposed in snd_compress_check_input() but not used later (TASK_CREATE ioctl). I will send v2 shortly.

- The error checks of memdup_user() are missing.
   A caveat is that memdup_user() returns ERR_PTR() instead of NULL.
   So with the automatic cleanup, you'd need to fiddle with
   PTR_ERR(no_free_ptr(p)).

Thanks.

- The use of __u64 is rather for uapi headers.  The kernel internals
   can be with straight u64.

Yes, thanks.

- The task list add/removal might need some locks.  It looks racy.
   Also the task runtime returned from snd_compr_find_task() might be
   freed.  Need for some locking or refcounting?

All task operations are covered using stream->driver->lock unless I overlooked something.

- A linear lookup is OK from performance POV?  Maybe enough for small
   number of tasks.

We can improve this later, but I would not expect too many queued tasks. I set the limit to 64 for the first implementation.

- Better to have a protocol version check and accept the R/W mode only
   with the newer version.

There is no R/W mode for passthrough direction. An error should be returned. The direction is returned from the driver using SNDRV_COMPRESS_GET_CAPS and also open mode must be O_RDWR so things seem to be separated enough.

					Jaroslav

--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.





[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux