linux-kernel - Re: [RFC PATCH v2 2/6] ALSA: compress: add new ioctl for setting codec parameters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ee2dc239-c1a7-f48f-c6f0-ec6e61ccdda6@linux.intel.com>
Date:   Tue, 21 Jul 2020 15:05:35 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        broonie@...nel.org
Cc:     alsa-devel@...a-project.org, ckeepax@...nsource.cirrus.com,
        tiwai@...e.com, lgirdwood@...il.com, linux-kernel@...r.kernel.org,
        vkoul@...nel.org
Subject: Re: [RFC PATCH v2 2/6] ALSA: compress: add new ioctl for setting
 codec parameters



On 7/21/20 12:00 PM, Srinivas Kandagatla wrote:
> For gapless playback it is possible that each track can have different
> codec profile with same decoder, for example we have WMA album,
> we may have different tracks as WMA v9, WMA v10 and so on
> 
> Or if DSP's like QDSP have abililty to switch decoders on single stream

ability

> for each track, then this call could be used to set new codec parameters.
> 
> Existing code does not allow to change this profile while doing gapless
> playback.
> 
> This patch adds new SNDRV_COMPRESS_SET_CODEC_PARAMS IOCTL to allow
> userspace to set this new parameters required for new codec profile.
> 
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> ---
>   .../sound/designs/compress-offload.rst        |  6 ++++
>   include/sound/compress_driver.h               |  5 +++
>   include/uapi/sound/compress_offload.h         |  1 +
>   sound/core/compress_offload.c                 | 34 +++++++++++++++++++
>   4 files changed, 46 insertions(+)
> 
> diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
> index 935f325dbc77..305ccc7bfdd9 100644
> --- a/Documentation/sound/designs/compress-offload.rst
> +++ b/Documentation/sound/designs/compress-offload.rst
> @@ -128,6 +128,12 @@ set_params
>     cases decoders will ignore other fields, while encoders will strictly
>     comply to the settings
>   
> +set_codec_params
> +  This routine is very much simillar to set_params but exculding stream

typos: similar, excluding

> +  information. Only codec related information is set as part of this.
> +  It is used in gapless playback where its required to change decoder
> +  or its parameters for next track. This is optional.
> +
>   get_params
>     This routines returns the actual settings used by the DSP. Changes to
>     the settings should remain the exception.
> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> index 70cbc5095e72..d9c00bcfce9b 100644
> --- a/include/sound/compress_driver.h
> +++ b/include/sound/compress_driver.h
> @@ -93,6 +93,9 @@ struct snd_compr_stream {
>    * @set_params: Sets the compressed stream parameters, mandatory
>    * This can be called in during stream creation only to set codec params
>    * and the stream properties
> + * @set_codec_params: Sets the compressed stream codec parameters, Optional
> + * This can be called in during gapless next track codec change only to set
> + * codec params

Would it be clearer if this was called set_next_codec_params()? or 
set_next_track_codec_params()?

Having set_params() and set_codec_params() is a bit confusing since the 
semantic difference is not captured in the callback name.