[alsa-devel] [PATCH] ALSA: compress: fix the states to check for allowing read
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; }
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) {
Aren't they "&&"?
Maybe better to use switch for avoiding such a trivial error...
Takashi
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) {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
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) {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
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
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) {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)
On Mon, Apr 29, 2013 at 10:29:00AM +0100, Richard Fitzgerald wrote:
Ok with me too. (You beat me to it with this fix)
Blame it on merge window :) Wanted to ensure 3.10 compress record is fine
-- ~Vinod
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!
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
participants (4)
-
Mark Brown -
Richard Fitzgerald -
Takashi Iwai -
Vinod Koul