* [PATCH v2 0/3] ALSA: compress: Document stream states and fix gapless SM
@ 2020-06-22 6:58 Vinod Koul
2020-06-22 6:58 ` [PATCH v2 1/3] ALSA: compress: document the compress audio state machine Vinod Koul
2020-06-22 6:58 ` [PATCH v2 2/3] ALSA: compress: document the compress gapless " Vinod Koul
0 siblings, 2 replies; 7+ messages in thread
From: Vinod Koul @ 2020-06-22 6:58 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela
Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Srinivas Kandagatla,
Pierre-Louis Bossart, Charles Keepax, alsa-devel, linux-kernel
Srini found issue with gapless implementation which prompted to look deeper
into SM for compressed stream.
So documenting SM was first step, so first two patches add that. Last patch
fixes the issue by keeping track on partial_drain and then moving state to
'running' in snd_compr_drain_notify() for partial_drain case on success.
While at it, noticed snd_compr_drain_notify() is lockless state change, so
fixed that as well.
I have tested this on Dragon board RB3, compressed audio works out of the
box on that platform and Srini will send driver and fcplay patches for
gapless soon.
Changes in v2:
- Added tested tag by Srini
- Update compress SM with Free state and compr_stop() transitions
Vinod Koul (3):
ALSA: compress: document the compress audio state machine
ALSA: compress: document the compress gapless audio state machine
ALSA: compress: fix partial_drain completion state
.../sound/designs/compress-offload.rst | 84 +++++++++++++++++++
include/sound/compress_driver.h | 12 ++-
sound/core/compress_offload.c | 4 +
3 files changed, 99 insertions(+), 1 deletion(-)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/3] ALSA: compress: document the compress audio state machine
2020-06-22 6:58 [PATCH v2 0/3] ALSA: compress: Document stream states and fix gapless SM Vinod Koul
@ 2020-06-22 6:58 ` Vinod Koul
2020-06-22 13:28 ` Pierre-Louis Bossart
2020-06-22 6:58 ` [PATCH v2 2/3] ALSA: compress: document the compress gapless " Vinod Koul
1 sibling, 1 reply; 7+ messages in thread
From: Vinod Koul @ 2020-06-22 6:58 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela
Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Srinivas Kandagatla,
Pierre-Louis Bossart, Charles Keepax, alsa-devel, linux-kernel
So we had some discussions of the stream states, so I thought it is a
good idea to document the state transitions, so add it documentation
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
.../sound/designs/compress-offload.rst | 52 +++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
index ad4bfbdacc83..6f86db82298b 100644
--- a/Documentation/sound/designs/compress-offload.rst
+++ b/Documentation/sound/designs/compress-offload.rst
@@ -151,6 +151,58 @@ Modifications include:
- Addition of encoding options when required (derived from OpenMAX IL)
- Addition of rateControlSupported (missing in OpenMAX AL)
+State Machine
+=============
+
+The compressed audio stream state machine is described below ::
+
+ +----------+
+ | |
+ | OPEN |
+ | |
+ +----------+
+ |
+ |
+ | compr_set_params()
+ |
+ v
+ compr_free() +----------+
+ +-------------------------------------| |
+ | | SETUP |
+ | +------------------------>| |<---------------------------------+
+ | | compr_drain_notify() +----------+ |
+ | | | |
+ | | | |
+ | | | compr_write() |
+ | | | |
+ | | v |
+ | | +----------+ |
+ | | | | |
+ | | | PREPARE | |
+ | | | | |
+ | | +----------+ |
+ | | | |
+ | | | |
+ | | | compr_start() |
+ | | | |
+ | | v |
+ | +----------+ +----------+ compr_pause() +----------+ |
+ | | | compr_drain() | |---------------->| | |
+ | | DRAIN |<-------------------| RUNNING | | PAUSE | |
+ | | | | |<----------------| | |
+ | +----------+ +----------+ compr_resume() +----------+ |
+ | | | | |
+ | | | | |
+ | | | | |
+ | | | | compr_stop() |
+ | | | +------------------------------------+
+ | | +----------+ |
+ | | | | |
+ +-----------+------>| |<-------+
+ compr_free() | FREE | compr_free()
+ | |
+ +----------+
+
Gapless Playback
================
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/3] ALSA: compress: document the compress gapless audio state machine
2020-06-22 6:58 [PATCH v2 0/3] ALSA: compress: Document stream states and fix gapless SM Vinod Koul
2020-06-22 6:58 ` [PATCH v2 1/3] ALSA: compress: document the compress audio state machine Vinod Koul
@ 2020-06-22 6:58 ` Vinod Koul
1 sibling, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-06-22 6:58 UTC (permalink / raw)
To: Takashi Iwai, Jaroslav Kysela
Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Srinivas Kandagatla,
Pierre-Louis Bossart, Charles Keepax, alsa-devel, linux-kernel
Also documented the galpess transitions. Please note that these are not
really stream states, but show how the stream steps in gapless mode
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
.../sound/designs/compress-offload.rst | 32 +++++++++++++++++++
1 file changed, 32 insertions(+)
diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
index 6f86db82298b..0bebfd33b933 100644
--- a/Documentation/sound/designs/compress-offload.rst
+++ b/Documentation/sound/designs/compress-offload.rst
@@ -251,6 +251,38 @@ Sequence flow for gapless would be:
(note: order for partial_drain and write for next track can be reversed as well)
+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 |
+ +----------+
Not supported
=============
--
2.26.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] ALSA: compress: document the compress audio state machine
2020-06-22 6:58 ` [PATCH v2 1/3] ALSA: compress: document the compress audio state machine Vinod Koul
@ 2020-06-22 13:28 ` Pierre-Louis Bossart
2020-06-22 15:13 ` Charles Keepax
2020-06-23 4:05 ` Vinod Koul
0 siblings, 2 replies; 7+ messages in thread
From: Pierre-Louis Bossart @ 2020-06-22 13:28 UTC (permalink / raw)
To: Vinod Koul, Takashi Iwai, Jaroslav Kysela
Cc: alsa-devel, Charles Keepax, linux-arm-msm, Bjorn Andersson,
Srinivas Kandagatla, linux-kernel
On 6/22/20 1:58 AM, Vinod Koul wrote:
> So we had some discussions of the stream states, so I thought it is a
> good idea to document the state transitions, so add it documentation
>
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> .../sound/designs/compress-offload.rst | 52 +++++++++++++++++++
> 1 file changed, 52 insertions(+)
>
> diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
> index ad4bfbdacc83..6f86db82298b 100644
> --- a/Documentation/sound/designs/compress-offload.rst
> +++ b/Documentation/sound/designs/compress-offload.rst
> @@ -151,6 +151,58 @@ Modifications include:
> - Addition of encoding options when required (derived from OpenMAX IL)
> - Addition of rateControlSupported (missing in OpenMAX AL)
>
> +State Machine
> +=============
> +
> +The compressed audio stream state machine is described below ::
> +
> + +----------+
> + | |
> + | OPEN |
> + | |
> + +----------+
> + |
> + |
> + | compr_set_params()
> + |
> + v
> + compr_free() +----------+
> + +-------------------------------------| |
> + | | SETUP |
> + | +------------------------>| |<---------------------------------+
> + | | compr_drain_notify() +----------+ |
> + | | | |
> + | | | |
> + | | | compr_write() |
> + | | | |
> + | | v |
> + | | +----------+ |
> + | | | | |
> + | | | PREPARE | |
> + | | | | |
> + | | +----------+ |
> + | | | |
> + | | | |
> + | | | compr_start() |
> + | | | |
> + | | v |
> + | +----------+ +----------+ compr_pause() +----------+ |
> + | | | compr_drain() | |---------------->| | |
> + | | DRAIN |<-------------------| RUNNING | | PAUSE | |
> + | | | | |<----------------| | |
> + | +----------+ +----------+ compr_resume() +----------+ |
> + | | | | |
> + | | | | |
> + | | | | |
> + | | | | compr_stop() |
> + | | | +------------------------------------+
> + | | +----------+ |
> + | | | | |
> + +-----------+------>| |<-------+
> + compr_free() | FREE | compr_free()
> + | |
> + +----------+
> +
Sorry, this confuses me even more...
a) can you clarify if we can go from running to free directly? is this
really a legit transition? There's already the option of doing a stop
and a a drain.
b) no way to go back to SETUP from PREPARE? What happens if the app
never starts but want to tear down the resources?
c) no way to stop a paused stream?
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] ALSA: compress: document the compress audio state machine
2020-06-22 13:28 ` Pierre-Louis Bossart
@ 2020-06-22 15:13 ` Charles Keepax
2020-06-23 4:09 ` Vinod Koul
2020-06-23 4:05 ` Vinod Koul
1 sibling, 1 reply; 7+ messages in thread
From: Charles Keepax @ 2020-06-22 15:13 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Vinod Koul, Takashi Iwai, Jaroslav Kysela, alsa-devel,
linux-arm-msm, Bjorn Andersson, Srinivas Kandagatla,
linux-kernel
On Mon, Jun 22, 2020 at 08:28:48AM -0500, Pierre-Louis Bossart wrote:
> On 6/22/20 1:58 AM, Vinod Koul wrote:
> >+ +----------+
> >+ | |
> >+ | OPEN |
> >+ | |
> >+ +----------+
> >+ |
> >+ |
> >+ | compr_set_params()
> >+ |
> >+ v
> >+ compr_free() +----------+
> >+ +-------------------------------------| |
> >+ | | SETUP |
> >+ | +------------------------>| |<---------------------------------+
> >+ | | compr_drain_notify() +----------+ |
> >+ | | | |
> >+ | | | |
> >+ | | | compr_write() |
> >+ | | | |
> >+ | | v |
> >+ | | +----------+ |
> >+ | | | | |
> >+ | | | PREPARE | |
> >+ | | | | |
> >+ | | +----------+ |
> >+ | | | |
> >+ | | | |
> >+ | | | compr_start() |
> >+ | | | |
> >+ | | v |
> >+ | +----------+ +----------+ compr_pause() +----------+ |
> >+ | | | compr_drain() | |---------------->| | |
> >+ | | DRAIN |<-------------------| RUNNING | | PAUSE | |
> >+ | | | | |<----------------| | |
> >+ | +----------+ +----------+ compr_resume() +----------+ |
> >+ | | | | |
> >+ | | | | |
> >+ | | | | |
> >+ | | | | compr_stop() |
> >+ | | | +------------------------------------+
> >+ | | +----------+ |
> >+ | | | | |
> >+ +-----------+------>| |<-------+
> >+ compr_free() | FREE | compr_free()
> >+ | |
> >+ +----------+
> a) can you clarify if we can go from running to free directly? is
> this really a legit transition? There's already the option of doing
> a stop and a a drain.
>
This is allowed in the current code, the kernel sends the stop
internally in this case, so it kinda does go through the setup
state just not from the users view point. I am not sure I have a
good handle on if that makes sense or not.
> c) no way to stop a paused stream?
Currently the code does allow this and it certainly makes sense
so should probably be added.
Thanks,
Charles
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] ALSA: compress: document the compress audio state machine
2020-06-22 13:28 ` Pierre-Louis Bossart
2020-06-22 15:13 ` Charles Keepax
@ 2020-06-23 4:05 ` Vinod Koul
1 sibling, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-06-23 4:05 UTC (permalink / raw)
To: Pierre-Louis Bossart
Cc: Takashi Iwai, Jaroslav Kysela, alsa-devel, Charles Keepax,
linux-arm-msm, Bjorn Andersson, Srinivas Kandagatla,
linux-kernel
On 22-06-20, 08:28, Pierre-Louis Bossart wrote:
>
>
> On 6/22/20 1:58 AM, Vinod Koul wrote:
> > So we had some discussions of the stream states, so I thought it is a
> > good idea to document the state transitions, so add it documentation
> >
> > Signed-off-by: Vinod Koul <vkoul@kernel.org>
> > ---
> > .../sound/designs/compress-offload.rst | 52 +++++++++++++++++++
> > 1 file changed, 52 insertions(+)
> >
> > diff --git a/Documentation/sound/designs/compress-offload.rst b/Documentation/sound/designs/compress-offload.rst
> > index ad4bfbdacc83..6f86db82298b 100644
> > --- a/Documentation/sound/designs/compress-offload.rst
> > +++ b/Documentation/sound/designs/compress-offload.rst
> > @@ -151,6 +151,58 @@ Modifications include:
> > - Addition of encoding options when required (derived from OpenMAX IL)
> > - Addition of rateControlSupported (missing in OpenMAX AL)
> > +State Machine
> > +=============
> > +
> > +The compressed audio stream state machine is described below ::
> > +
> > + +----------+
> > + | |
> > + | OPEN |
> > + | |
> > + +----------+
> > + |
> > + |
> > + | compr_set_params()
> > + |
> > + v
> > + compr_free() +----------+
> > + +-------------------------------------| |
> > + | | SETUP |
> > + | +------------------------>| |<---------------------------------+
> > + | | compr_drain_notify() +----------+ |
> > + | | | |
> > + | | | |
> > + | | | compr_write() |
> > + | | | |
> > + | | v |
> > + | | +----------+ |
> > + | | | | |
> > + | | | PREPARE | |
> > + | | | | |
> > + | | +----------+ |
> > + | | | |
> > + | | | |
> > + | | | compr_start() |
> > + | | | |
> > + | | v |
> > + | +----------+ +----------+ compr_pause() +----------+ |
> > + | | | compr_drain() | |---------------->| | |
> > + | | DRAIN |<-------------------| RUNNING | | PAUSE | |
> > + | | | | |<----------------| | |
> > + | +----------+ +----------+ compr_resume() +----------+ |
> > + | | | | |
> > + | | | | |
> > + | | | | |
> > + | | | | compr_stop() |
> > + | | | +------------------------------------+
> > + | | +----------+ |
> > + | | | | |
> > + +-----------+------>| |<-------+
> > + compr_free() | FREE | compr_free()
> > + | |
> > + +----------+
> > +
>
> Sorry, this confuses me even more...
Oops
> a) can you clarify if we can go from running to free directly? is this
> really a legit transition? There's already the option of doing a stop and a
> a drain.
As Charles pointed it is legit one, but then from SM we should remove
running->free arrow to clarify. Internally, free in running, draining
and paused triggers a stop and then free.
> b) no way to go back to SETUP from PREPARE? What happens if the app never
> starts but want to tear down the resources?
See above, free would trigger an internal step to stop (setup) and then
freed.
Btw I am not adding changes to code but documenting the 'existing'
behaviour, so a code lookup will help
> c) no way to stop a paused stream?
Yes missed that, will add.
--
~Vinod
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/3] ALSA: compress: document the compress audio state machine
2020-06-22 15:13 ` Charles Keepax
@ 2020-06-23 4:09 ` Vinod Koul
0 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2020-06-23 4:09 UTC (permalink / raw)
To: Charles Keepax
Cc: Pierre-Louis Bossart, Takashi Iwai, Jaroslav Kysela, alsa-devel,
linux-arm-msm, Bjorn Andersson, Srinivas Kandagatla,
linux-kernel
On 22-06-20, 15:13, Charles Keepax wrote:
> On Mon, Jun 22, 2020 at 08:28:48AM -0500, Pierre-Louis Bossart wrote:
> > On 6/22/20 1:58 AM, Vinod Koul wrote:
+----------+
> > a) can you clarify if we can go from running to free directly? is
> > this really a legit transition? There's already the option of doing
> > a stop and a a drain.
> >
>
> This is allowed in the current code, the kernel sends the stop
> internally in this case, so it kinda does go through the setup
> state just not from the users view point. I am not sure I have a
> good handle on if that makes sense or not.
The idea was to stop first so that we can handle dmas which might be
setup (like running/paused/prepared). So we should stop first and then
free up. But i think it was an overkill... :)
> > c) no way to stop a paused stream?
>
> Currently the code does allow this and it certainly makes sense so
> should probably be added.
Yes will add
--
~Vinod
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-06-23 4:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-22 6:58 [PATCH v2 0/3] ALSA: compress: Document stream states and fix gapless SM Vinod Koul
2020-06-22 6:58 ` [PATCH v2 1/3] ALSA: compress: document the compress audio state machine Vinod Koul
2020-06-22 13:28 ` Pierre-Louis Bossart
2020-06-22 15:13 ` Charles Keepax
2020-06-23 4:09 ` Vinod Koul
2020-06-23 4:05 ` Vinod Koul
2020-06-22 6:58 ` [PATCH v2 2/3] ALSA: compress: document the compress gapless " Vinod Koul
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).