| Message ID | 20190709105211.11741-3-ckeepax@opensource.cirrus.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] ALSA: compress: Fix regression on compressed capture streams | expand |
On 09-07-19, 11:52, Charles Keepax wrote: > Partial drain and next track are intended for gapless playback and > don't really have an obvious interpretation for a capture stream, so > makes sense to not allow those operations on capture streams. Drain Sounds right > would make sense on a capture stream but currently the implementation > of drain involves the kernel waiting for the DSP to consume its > available data, whereas a capture drain would involve waiting for > user-space to consume the data available on the DSP. Disallow drain > on capture streams until that is implemented. Well it in unclear to me about the support required in kernel! Kernel issues drain request to DSP and waits for that to be done. When DSP has encoded and copied the data the notification should make drain complete. So I dont see anything in kernel required for that > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > --- > sound/core/compress_offload.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c > index e1a216fd832f9..c7d56cee0d510 100644 > --- a/sound/core/compress_offload.c > +++ b/sound/core/compress_offload.c > @@ -829,6 +829,10 @@ static int snd_compr_drain(struct snd_compr_stream *stream) > break; > } > > + /* drain not implemented for capture streams yet */ > + if (stream->direction == SND_COMPRESS_CAPTURE) > + return -EPERM; > + > retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_DRAIN); > if (retval) { > pr_debug("SND_COMPR_TRIGGER_DRAIN failed %d\n", retval); > @@ -847,6 +851,10 @@ static int snd_compr_next_track(struct snd_compr_stream *stream) > if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) > return -EPERM; > > + /* next track doesn't have any meaning for capture streams */ > + if (stream->direction == SND_COMPRESS_CAPTURE) > + return -EPERM; > + > /* you can signal next track if this is intended to be a gapless stream > * and current track metadata is set > */ > @@ -874,6 +882,10 @@ static int snd_compr_partial_drain(struct snd_compr_stream *stream) > break; > } > > + /* partial drain doesn't have any meaning for capture streams */ > + if (stream->direction == SND_COMPRESS_CAPTURE) > + return -EPERM; > + > /* stream can be drained only when next track has been signalled */ > if (stream->next_track == false) > return -EPERM; > -- > 2.11.0
On Tue, Jul 09, 2019 at 05:06:33PM +0530, Vinod Koul wrote: > On 09-07-19, 11:52, Charles Keepax wrote: > > would make sense on a capture stream but currently the implementation > > of drain involves the kernel waiting for the DSP to consume its > > available data, whereas a capture drain would involve waiting for > > user-space to consume the data available on the DSP. Disallow drain > > on capture streams until that is implemented. > > Well it in unclear to me about the support required in kernel! Kernel > issues drain request to DSP and waits for that to be done. When DSP has > encoded and copied the data the notification should make drain complete. > So I dont see anything in kernel required for that > Ok that makes sense I think what I missed there is we are looking at the capture drain being complete when the data is in the DSP buffer, not when it has been pulled by user-space as I had. I will update the patch to remove this one. Thanks, Charles
diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c index e1a216fd832f9..c7d56cee0d510 100644 --- a/sound/core/compress_offload.c +++ b/sound/core/compress_offload.c @@ -829,6 +829,10 @@ static int snd_compr_drain(struct snd_compr_stream *stream) break; } + /* drain not implemented for capture streams yet */ + if (stream->direction == SND_COMPRESS_CAPTURE) + return -EPERM; + retval = stream->ops->trigger(stream, SND_COMPR_TRIGGER_DRAIN); if (retval) { pr_debug("SND_COMPR_TRIGGER_DRAIN failed %d\n", retval); @@ -847,6 +851,10 @@ static int snd_compr_next_track(struct snd_compr_stream *stream) if (stream->runtime->state != SNDRV_PCM_STATE_RUNNING) return -EPERM; + /* next track doesn't have any meaning for capture streams */ + if (stream->direction == SND_COMPRESS_CAPTURE) + return -EPERM; + /* you can signal next track if this is intended to be a gapless stream * and current track metadata is set */ @@ -874,6 +882,10 @@ static int snd_compr_partial_drain(struct snd_compr_stream *stream) break; } + /* partial drain doesn't have any meaning for capture streams */ + if (stream->direction == SND_COMPRESS_CAPTURE) + return -EPERM; + /* stream can be drained only when next track has been signalled */ if (stream->next_track == false) return -EPERM;
Partial drain and next track are intended for gapless playback and don't really have an obvious interpretation for a capture stream, so makes sense to not allow those operations on capture streams. Drain would make sense on a capture stream but currently the implementation of drain involves the kernel waiting for the DSP to consume its available data, whereas a capture drain would involve waiting for user-space to consume the data available on the DSP. Disallow drain on capture streams until that is implemented. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> --- sound/core/compress_offload.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)