On 23.10.2017 11:48, Seppo Ingalsuo wrote:
should this patch be split in two, one that deals with control IPC and one with internal implementation changes? it'd make the review easier, no?
Yep, it's a fair request, will do.
From ea85a69b759144a214f34ce943a118375a66cf2e Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com Date: Tue, 24 Oct 2017 11:28:21 +0300 Subject: [PATCH 1/2] Tone: Independent tones for each audio channel (DO NOT APPLY THIS)
The single large tone patch has been applied already. This split patch only shows changes per feature to ease the patch review.
This patch contains the multi-channel tone generation and code cleanup such as remove unused function argument such as source buffer struct for tone copy. Also the samples copy loop is simplified and buffer write pointer circular wrap is done with an inline function for nicer looking code. In the end of tone copy() the call of comp_update_buffer_produce() is done with actual value instead of zero since all the sink buffer updates are now done in the update produce function. Calling this function in buffer.h with zero as parameter is not allowed usage (may corrupt the buffer). White space changes include some edits for Linux kernel style.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/tone.c | 76 ++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 44 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index 1fde455..24ea901 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -82,64 +82,48 @@ struct comp_data { uint32_t channels; uint32_t frame_bytes; uint32_t rate; - struct tone_state sg; + struct tone_state sg[PLATFORM_MAX_CHANNELS]; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames); + uint32_t frames); };
/* * Tone generator algorithm code */
+static inline void tone_circ_inc_wrap(int32_t **ptr, int32_t *end, size_t size) +{ + if (*ptr >= end) + *ptr = (int32_t *) ((size_t) * ptr - size); +} + static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, - struct comp_buffer *source, uint32_t frames) + uint32_t frames) { struct comp_data *cd = comp_get_drvdata(dev); - int32_t sine_sample; int32_t *dest = (int32_t*) sink->w_ptr; int i; int n; int n_wrap_dest; + int n_min; int nch = cd->channels;
n = frames * nch; while (n > 0) { n_wrap_dest = (int32_t *) sink->end_addr - dest; - if (n < n_wrap_dest) { - /* No circular wrap need */ - while (n > 0) { - /* Update period count for sweeps, etc. */ - tonegen_control(&cd->sg); - /* Calculate mono sine wave sample and then - * duplicate to channels. - */ - sine_sample = tonegen(&cd->sg); - n -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } - } - } else { - /* Process until wrap */ - while (n_wrap_dest > 0) { - tonegen_control(&cd->sg); - sine_sample = tonegen(&cd->sg); - n -= nch; - n_wrap_dest -= nch; - for (i = 0; i < nch; i++) { - *dest = sine_sample; - dest++; - } + n_min = (n < n_wrap_dest) ? n : n_wrap_dest; + /* Process until wrap or completed n */ + while (n_min > 0) { + n -= nch; + n_min -= nch; + for (i = 0; i < nch; i++) { + tonegen_control(&cd->sg[i]); + *dest = tonegen(&cd->sg[i]); + dest++; } - /* No need to check if past end_addr, - * it is so just subtract buffer size. - */ - dest = (int32_t *) ((size_t) dest - - sink->ipc_buffer.size); } + tone_circ_inc_wrap(&dest, sink->end_addr, sink->size); } - sink->w_ptr = dest; }
static int32_t tonegen(struct tone_state *sg) @@ -382,6 +366,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) struct sof_ipc_comp_tone *tone; struct sof_ipc_comp_tone *ipc_tone = (struct sof_ipc_comp_tone *) comp; struct comp_data *cd; + int i;
trace_tone("new");
@@ -403,7 +388,8 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) cd->tone_func = tone_s32_default;
/* Reset tone generator and set channels volumes to default */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]);
dev->state = COMP_STATE_READY; return dev; @@ -496,10 +482,9 @@ static int tone_cmd(struct comp_dev *dev, int cmd, void *data) }
/* copy and process stream data from source to sink buffers */ -static int tone_copy(struct comp_dev *dev) +static int tone_copy(struct comp_dev * dev) { struct comp_buffer *sink; - struct comp_buffer *source = NULL; struct comp_data *cd = comp_get_drvdata(dev);
trace_comp("cpy"); @@ -513,8 +498,10 @@ static int tone_copy(struct comp_dev *dev) */ if (sink->free >= cd->period_bytes) { /* create tone */ - cd->tone_func(dev, sink, source, dev->frames); - comp_update_buffer_produce(sink, 0); + cd->tone_func(dev, sink, dev->frames); + + /* calc new free and available */ + comp_update_buffer_produce(sink, cd->period_bytes); }
return dev->frames; @@ -557,11 +544,13 @@ static int tone_reset(struct comp_dev *dev) {
struct comp_data *cd = comp_get_drvdata(dev); + int i;
trace_tone("TRe");
/* Initialize with the defaults */ - tonegen_reset(&cd->sg); + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) + tonegen_reset(&cd->sg[i]);
comp_set_state(dev, COMP_CMD_RESET);
@@ -570,8 +559,7 @@ static int tone_reset(struct comp_dev *dev)
struct comp_driver comp_tone = { .type = SOF_COMP_TONE, - .ops = - { + .ops = { .new = tone_new, .free = tone_free, .params = tone_params,