On Mon, 11 Apr 2016 16:27:33 +0200,
Charles Keepax wrote:
>
> Currently, the avail IOCTL doesn't pass any error status, which
> means typically on error it simply shows no data available. This
> can lead to situations where user-space is waiting indefinitely
> for data that will never come as the DSP has suffered an
> unrecoverable error.
>
> Add snd_compr_stop_error which end drivers can call to indicate
> the stream has suffered an unrecoverable error and stop it. The
> avail and poll IOCTLs are then updated to report if the stream is
> in an error state to user-space. Allowing the error to propagate
> out. Processing of the actual snd_compr_stop needs to be deferred
> to a worker thread as the end driver may detect the errors during
> an existing operation callback.
>
> Signed-off-by: Charles Keepax <ckeepax@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
> include/sound/compress_driver.h | 4 +++
> sound/core/compress_offload.c | 60 ++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/include/sound/compress_driver.h b/include/sound/compress_driver.h
> index c0abcdc..a057038 100644
> --- a/include/sound/compress_driver.h
> +++ b/include/sound/compress_driver.h
> @@ -78,6 +78,7 @@ struct snd_compr_stream {
> struct snd_compr_ops *ops;
> struct snd_compr_runtime *runtime;
> struct snd_compr *device;
> + struct delayed_work error_work;
> enum snd_compr_direction direction;
> bool metadata_set;
> bool next_track;
> @@ -187,4 +188,7 @@ static inline void snd_compr_drain_notify(struct snd_compr_stream *stream)
> wake_up(&stream->runtime->sleep);
> }
>
> +int snd_compr_stop_error(struct snd_compr_stream *stream,
> + snd_pcm_state_t state);
> +
> #endif
> diff --git a/sound/core/compress_offload.c b/sound/core/compress_offload.c
> index 507071d..86d1d85 100644
> --- a/sound/core/compress_offload.c
> +++ b/sound/core/compress_offload.c
> @@ -67,6 +67,8 @@ struct snd_compr_file {
> struct snd_compr_stream stream;
> };
>
> +static void error_delayed_work(struct work_struct *work);
> +
> /*
> * a note on stream states used:
> * we use following states in the compressed core
> @@ -123,6 +125,9 @@ static int snd_compr_open(struct inode *inode, struct file *f)
> snd_card_unref(compr->card);
> return -ENOMEM;
> }
> +
> + INIT_DELAYED_WORK(&data->stream.error_work, error_delayed_work);
> +
> data->stream.ops = compr->ops;
> data->stream.direction = dirn;
> data->stream.private_data = compr->private_data;
> @@ -153,6 +158,8 @@ static int snd_compr_free(struct inode *inode, struct file *f)
> struct snd_compr_file *data = f->private_data;
> struct snd_compr_runtime *runtime = data->stream.runtime;
>
> + cancel_delayed_work_sync(&data->stream.error_work);
> +
> switch (runtime->state) {
> case SNDRV_PCM_STATE_RUNNING:
> case SNDRV_PCM_STATE_DRAINING:
> @@ -237,6 +244,14 @@ snd_compr_ioctl_avail(struct snd_compr_stream *stream, unsigned long arg)
> avail = snd_compr_calc_avail(stream, &ioctl_avail);
> ioctl_avail.avail = avail;
>
> + switch (stream->runtime->state) {
> + case SNDRV_PCM_STATE_OPEN:
> + case SNDRV_PCM_STATE_XRUN:
> + return -EBADFD;
One question is whether we want a dedicated error code for XRUN or
such a DSP error. On PCM, for example, we return -EPIPE traditionally
for XRUN state. This is a clear indicator for user what to do at
next.
Other than that, the patch series looks good to me.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
