[Sound-open-firmware] [PATCH] comp: buffer: Fix xrun reporting
Liam Girdwood
liam.r.girdwood at linux.intel.com
Thu Nov 16 23:01:27 CET 2017
Fix the free/avail buffer calculations to make sure any overrun or underrun
is reported in trace and returned as a negative error in copy().
Signed-off-by: Liam Girdwood <liam.r.girdwood at linux.intel.com>
---
src/audio/dai.c | 12 ++++++++++
src/audio/eq_fir.c | 22 +++++++++---------
src/audio/eq_iir.c | 22 +++++++++---------
src/audio/host.c | 20 +++++++++++++----
src/audio/mixer.c | 27 ++++++++++++-----------
src/audio/pipeline.c | 6 +++--
src/audio/src.c | 17 +++++++++++++-
src/audio/tone.c | 9 ++++++--
src/audio/volume.c | 22 +++++++++---------
src/include/reef/audio/buffer.h | 49 ++++++++++++++---------------------------
10 files changed, 118 insertions(+), 88 deletions(-)
diff --git a/src/audio/dai.c b/src/audio/dai.c
index 96b37ba..afa4aa2 100644
--- a/src/audio/dai.c
+++ b/src/audio/dai.c
@@ -117,6 +117,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next)
dma_buffer->r_ptr - dma_buffer->addr;
}
+ /* make sure there is availble bytes for next period */
+ if (dma_buffer->avail < dd->period_bytes) {
+ trace_dai_error("xru");
+ comp_underrun(dev, dma_buffer, copied_size, 0);
+ }
+
} else {
dma_buffer = list_first_item(&dev->bsink_list,
struct comp_buffer, source_list);
@@ -134,6 +140,12 @@ static void dai_dma_cb(void *data, uint32_t type, struct dma_sg_elem *next)
*dd->dai_pos = dd->dai_pos_blks +
dma_buffer->w_ptr - dma_buffer->addr;
}
+
+ /* make sure there is free bytes for next period */
+ if (dma_buffer->free < dd->period_bytes) {
+ trace_dai_error("xro");
+ comp_overrun(dev, dma_buffer, dd->period_bytes, 0);
+ }
}
/* notify pipeline that DAI needs its buffer processed */
diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c
index e05920f..2dfa11c 100644
--- a/src/audio/eq_fir.c
+++ b/src/audio/eq_fir.c
@@ -448,7 +448,7 @@ static int eq_fir_copy(struct comp_dev *dev)
struct comp_data *sd = comp_get_drvdata(dev);
struct comp_buffer *source;
struct comp_buffer *sink;
- uint32_t copy_bytes;
+ int res;
trace_comp("EqF");
@@ -458,20 +458,20 @@ static int eq_fir_copy(struct comp_dev *dev)
sink = list_first_item(&dev->bsink_list, struct comp_buffer,
source_list);
- /* Check that source has enough frames available and sink enough
- * frames free.
- */
- copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink);
-
- /* Run EQ if buffers have enough room */
- if (copy_bytes < sd->period_bytes)
- return 0;
+ /* make sure source component buffer has enough data available and that
+ * the sink component buffer has enough free bytes for copy. Also
+ * check for XRUNs */
+ res = comp_buffer_can_copy_bytes(source, sink, sd->period_bytes);
+ if (res) {
+ trace_eq_error("xrn");
+ return -EIO; /* xrun */
+ }
sd->eq_fir_func(dev, source, sink, dev->frames);
/* calc new free and available */
- comp_update_buffer_consume(source, copy_bytes);
- comp_update_buffer_produce(sink, copy_bytes);
+ comp_update_buffer_consume(source, sd->period_bytes);
+ comp_update_buffer_produce(sink, sd->period_bytes);
return dev->frames;
}
diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c
index 1ac8c23..eb464af 100644
--- a/src/audio/eq_iir.c
+++ b/src/audio/eq_iir.c
@@ -446,7 +446,7 @@ static int eq_iir_copy(struct comp_dev *dev)
struct comp_data *cd = comp_get_drvdata(dev);
struct comp_buffer *source;
struct comp_buffer *sink;
- uint32_t copy_bytes;
+ int res;
trace_comp("EqI");
@@ -456,20 +456,20 @@ static int eq_iir_copy(struct comp_dev *dev)
sink = list_first_item(&dev->bsink_list, struct comp_buffer,
source_list);
- /* Check that source has enough frames available and sink enough
- * frames free.
- */
- copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink);
-
- /* Run EQ if buffers have enough room */
- if (copy_bytes < cd->period_bytes)
- return 0;
+ /* make sure source component buffer has enough data available and that
+ * the sink component buffer has enough free bytes for copy. Also
+ * check for XRUNs */
+ res = comp_buffer_can_copy_bytes(source, sink, cd->period_bytes);
+ if (res) {
+ trace_eq_iir_error("xrn");
+ return -EIO; /* xrun */
+ }
cd->eq_iir_func(dev, source, sink, dev->frames);
/* calc new free and available */
- comp_update_buffer_consume(source, copy_bytes);
- comp_update_buffer_produce(sink, copy_bytes);
+ comp_update_buffer_consume(source, cd->period_bytes);
+ comp_update_buffer_produce(sink, cd->period_bytes);
return dev->frames;
}
diff --git a/src/audio/host.c b/src/audio/host.c
index f542eec..88d45e4 100644
--- a/src/audio/host.c
+++ b/src/audio/host.c
@@ -610,15 +610,27 @@ static int host_copy(struct comp_dev *dev)
/* enough free or avail to copy ? */
if (dev->params.direction == SOF_IPC_STREAM_PLAYBACK) {
+
dma_buffer = list_first_item(&dev->bsink_list,
struct comp_buffer, source_list);
- if (dma_buffer->free < local_elem->size)
- return 0;
+
+ if (dma_buffer->free < local_elem->size) {
+ /* make sure there is free bytes for next period */
+ trace_host_error("xro");
+ comp_overrun(dev, dma_buffer, local_elem->size, 0);
+ return -EIO;
+ }
} else {
+
dma_buffer = list_first_item(&dev->bsource_list,
struct comp_buffer, sink_list);
- if (dma_buffer->avail < local_elem->size)
- return 0;
+
+ if (dma_buffer->avail < local_elem->size) {
+ /* make sure there is avail bytes for next period */
+ trace_host_error("xru");
+ comp_underrun(dev, dma_buffer, local_elem->size, 0);
+ return -EIO;
+ }
}
/* do DMA transfer */
diff --git a/src/audio/mixer.c b/src/audio/mixer.c
index e72f1f2..e3611d4 100644
--- a/src/audio/mixer.c
+++ b/src/audio/mixer.c
@@ -220,10 +220,12 @@ static int mixer_copy(struct comp_dev *dev)
struct list_item *blist;
int32_t i = 0;
int32_t num_mix_sources = 0;
- int32_t xru = 0;
+ int res;
tracev_mixer("cpy");
+ sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
+
/* calculate the highest runtime component status between input streams */
list_for_item(blist, &dev->bsource_list) {
source = container_of(blist, struct comp_buffer, sink_list);
@@ -243,22 +245,21 @@ static int mixer_copy(struct comp_dev *dev)
/* make sure no sources have underruns */
for (i = 0; i < num_mix_sources; i++) {
- if (sources[i]->avail < md->period_bytes) {
+
+ /* make sure source component buffer has enough data available
+ * and that the sink component buffer has enough free bytes
+ * for copy. Also check for XRUNs */
+ res = comp_buffer_can_copy_bytes(sources[i], sink, md->period_bytes);
+ if (res < 0) {
+ trace_mixer_error("xru");
comp_underrun(dev, sources[i], sources[i]->avail,
md->period_bytes);
- xru = 1;
+ } else if (res > 0) {
+ trace_mixer_error("xru");
+ comp_overrun(dev, sources[i], sink->free,
+ md->period_bytes);
}
}
- /* underrun ? */
- if (xru)
- return 0;
-
- /* make sure sink has no overruns */
- sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- if (sink->free < md->period_bytes) {
- comp_overrun(dev, sink, sink->free, md->period_bytes);
- return 0;
- }
/* mix streams */
md->mix_func(dev, sink, sources, i, dev->frames);
diff --git a/src/audio/pipeline.c b/src/audio/pipeline.c
index 947b64b..bec7519 100644
--- a/src/audio/pipeline.c
+++ b/src/audio/pipeline.c
@@ -742,7 +742,8 @@ static int pipeline_copy_from_upstream(struct comp_dev *start,
err = pipeline_copy_from_upstream(start, buffer->source);
if (err < 0) {
trace_pipe_error("ePU");
- break;
+ trace_value(current->comp.id);
+ return err;
}
}
@@ -800,7 +801,8 @@ static int pipeline_copy_to_downstream(struct comp_dev *start,
err = pipeline_copy_to_downstream(start, buffer->sink);
if (err < 0) {
trace_pipe_error("ePD");
- break;
+ trace_value(current->comp.id);
+ return err;
}
}
diff --git a/src/audio/src.c b/src/audio/src.c
index c617038..3378065 100644
--- a/src/audio/src.c
+++ b/src/audio/src.c
@@ -496,8 +496,9 @@ static int src_copy(struct comp_dev *dev)
int need_sink;
size_t consumed = 0;
size_t produced = 0;
+ int res;
- trace_comp("SRC");
+ trace_src("SRC");
/* src component needs 1 source and 1 sink buffer */
source = list_first_item(&dev->bsource_list, struct comp_buffer,
@@ -515,6 +516,20 @@ static int src_copy(struct comp_dev *dev)
need_source = cd->param.blk_in * dev->frame_bytes;
need_sink = cd->param.blk_out * dev->frame_bytes;
+ /* make sure source component buffer has enough data available and that
+ * the sink component buffer has enough free bytes for copy. Also
+ * check for XRUNs */
+ res = comp_buffer_can_copy_bytes(source, sink, need_source);
+ if (res) {
+ trace_src_error("xru");
+ return -EIO; /* xrun */
+ }
+ res = comp_buffer_can_copy_bytes(source, sink, need_sink);
+ if (res) {
+ trace_src_error("xro");
+ return -EIO; /* xrun */
+ }
+
/* Run SRC function if buffers avail and free allow */
if (((int) source->avail >= need_source) && ((int) sink->free >= need_sink)) {
cd->src_func(dev, source, sink, &consumed, &produced);
diff --git a/src/audio/tone.c b/src/audio/tone.c
index 8fc513d..581f87e 100644
--- a/src/audio/tone.c
+++ b/src/audio/tone.c
@@ -613,9 +613,14 @@ static int tone_copy(struct comp_dev * dev)
/* calc new free and available */
comp_update_buffer_produce(sink, cd->period_bytes);
- }
- return dev->frames;
+ return dev->frames;
+ } else {
+ /* XRUN */
+ trace_tone_error("xrn");
+ comp_overrun(dev, sink, cd->period_bytes, sink->free);
+ return -EIO;
+ }
}
static int tone_prepare(struct comp_dev * dev)
diff --git a/src/audio/volume.c b/src/audio/volume.c
index 3671046..e741b1a 100644
--- a/src/audio/volume.c
+++ b/src/audio/volume.c
@@ -561,7 +561,6 @@ static int volume_copy(struct comp_dev *dev)
struct comp_data *cd = comp_get_drvdata(dev);
struct comp_buffer *sink;
struct comp_buffer *source;
- uint32_t copy_bytes;
tracev_volume("cpy");
@@ -569,17 +568,18 @@ static int volume_copy(struct comp_dev *dev)
source = list_first_item(&dev->bsource_list, struct comp_buffer, sink_list);
sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list);
- /* get max number of bytes that can be copied */
- copy_bytes = comp_buffer_get_copy_bytes(dev, source, sink);
-
- /* Run volume if buffers have enough room */
- if (copy_bytes < cd->source_period_bytes) {
- comp_underrun(dev, source, copy_bytes, cd->source_period_bytes);
- return 0;
+ /* make sure source component buffer has enough data available and that
+ * the sink component buffer has enough free bytes for copy. Also
+ * check for XRUNs */
+ if (source->avail < cd->source_period_bytes) {
+ trace_volume_error("xru");
+ comp_overrun(dev, source, cd->source_period_bytes, 0);
+ return -EIO; /* xrun */
}
- if (copy_bytes < cd->sink_period_bytes) {
- comp_overrun(dev, sink, copy_bytes, cd->sink_period_bytes);
- return 0;
+ if (sink->free < cd->sink_period_bytes) {
+ trace_volume_error("xro");
+ comp_underrun(dev, sink, cd->sink_period_bytes, 0);
+ return -EIO; /* xrun */
}
/* copy and scale volume */
diff --git a/src/include/reef/audio/buffer.h b/src/include/reef/audio/buffer.h
index 179a84f..2bb2033 100644
--- a/src/include/reef/audio/buffer.h
+++ b/src/include/reef/audio/buffer.h
@@ -81,14 +81,6 @@ void buffer_free(struct comp_buffer *buffer);
static inline void comp_update_buffer_produce(struct comp_buffer *buffer,
uint32_t bytes)
{
- /* complain loudly if component tries to overrun buffer
- * components MUST check for free space first !! */
- if (bytes > buffer->free) {
- trace_buffer_error("Xxo");
- trace_value(buffer->ipc_buffer.comp.id);
- return;
- }
-
buffer->w_ptr += bytes;
/* check for pointer wrap */
@@ -116,14 +108,6 @@ static inline void comp_update_buffer_produce(struct comp_buffer *buffer,
static inline void comp_update_buffer_consume(struct comp_buffer *buffer,
uint32_t bytes)
{
- /* complain loudly if component tries to underrun buffer
- * components MUST check for avail space first !! */
- if (buffer->avail < bytes) {
- trace_buffer_error("Xxu");
- trace_value(buffer->ipc_buffer.comp.id);
- return;
- }
-
buffer->r_ptr += bytes;
/* check for pointer wrap */
@@ -148,29 +132,28 @@ static inline void comp_update_buffer_consume(struct comp_buffer *buffer,
}
/* get the max number of bytes that can be copied between sink and source */
-static inline uint32_t comp_buffer_get_copy_bytes(struct comp_dev *dev,
- struct comp_buffer *source, struct comp_buffer *sink)
+static inline int comp_buffer_can_copy_bytes(struct comp_buffer *source,
+ struct comp_buffer *sink, uint32_t bytes)
{
- uint32_t copy_bytes;
+ /* check for underrun */
+ if (source->avail < bytes)
+ return -1;
- /* Check that source has enough frames available and sink enough
- * frames free.
- */
- if (source->avail > sink->free)
- copy_bytes = sink->free;
- else
- copy_bytes = source->avail;
+ /* check for overrun */
+ if (sink->free < bytes)
+ return 1;
- return copy_bytes;
+ /* we are good to copy */
+ return 0;
}
-static inline void buffer_reset_pos(struct comp_buffer *buffer,
- uint32_t sink_period_bytes)
+static inline uint32_t comp_buffer_get_copy_bytes(struct comp_buffer *source,
+ struct comp_buffer *sink)
{
- buffer->r_ptr = buffer->addr + buffer->size - sink_period_bytes;
- buffer->w_ptr = buffer->addr;
- buffer->free = buffer->size;
- buffer->avail = 0;
+ if (source->avail < sink->free)
+ return sink->free;
+ else
+ return source->avail;
}
static inline void buffer_reset_pos(struct comp_buffer *buffer)
--
2.11.0
More information about the Sound-open-firmware
mailing list