* [PATCH] ALSA: compress: fix the states to check for allowing read
@ 2013-04-28 8:05 Vinod Koul
2013-04-29 8:55 ` Takashi Iwai
2013-04-29 11:07 ` Mark Brown
0 siblings, 2 replies; 9+ messages in thread
From: Vinod Koul @ 2013-04-28 8:05 UTC (permalink / raw)
To: alsa-devel
Cc: tiwai, Vinod Koul, broonie, liam.r.girdwood, Charles Keepax,
Richard Fitzgerald
for reading compressed data, we need to allow when we are paused, draining or
stopped.
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
Cc: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
---
sound/core/compress_offload.c | 10 ++++++++--
1 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
index a0bc47f..5389b9a 100644
--- a/sound/core/compress_offload.c
+++ b/sound/core/compress_offload.c
@@ -311,8 +311,14 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
stream = &data->stream;
mutex_lock(&stream->device->lock);
- /* read is allowed when stream is running */
- if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
+ /* read is allowed when stream is running, paused, draining and setup
+ * (yes setup is state which we transistion to after stop, so if user
+ * wants to read data after stop we allow that
+ */
+ if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING ||
+ stream->runtime->state != SNDRV_PCM_STATE_DRAINING ||
+ stream->runtime->state != SNDRV_PCM_STATE_PAUSED ||
+ stream->runtime->state != SNDRV_PCM_STATE_SETUP) {
retval = -EBADFD;
goto out;
}
--
1.7.0.4
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-28 8:05 [PATCH] ALSA: compress: fix the states to check for allowing read Vinod Koul
@ 2013-04-29 8:55 ` Takashi Iwai
2013-04-29 8:45 ` Vinod Koul
2013-04-29 11:07 ` Mark Brown
1 sibling, 1 reply; 9+ messages in thread
From: Takashi Iwai @ 2013-04-29 8:55 UTC (permalink / raw)
To: Vinod Koul
Cc: alsa-devel, broonie, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
At Sun, 28 Apr 2013 13:35:22 +0530,
Vinod Koul wrote:
>
> for reading compressed data, we need to allow when we are paused, draining or
> stopped.
>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> Cc: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
>
> ---
> sound/core/compress_offload.c | 10 ++++++++--
> 1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index a0bc47f..5389b9a 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -311,8 +311,14 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
> stream = &data->stream;
> mutex_lock(&stream->device->lock);
>
> - /* read is allowed when stream is running */
> - if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> + /* read is allowed when stream is running, paused, draining and setup
> + * (yes setup is state which we transistion to after stop, so if user
> + * wants to read data after stop we allow that
> + */
> + if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING ||
> + stream->runtime->state != SNDRV_PCM_STATE_DRAINING ||
> + stream->runtime->state != SNDRV_PCM_STATE_PAUSED ||
> + stream->runtime->state != SNDRV_PCM_STATE_SETUP) {
> retval = -EBADFD;
Aren't they "&&"?
Maybe better to use switch for avoiding such a trivial error...
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-29 8:55 ` Takashi Iwai
@ 2013-04-29 8:45 ` Vinod Koul
2013-04-29 9:20 ` Takashi Iwai
0 siblings, 1 reply; 9+ messages in thread
From: Vinod Koul @ 2013-04-29 8:45 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, broonie, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
On Mon, Apr 29, 2013 at 10:55:04AM +0200, Takashi Iwai wrote:
> At Sun, 28 Apr 2013 13:35:22 +0530,
> Vinod Koul wrote:
> >
> > for reading compressed data, we need to allow when we are paused, draining or
> > stopped.
> >
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > Cc: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> >
> > ---
> > sound/core/compress_offload.c | 10 ++++++++--
> > 1 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > index a0bc47f..5389b9a 100644
> > --- a/sound/core/compress_offload.c
> > +++ b/sound/core/compress_offload.c
> > @@ -311,8 +311,14 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
> > stream = &data->stream;
> > mutex_lock(&stream->device->lock);
> >
> > - /* read is allowed when stream is running */
> > - if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> > + /* read is allowed when stream is running, paused, draining and setup
> > + * (yes setup is state which we transistion to after stop, so if user
> > + * wants to read data after stop we allow that
> > + */
> > + if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING ||
> > + stream->runtime->state != SNDRV_PCM_STATE_DRAINING ||
> > + stream->runtime->state != SNDRV_PCM_STATE_PAUSED ||
> > + stream->runtime->state != SNDRV_PCM_STATE_SETUP) {
> > retval = -EBADFD;
>
> Aren't they "&&"?
Yup :(
>
> Maybe better to use switch for avoiding such a trivial error...
I started with switch and then didnt want to code for all the PCM_STATES... but
yes that makes more sense, i will send that right away.
Is it okay if this get in for 3.10...
--
~Vinod
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-29 8:45 ` Vinod Koul
@ 2013-04-29 9:20 ` Takashi Iwai
2013-04-29 8:57 ` Vinod Koul
2013-04-29 9:29 ` Richard Fitzgerald
0 siblings, 2 replies; 9+ messages in thread
From: Takashi Iwai @ 2013-04-29 9:20 UTC (permalink / raw)
To: Vinod Koul
Cc: alsa-devel, broonie, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
At Mon, 29 Apr 2013 14:15:31 +0530,
Vinod Koul wrote:
>
> On Mon, Apr 29, 2013 at 10:55:04AM +0200, Takashi Iwai wrote:
> > At Sun, 28 Apr 2013 13:35:22 +0530,
> > Vinod Koul wrote:
> > >
> > > for reading compressed data, we need to allow when we are paused, draining or
> > > stopped.
> > >
> > > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > > Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > > Cc: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > >
> > > ---
> > > sound/core/compress_offload.c | 10 ++++++++--
> > > 1 files changed, 8 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > > index a0bc47f..5389b9a 100644
> > > --- a/sound/core/compress_offload.c
> > > +++ b/sound/core/compress_offload.c
> > > @@ -311,8 +311,14 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
> > > stream = &data->stream;
> > > mutex_lock(&stream->device->lock);
> > >
> > > - /* read is allowed when stream is running */
> > > - if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> > > + /* read is allowed when stream is running, paused, draining and setup
> > > + * (yes setup is state which we transistion to after stop, so if user
> > > + * wants to read data after stop we allow that
> > > + */
> > > + if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING ||
> > > + stream->runtime->state != SNDRV_PCM_STATE_DRAINING ||
> > > + stream->runtime->state != SNDRV_PCM_STATE_PAUSED ||
> > > + stream->runtime->state != SNDRV_PCM_STATE_SETUP) {
> > > retval = -EBADFD;
> >
> > Aren't they "&&"?
> Yup :(
> >
> > Maybe better to use switch for avoiding such a trivial error...
> I started with switch and then didnt want to code for all the PCM_STATES... but
> yes that makes more sense, i will send that right away.
You can code like
switch (stream->runtime->state) {
case SNDRV_PCM_STATE_RUNNING:
case SNDRV_PCM_STATE_DRAINING:
case SNDRV_PCM_STATE_PAUSED:
case SNDRV_PCM_STATE_SETUP:
break; /* OK */
default:
retval = -EBADFD;
break;
}
> Is it okay if this get in for 3.10...
Yes. It's just a damn simple fix.
thanks,
Takashi
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-29 9:20 ` Takashi Iwai
@ 2013-04-29 8:57 ` Vinod Koul
2013-04-29 9:29 ` Richard Fitzgerald
1 sibling, 0 replies; 9+ messages in thread
From: Vinod Koul @ 2013-04-29 8:57 UTC (permalink / raw)
To: Takashi Iwai
Cc: alsa-devel, broonie, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
On Mon, Apr 29, 2013 at 11:20:42AM +0200, Takashi Iwai wrote:
> > I started with switch and then didnt want to code for all the PCM_STATES... but
> > yes that makes more sense, i will send that right away.
>
> You can code like
> switch (stream->runtime->state) {
> case SNDRV_PCM_STATE_RUNNING:
> case SNDRV_PCM_STATE_DRAINING:
> case SNDRV_PCM_STATE_PAUSED:
> case SNDRV_PCM_STATE_SETUP:
> break; /* OK */
> default:
> retval = -EBADFD;
> break;
> }
Yup, and i did exactly opposite, let me know if you want like this :)
--
~Vinod
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-29 9:20 ` Takashi Iwai
2013-04-29 8:57 ` Vinod Koul
@ 2013-04-29 9:29 ` Richard Fitzgerald
2013-04-29 9:00 ` Vinod Koul
1 sibling, 1 reply; 9+ messages in thread
From: Richard Fitzgerald @ 2013-04-29 9:29 UTC (permalink / raw)
To: Takashi Iwai
Cc: Vinod Koul, alsa-devel, broonie, Charles Keepax, liam.r.girdwood
On Mon, Apr 29, 2013 at 11:20:42AM +0200, Takashi Iwai wrote:
> At Mon, 29 Apr 2013 14:15:31 +0530,
> Vinod Koul wrote:
> >
> > On Mon, Apr 29, 2013 at 10:55:04AM +0200, Takashi Iwai wrote:
> > > At Sun, 28 Apr 2013 13:35:22 +0530,
> > > Vinod Koul wrote:
> > > >
> > > > for reading compressed data, we need to allow when we are paused, draining or
> > > > stopped.
> > > >
> > > > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> > > > Cc: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> > > > Cc: Richard Fitzgerald <rf@opensource.wolfsonmicro.com>
> > > >
> > > > ---
> > > > sound/core/compress_offload.c | 10 ++++++++--
> > > > 1 files changed, 8 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> > > > index a0bc47f..5389b9a 100644
> > > > --- a/sound/core/compress_offload.c
> > > > +++ b/sound/core/compress_offload.c
> > > > @@ -311,8 +311,14 @@ static ssize_t snd_compr_read(struct file *f, char __user *buf,
> > > > stream = &data->stream;
> > > > mutex_lock(&stream->device->lock);
> > > >
> > > > - /* read is allowed when stream is running */
> > > > - if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) {
> > > > + /* read is allowed when stream is running, paused, draining and setup
> > > > + * (yes setup is state which we transistion to after stop, so if user
> > > > + * wants to read data after stop we allow that
> > > > + */
> > > > + if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING ||
> > > > + stream->runtime->state != SNDRV_PCM_STATE_DRAINING ||
> > > > + stream->runtime->state != SNDRV_PCM_STATE_PAUSED ||
> > > > + stream->runtime->state != SNDRV_PCM_STATE_SETUP) {
> > > > retval = -EBADFD;
> > >
> > > Aren't they "&&"?
> > Yup :(
> > >
> > > Maybe better to use switch for avoiding such a trivial error...
> > I started with switch and then didnt want to code for all the PCM_STATES... but
> > yes that makes more sense, i will send that right away.
>
> You can code like
> switch (stream->runtime->state) {
> case SNDRV_PCM_STATE_RUNNING:
> case SNDRV_PCM_STATE_DRAINING:
> case SNDRV_PCM_STATE_PAUSED:
> case SNDRV_PCM_STATE_SETUP:
> break; /* OK */
> default:
> retval = -EBADFD;
> break;
> }
>
>
> > Is it okay if this get in for 3.10...
>
> Yes. It's just a damn simple fix.
>
>
> thanks,
>
> Takashi
Ok with me too.
(You beat me to it with this fix)
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-28 8:05 [PATCH] ALSA: compress: fix the states to check for allowing read Vinod Koul
2013-04-29 8:55 ` Takashi Iwai
@ 2013-04-29 11:07 ` Mark Brown
2013-04-29 10:54 ` Vinod Koul
1 sibling, 1 reply; 9+ messages in thread
From: Mark Brown @ 2013-04-29 11:07 UTC (permalink / raw)
To: Vinod Koul
Cc: tiwai, alsa-devel, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
[-- Attachment #1.1: Type: text/plain, Size: 464 bytes --]
On Sun, Apr 28, 2013 at 01:35:22PM +0530, Vinod Koul wrote:
> for reading compressed data, we need to allow when we are paused, draining or
> stopped.
Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> + /* read is allowed when stream is running, paused, draining and setup
> + * (yes setup is state which we transistion to after stop, so if user
> + * wants to read data after stop we allow that
> + */
but I think there's a ) missing in here!
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ALSA: compress: fix the states to check for allowing read
2013-04-29 11:07 ` Mark Brown
@ 2013-04-29 10:54 ` Vinod Koul
0 siblings, 0 replies; 9+ messages in thread
From: Vinod Koul @ 2013-04-29 10:54 UTC (permalink / raw)
To: Mark Brown
Cc: tiwai, alsa-devel, Richard Fitzgerald, Charles Keepax,
liam.r.girdwood
[-- Attachment #1.1: Type: text/plain, Size: 695 bytes --]
On Mon, Apr 29, 2013 at 12:07:14PM +0100, Mark Brown wrote:
> On Sun, Apr 28, 2013 at 01:35:22PM +0530, Vinod Koul wrote:
> > for reading compressed data, we need to allow when we are paused, draining or
> > stopped.
>
> Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
>
> > + /* read is allowed when stream is running, paused, draining and setup
> > + * (yes setup is state which we transistion to after stop, so if user
> > + * wants to read data after stop we allow that
> > + */
>
> but I think there's a ) missing in here!
Yup :( I guess since Takshi applied it he can fixup it at end of para, and I
will owe him a beer next time we meet :)
--
~Vinod
[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
[-- Attachment #2: Type: text/plain, Size: 0 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-04-29 11:27 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-04-28 8:05 [PATCH] ALSA: compress: fix the states to check for allowing read Vinod Koul
2013-04-29 8:55 ` Takashi Iwai
2013-04-29 8:45 ` Vinod Koul
2013-04-29 9:20 ` Takashi Iwai
2013-04-29 8:57 ` Vinod Koul
2013-04-29 9:29 ` Richard Fitzgerald
2013-04-29 9:00 ` Vinod Koul
2013-04-29 11:07 ` Mark Brown
2013-04-29 10:54 ` Vinod Koul
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.