[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87edmebk2w.wl-tiwai@suse.de>
Date: Wed, 14 Jun 2023 09:04:55 +0200
From: Takashi Iwai <tiwai@...e.de>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: vkoul@...nel.org, broonie@...nel.org, tiwai@...e.com,
corbet@....net, alsa-devel@...a-project.org,
linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
quic_plai@...cinc.com, quic_mohs@...cinc.com
Subject: Re: [PATCH v2] ALSA: compress: allow setting codec params after next track
On Fri, 09 Jun 2023 17:04:16 +0200,
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
> 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.
>
> Reuse existing SNDRV_COMPRESS_SET_PARAMS to set this new track params along
> some additional checks to enforce proper state machine.
>
> With this new changes now the user can call SNDRV_COMPRESS_SET_PARAMS
> anytime after setting next track and additional check in write should
> also ensure that params are set before writing new data.
>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> ---
> Changes since v1/RFC:
> - removed introduction of new IOCTL, as suggested.
> - update the state-machine doc.
>
> .../sound/designs/compress-offload.rst | 52 +++++++++----------
> sound/core/compress_offload.c | 10 ++--
> 2 files changed, 33 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
> index 935f325dbc77..205cadcabe70 100644
> --- a/Documentation/sound/designs/compress-offload.rst
> +++ b/Documentation/sound/designs/compress-offload.rst
> @@ -256,32 +256,32 @@ Gapless Playback SM
> For Gapless, we move from running state to partial drain and back, along
> with setting of meta_data and signalling for next track ::
>
> -
> - +----------+
> - compr_drain_notify() | |
> - +------------------------>| RUNNING |
> - | | |
> - | +----------+
> - | |
> - | |
> - | | compr_next_track()
> - | |
> - | V
> - | +----------+
> - | | |
> - | |NEXT_TRACK|
> - | | |
> - | +----------+
> - | |
> - | |
> - | | compr_partial_drain()
> - | |
> - | V
> - | +----------+
> - | | |
> - +------------------------ | PARTIAL_ |
> - | DRAIN |
> - +----------+
> + +----------+
> + compr_drain_notify() | | compr_set_params() iff next-track set.
> + +------------------------>| RUNNING |----------------------+
> + | | | |
> + | +----------+ |
> + | | |
> + | | |
> + | | compr_next_track() |
> + | | V
> + | V |
> + | +----------+ |
> + | | | |
> + | |NEXT_TRACK| |
> + | | | |
> + | +----------+ |
> + | | |
> + | +----------------------------+
> + | |
> + | | compr_partial_drain()
> + | |
> + | V
> + | +----------+
> + | | |
> + +------------------------ | PARTIAL_ |
> + | DRAIN |
> + +----------+
>
> Not supported
> =============
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 243acad89fd3..fe67228e74b3 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -294,6 +294,9 @@ static ssize_t snd_compr_write(struct file *f, const char __user *buf,
> case SNDRV_PCM_STATE_SETUP:
> case SNDRV_PCM_STATE_PREPARED:
> case SNDRV_PCM_STATE_RUNNING:
> + /* Make sure next track params are set before writing new data */
> + if (stream->next_track)
> + return -EPERM;
Hm, does this logic correctly match with the comment above?
Also, this misses the mutex unlock.
thanks,
Takashi
Powered by blists - more mailing lists