[Sound-open-firmware] [PATCH 1/4] SRC: IPC updates and fixes
SRC sets sink and downstream pipeline PCM rate in src_params() to received out_rate. Also the use of incorrect alloc_size is replaced by ipc_buffer.size.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/src.c | 22 +++++++++++++--------- src/include/reef/audio/buffer.h | 8 ++++---- src/include/uapi/ipc.h | 1 + 3 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/src/audio/src.c b/src/audio/src.c index c338220..58ecf4b 100644 --- a/src/audio/src.c +++ b/src/audio/src.c @@ -155,7 +155,7 @@ static void src_2s_s32_default(struct comp_dev *dev,
s1.times = n_times1; s1.x_end_addr = source->end_addr; - s1.x_size = source->alloc_size; + s1.x_size = source->ipc_buffer.size; s1.x_inc = nch; s1.y_end_addr = &cd->delay_lines[cd->scratch_length]; s1.y_size = STAGE_BUF_SIZE * sizeof(int32_t); @@ -166,7 +166,7 @@ static void src_2s_s32_default(struct comp_dev *dev, s2.x_size = STAGE_BUF_SIZE * sizeof(int32_t); s2.x_inc = 1; s2.y_end_addr = sink->end_addr; - s2.y_size = sink->alloc_size; + s2.y_size = sink->ipc_buffer.size; s2.y_inc = nch;
s1.x_rptr = src + nch - 1; @@ -227,10 +227,10 @@ static void src_1s_s32_default(struct comp_dev *dev,
s1.times = n_times; s1.x_end_addr = source->end_addr; - s1.x_size = source->alloc_size; + s1.x_size = source->ipc_buffer.size; s1.x_inc = nch; s1.y_end_addr = sink->end_addr; - s1.y_size = sink->alloc_size; + s1.y_size = sink->ipc_buffer.size; s1.y_inc = nch; s1.x_rptr = src + nch - 1; s1.y_wptr = dest + nch - 1; @@ -263,7 +263,7 @@ static struct comp_dev *src_new(struct sof_ipc_comp *comp) { struct comp_dev *dev; struct sof_ipc_comp_src *src; - struct sof_ipc_comp_src *ipc_src = (struct sof_ipc_comp_src *)comp; + struct sof_ipc_comp_src *ipc_src = (struct sof_ipc_comp_src *) comp; struct comp_data *cd; int i;
@@ -274,7 +274,7 @@ static struct comp_dev *src_new(struct sof_ipc_comp *comp) if (dev == NULL) return NULL;
- src = (struct sof_ipc_comp_src *)&dev->comp; + src = (struct sof_ipc_comp_src *) &dev->comp; memcpy(src, ipc_src, sizeof(struct sof_ipc_comp_src));
cd = rmalloc(RZONE_RUNTIME, RFLAGS_NONE, sizeof(*cd)); @@ -317,6 +317,7 @@ static int src_params(struct comp_dev *dev, struct stream_params *params) int32_t *buffer_start; int n = 0; struct comp_data *cd = comp_get_drvdata(dev); + struct sof_ipc_comp_src *src = (struct sof_ipc_comp_src *) &dev->comp;
trace_src("SPa");
@@ -325,7 +326,8 @@ static int src_params(struct comp_dev *dev, struct stream_params *params) || (params->pcm->frame_fmt != SOF_IPC_FRAME_S32_LE)) return -EINVAL;
- /* No data transformation */ + /* Stored IPC from src_new() contains the output rate for sink */ + params->pcm->rate = src->out_rate; comp_set_sink_params(dev, params);
/* Allocate needed memory for delay lines */ @@ -333,8 +335,10 @@ static int src_params(struct comp_dev *dev, struct stream_params *params) sink_list); sink = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - src_buffer_lengths(&need, source->params.pcm->rate, - sink->params.pcm->rate, source->params.pcm->channels); + if (src_buffer_lengths(&need, source->params.pcm->rate, + sink->params.pcm->rate, source->params.pcm->channels) < 0) + return -EINVAL; + delay_lines_size = sizeof(int32_t) * need.total; if (cd->delay_lines != NULL) rfree(cd->delay_lines); diff --git a/src/include/reef/audio/buffer.h b/src/include/reef/audio/buffer.h index 5f2dbb7..0817a40 100644 --- a/src/include/reef/audio/buffer.h +++ b/src/include/reef/audio/buffer.h @@ -119,20 +119,20 @@ static inline void comp_update_sink_free_avail(struct comp_buffer *snk, int n) static inline void comp_wrap_source_r_ptr_circular(struct comp_buffer *src) { if (src->r_ptr >= src->end_addr) - src->r_ptr -= src->alloc_size; + src->r_ptr -= src->ipc_buffer.size;
if (src->r_ptr < src->addr) - src->r_ptr += src->alloc_size; + src->r_ptr += src->ipc_buffer.size; }
static inline void comp_wrap_sink_w_ptr_circular(struct comp_buffer *snk) { if (snk->w_ptr >= snk->end_addr) - snk->w_ptr -= snk->alloc_size; + snk->w_ptr -= snk->ipc_buffer.size;
if (snk->w_ptr < snk->addr) - snk->w_ptr += snk->alloc_size; + snk->w_ptr += snk->ipc_buffer.size; }
#endif diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index 3d78db3..d89039e 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -488,6 +488,7 @@ struct sof_ipc_comp_src { struct sof_ipc_pcm_comp pcm; uint32_t in_mask; /* SOF_RATE_ supported input rates */ uint32_t out_mask; /* SOF_RATE_ supported output rates */ + int32_t out_rate; } __attribute__((packed));
/* generic MUX component */
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@
#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
static int32_t tonegen(struct tone_state *sg); static void tonegen_control(struct tone_state *sg); @@ -77,10 +78,11 @@ static const int32_t tone_pi2_div_fs[TONE_NUM_FS] = {
/* TODO: Remove *source when internal endpoint is possible */ struct comp_data { + enum sof_ipc_chmap chan[PLATFORM_MAX_CHANNELS]; + int32_t volume[PLATFORM_MAX_CHANNELS]; struct tone_state sg; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, struct comp_buffer *source, uint32_t frames); - };
/* @@ -109,8 +111,12 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, */ sine_sample = tonegen(&cd->sg); n -= nch; + /* Multiply Q1.31 sine with channel specific + * Q16.16 volume. Stream is Q1.31. */ for (i = 0; i < nch; i++) { - *dest = sine_sample; + *dest = sat_int32(q_mults_32x32( + sine_sample, + cd->volume[i], 31, 16, 31)); dest++; } } @@ -122,14 +128,17 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, n -= nch; n_wrap_dest -= nch; for (i = 0; i < nch; i++) { - *dest = sine_sample; + *dest = sat_int32(q_mults_32x32( + sine_sample, + cd->volume[i], 31, 16, 31)); dest++; } } /* No need to check if past end_addr, * it is so just subtract buffer size. */ - dest = (int32_t *) ((size_t) dest - sink->alloc_size); + dest = (int32_t *) ((size_t) dest + - sink->ipc_buffer.size); } } sink->w_ptr = dest; @@ -364,6 +373,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) { struct comp_dev *dev; struct comp_data *cd; + int i;
trace_tone("TNw"); dev = rmalloc(RZONE_RUNTIME, RFLAGS_NONE, sizeof(*dev)); @@ -381,8 +391,12 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp)
comp_set_drvdata(dev, cd); comp_set_endpoint(dev); + 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++) + cd->volume[i] = TONE_VOLUME_DEFAULT;
return dev; } @@ -400,6 +414,9 @@ static void tone_free(struct comp_dev *dev) /* set component audio stream parameters */ static int tone_params(struct comp_dev *dev, struct stream_params *params) { + int i; + struct comp_data *cd = comp_get_drvdata(dev); + trace_tone("TPa");
/* Tone generator supports only S32_LE PCM format */ @@ -407,6 +424,10 @@ static int tone_params(struct comp_dev *dev, struct stream_params *params) || (params->pcm->frame_fmt != SOF_IPC_FRAME_S32_LE)) return -EINVAL;
+ /* Copy channels map that is used by volume IPC */ + for (i = 0; i < params->pcm->channels; i++) + cd->chan[i] = params->pcm->channel_map[i]; + /* Don't do any data transformation */ comp_set_sink_params(dev, params);
@@ -417,22 +438,31 @@ static int tone_params(struct comp_dev *dev, struct stream_params *params) static int tone_cmd(struct comp_dev *dev, int cmd, void *data) { struct comp_data *cd = comp_get_drvdata(dev); - struct sof_ipc_comp_tone *cv; + struct sof_ipc_comp_tone *ct; + struct sof_ipc_ctrl_values *cv; + int i, j; trace_tone("TCm");
switch (cmd) { case COMP_CMD_TONE: trace_tone("Tto"); - cv = (struct sof_ipc_comp_tone *) data; + ct = (struct sof_ipc_comp_tone *) data; /* Ignore channels while tone implementation is mono */ - tonegen_set_f(&cd->sg, cv->frequency); - tonegen_set_a(&cd->sg, cv->amplitude); - tonegen_set_sweep(&cd->sg, cv->freq_mult, cv->ampl_mult, - cv->length, cv->period, cv->repeats); - tonegen_set_linramp(&cd->sg, cv->ramp_step); + tonegen_set_f(&cd->sg, ct->frequency); + tonegen_set_a(&cd->sg, ct->amplitude); + tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult, + ct->length, ct->period, ct->repeats); + tonegen_set_linramp(&cd->sg, ct->ramp_step); break; case COMP_CMD_VOLUME: trace_tone("TVo"); + cv = (struct sof_ipc_ctrl_values *) data; + for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) { + for (j = 0; j < cv->num_values; j++) { + if (cv->values[j].channel == cd->chan[i]) + cd->volume[i] = cv->values[j].value; + } + } break; case COMP_CMD_MUTE: trace_tone("TMu"); @@ -496,7 +526,6 @@ static int tone_copy(struct comp_dev *dev) */ need_sink = cframes * sink->params.pcm->frame_size; if (sink->free >= need_sink) { - /* create tone */ cd->tone_func(dev, sink, source, cframes); } @@ -517,7 +546,6 @@ static int tone_prepare(struct comp_dev *dev) trace_value(sink->params.pcm->channels); trace_value(sink->params.pcm->rate);
- cd->tone_func = tone_s32_default; f = tonegen_get_f(&cd->sg); a = tonegen_get_a(&cd->sg); if (tonegen_init(&cd->sg, sink->params.pcm->rate, f, a) < 0)
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@
#define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
thanks, ~Keyon
static int32_t tonegen(struct tone_state *sg); static void tonegen_control(struct tone_state *sg); @@ -77,10 +78,11 @@ static const int32_t tone_pi2_div_fs[TONE_NUM_FS] = {
/* TODO: Remove *source when internal endpoint is possible */ struct comp_data {
- enum sof_ipc_chmap chan[PLATFORM_MAX_CHANNELS];
- int32_t volume[PLATFORM_MAX_CHANNELS]; struct tone_state sg; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, struct comp_buffer *source, uint32_t frames);
};
/*
@@ -109,8 +111,12 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, */ sine_sample = tonegen(&cd->sg); n -= nch;
/* Multiply Q1.31 sine with channel specific* Q16.16 volume. Stream is Q1.31. */ for (i = 0; i < nch; i++) {
*dest = sine_sample;
*dest = sat_int32(q_mults_32x32(sine_sample,cd->volume[i], 31, 16, 31)); dest++; } }@@ -122,14 +128,17 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, n -= nch; n_wrap_dest -= nch; for (i = 0; i < nch; i++) {
*dest = sine_sample;
*dest = sat_int32(q_mults_32x32(sine_sample,cd->volume[i], 31, 16, 31)); dest++; } } /* No need to check if past end_addr, * it is so just subtract buffer size. */
dest = (int32_t *) ((size_t) dest - sink->alloc_size);
dest = (int32_t *) ((size_t) dest- sink->ipc_buffer.size);@@ -364,6 +373,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) { struct comp_dev *dev; struct comp_data *cd;
int i;
trace_tone("TNw"); dev = rmalloc(RZONE_RUNTIME, RFLAGS_NONE, sizeof(*dev));
@@ -381,8 +391,12 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp)
comp_set_drvdata(dev, cd); comp_set_endpoint(dev);
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++)
cd->volume[i] = TONE_VOLUME_DEFAULT;return dev; }
@@ -400,6 +414,9 @@ static void tone_free(struct comp_dev *dev) /* set component audio stream parameters */ static int tone_params(struct comp_dev *dev, struct stream_params *params) {
int i;
struct comp_data *cd = comp_get_drvdata(dev);
trace_tone("TPa");
/* Tone generator supports only S32_LE PCM format */
@@ -407,6 +424,10 @@ static int tone_params(struct comp_dev *dev, struct stream_params *params) || (params->pcm->frame_fmt != SOF_IPC_FRAME_S32_LE)) return -EINVAL;
- /* Copy channels map that is used by volume IPC */
- for (i = 0; i < params->pcm->channels; i++)
cd->chan[i] = params->pcm->channel_map[i];- /* Don't do any data transformation */ comp_set_sink_params(dev, params);
@@ -417,22 +438,31 @@ static int tone_params(struct comp_dev *dev, struct stream_params *params) static int tone_cmd(struct comp_dev *dev, int cmd, void *data) { struct comp_data *cd = comp_get_drvdata(dev);
- struct sof_ipc_comp_tone *cv;
struct sof_ipc_comp_tone *ct;
struct sof_ipc_ctrl_values *cv;
int i, j; trace_tone("TCm");
switch (cmd) { case COMP_CMD_TONE: trace_tone("Tto");
cv = (struct sof_ipc_comp_tone *) data;
ct = (struct sof_ipc_comp_tone *) data;
tonegen_set_f(&cd->sg, cv->frequency);tonegen_set_a(&cd->sg, cv->amplitude);tonegen_set_sweep(&cd->sg, cv->freq_mult, cv->ampl_mult,cv->length, cv->period, cv->repeats);tonegen_set_linramp(&cd->sg, cv->ramp_step);
tonegen_set_f(&cd->sg, ct->frequency);tonegen_set_a(&cd->sg, ct->amplitude);tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult,ct->length, ct->period, ct->repeats);tonegen_set_linramp(&cd->sg, ct->ramp_step);cv = (struct sof_ipc_ctrl_values *) data;for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) {for (j = 0; j < cv->num_values; j++) {if (cv->values[j].channel == cd->chan[i])cd->volume[i] = cv->values[j].value;}}@@ -496,7 +526,6 @@ static int tone_copy(struct comp_dev *dev) */ need_sink = cframes * sink->params.pcm->frame_size; if (sink->free >= need_sink) {
- /* create tone */ cd->tone_func(dev, sink, source, cframes); }
@@ -517,7 +546,6 @@ static int tone_prepare(struct comp_dev *dev) trace_value(sink->params.pcm->channels); trace_value(sink->params.pcm->rate);
- cd->tone_func = tone_s32_default; f = tonegen_get_f(&cd->sg); a = tonegen_get_a(&cd->sg); if (tonegen_init(&cd->sg, sink->params.pcm->rate, f, a) < 0)
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@ #define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
Thanks, Seppo
thanks, ~Keyon
static int32_t tonegen(struct tone_state *sg);static void tonegen_control(struct tone_state *sg); @@ -77,10 +78,11 @@ static const int32_t tone_pi2_div_fs[TONE_NUM_FS] = { /* TODO: Remove *source when internal endpoint is possible */ struct comp_data {
- enum sof_ipc_chmap chan[PLATFORM_MAX_CHANNELS];
- int32_t volume[PLATFORM_MAX_CHANNELS]; struct tone_state sg; void (*tone_func)(struct comp_dev *dev, struct comp_buffer *sink, struct comp_buffer *source, uint32_t frames);
- }; /*
@@ -109,8 +111,12 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, */ sine_sample = tonegen(&cd->sg); n -= nch;
/* Multiply Q1.31 sine with channel specific* Q16.16 volume. Stream is Q1.31. */ for (i = 0; i < nch; i++) {
*dest = sine_sample;
*dest = sat_int32(q_mults_32x32(sine_sample,cd->volume[i], 31, 16, 31)); dest++; } }@@ -122,14 +128,17 @@ static void tone_s32_default(struct comp_dev *dev, struct comp_buffer *sink, n -= nch; n_wrap_dest -= nch; for (i = 0; i < nch; i++) {
*dest = sine_sample;
*dest = sat_int32(q_mults_32x32(sine_sample,cd->volume[i], 31, 16, 31)); dest++; } } /* No need to check if past end_addr, * it is so just subtract buffer size. */
dest = (int32_t *) ((size_t) dest - sink->alloc_size);
dest = (int32_t *) ((size_t) dest- sink->ipc_buffer.size); } } sink->w_ptr = dest;@@ -364,6 +373,7 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) { struct comp_dev *dev; struct comp_data *cd;
- int i; trace_tone("TNw"); dev = rmalloc(RZONE_RUNTIME, RFLAGS_NONE, sizeof(*dev));
@@ -381,8 +391,12 @@ static struct comp_dev *tone_new(struct sof_ipc_comp *comp) comp_set_drvdata(dev, cd); comp_set_endpoint(dev);
- cd->tone_func = tone_s32_default;
tonegen_reset(&cd->sg);
- /* Reset tone generator and set channels volumes to default */
- for (i = 0; i < PLATFORM_MAX_CHANNELS; i++)
cd->volume[i] = TONE_VOLUME_DEFAULT; return dev;@@ -400,6 +414,9 @@ static void tone_free(struct comp_dev *dev) /* set component audio stream parameters */ static int tone_params(struct comp_dev *dev, struct stream_params *params) {
- int i;
- struct comp_data *cd = comp_get_drvdata(dev);
trace_tone("TPa"); /* Tone generator supports only S32_LE PCM format */@@ -407,6 +424,10 @@ static int tone_params(struct comp_dev *dev, struct stream_params *params) || (params->pcm->frame_fmt != SOF_IPC_FRAME_S32_LE)) return -EINVAL;
- /* Copy channels map that is used by volume IPC */
- for (i = 0; i < params->pcm->channels; i++)
cd->chan[i] = params->pcm->channel_map[i];/* Don't do any data transformation */ comp_set_sink_params(dev, params);struct stream_params *params) static int tone_cmd(struct comp_dev *dev, int cmd, void *data) { struct comp_data *cd = comp_get_drvdata(dev);
- struct sof_ipc_comp_tone *cv;
- struct sof_ipc_comp_tone *ct;
- struct sof_ipc_ctrl_values *cv;
- int i, j; trace_tone("TCm"); switch (cmd) { case COMP_CMD_TONE: trace_tone("Tto");
cv = (struct sof_ipc_comp_tone *) data;
ct = (struct sof_ipc_comp_tone *) data; /* Ignore channels while tone implementation is mono */
tonegen_set_f(&cd->sg, cv->frequency);tonegen_set_a(&cd->sg, cv->amplitude);tonegen_set_sweep(&cd->sg, cv->freq_mult, cv->ampl_mult,cv->length, cv->period, cv->repeats);tonegen_set_linramp(&cd->sg, cv->ramp_step);
tonegen_set_f(&cd->sg, ct->frequency);tonegen_set_a(&cd->sg, ct->amplitude);tonegen_set_sweep(&cd->sg, ct->freq_mult, ct->ampl_mult,ct->length, ct->period, ct->repeats);tonegen_set_linramp(&cd->sg, ct->ramp_step); break; case COMP_CMD_VOLUME: trace_tone("TVo");cv = (struct sof_ipc_ctrl_values *) data;for (i = 0; i < PLATFORM_MAX_CHANNELS; i++) {for (j = 0; j < cv->num_values; j++) {if (cv->values[j].channel == cd->chan[i])cd->volume[i] = cv->values[j].value;}} break; case COMP_CMD_MUTE: trace_tone("TMu");@@ -496,7 +526,6 @@ static int tone_copy(struct comp_dev *dev) */ need_sink = cframes * sink->params.pcm->frame_size; if (sink->free >= need_sink) {
/* create tone */ cd->tone_func(dev, sink, source, cframes); }@@ -517,7 +546,6 @@ static int tone_prepare(struct comp_dev *dev) trace_value(sink->params.pcm->channels); trace_value(sink->params.pcm->rate);
- cd->tone_func = tone_s32_default; f = tonegen_get_f(&cd->sg); a = tonegen_get_a(&cd->sg); if (tonegen_init(&cd->sg, sink->params.pcm->rate, f, a) < 0)
Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@ #define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Takashi
On 22.06.2017 11:28, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@ #define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Yes, that works for integers and it is readable but the needed frequencies are often decimal numbers in Hz. E.g. C5 note is 523.251 Hz or if a sweep is started from 4th third octave band the frequency would be 31.5 Hz.
So I myself do with Octave that I use as pocket calculator
printf('%d\n',round(523.251*2^16))
34291778
printf('%d\n',round(31.5*2^16))
2064384
printf('%d\n',round(10^(-60/20)*2^31))
2147484
Cheers, Seppo
Takashi
On Thu, 22 Jun 2017 10:47:54 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 11:28, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56 ++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@ #define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Yes, that works for integers and it is readable but the needed frequencies are often decimal numbers in Hz. E.g. C5 note is 523.251 Hz or if a sweep is started from 4th third octave band the frequency would be 31.5 Hz.
Yeah, the decimal point is tricky.
So I myself do with Octave that I use as pocket calculator
printf('%d\n',round(523.251*2^16))
34291778
printf('%d\n',round(31.5*2^16))
2064384
printf('%d\n',round(10^(-60/20)*2^31))
2147484
I thought C compiler (not preprocessor) should optimize the conversion and handle as an integer constant.
#define TONE_FREQ(f) (int)((f) * (2 << 16)) /* f = float */
and int x = TONE_FREQ(523.251);
should be converted to
int x = 34291778;
Takashi
On 22.06.2017 12:02, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:47:54 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 11:28, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote:
The mono tone level can be controlled per channel. A similar channel map feature as in the volume component was added into volume command of tone.
The default tone frequency of 997 Hz was updated to a proper Q16.16 value.
Moved tone function pointer set to tone_new() since it is not modified later.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com
src/audio/tone.c | 56++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/src/audio/tone.c b/src/audio/tone.c index f45d8be..d1c7468 100644 --- a/src/audio/tone.c +++ b/src/audio/tone.c @@ -56,7 +56,8 @@ #define TONE_NUM_FS 13 /* Table size for 8-192 kHz range */ #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full scale */ -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */
will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Yes, that works for integers and it is readable but the needed frequencies are often decimal numbers in Hz. E.g. C5 note is 523.251 Hz or if a sweep is started from 4th third octave band the frequency would be 31.5 Hz.
Yeah, the decimal point is tricky.
So I myself do with Octave that I use as pocket calculator
printf('%d\n',round(523.251*2^16))
34291778
printf('%d\n',round(31.5*2^16))
2064384
printf('%d\n',round(10^(-60/20)*2^31))
2147484
I thought C compiler (not preprocessor) should optimize the conversion and handle as an integer constant.
#define TONE_FREQ(f) (int)((f) * (2 << 16)) /* f = float */
and int x = TONE_FREQ(523.251);
should be converted to
int x = 34291778;
Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing does the conversion to fixed without increase of the firmware image size. I also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro since compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
If there's no concern from other people who know better the compilers I'm OK to make this change to increase readability and help avoiding mistakes with frequencies.
Seppo
Takashi
Sorry, a mistake in the macro below:
On 22.06.2017 14:50, Seppo Ingalsuo wrote:
On 22.06.2017 12:02, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:47:54 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 11:28, Takashi Iwai wrote:
On Thu, 22 Jun 2017 10:04:27 +0200, Seppo Ingalsuo wrote:
On 22.06.2017 04:31, Keyon Jie wrote:
On 2017年06月22日 01:12, Seppo Ingalsuo wrote: > The mono tone level can be controlled per channel. A similar > channel map > feature as in the volume component was added into volume command of > tone. > > The default tone frequency of 997 Hz was updated to a proper Q16.16 > value. > > Moved tone function pointer set to tone_new() since it is not > modified > later. > > Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com > --- > src/audio/tone.c | 56 > ++++++++++++++++++++++++++++++++++++++++++-------------- > 1 file changed, 42 insertions(+), 14 deletions(-) > > diff --git a/src/audio/tone.c b/src/audio/tone.c > index f45d8be..d1c7468 100644 > --- a/src/audio/tone.c > +++ b/src/audio/tone.c > @@ -56,7 +56,8 @@ > #define TONE_NUM_FS 13 /* Table size for > 8-192 > kHz range */ > #define TONE_AMPLITUDE_DEFAULT 2147484 /* -60 dB from full > scale */ > -#define TONE_FREQUENCY_DEFAULT 16334848 /* 997 in Q18.14 */ > +#define TONE_FREQUENCY_DEFAULT 65339392 /* 997 Hz in Q16.16 */ > +#define TONE_VOLUME_DEFAULT 65536 /* 1.0 in Q16.16 */ will they be more readable in hexadecimal?
Unfortunately the C preprocessor doesn't process float to fixed math so I can't use any nice macro to create these. In hex it would look like this and in my opinion it's not any better. The Q notation is only sometimes by chance such that integer and decimal bits are not mixed into same hex digit. For frequency Qx.y notation the used x is sufficient to get to ultrasound frequencies and y to make precise enough sweeps.
#define TONE_AMPLITUDE_DEFAULT 0x20C49C /* -60 dB from full scale */ #define TONE_FREQUENCY_DEFAULT 0xF940000 /* 997 Hz in Q18.14 */ #define TONE_FREQUENCY_DEFAULT 0x3E50000 /* 997 Hz in Q16.16 */ #define TONE_VOLUME_DEFAULT 0x10000 /* 1.0 in Q16.16 */
To convert these with any calculator to floats when debugging code (these are fractional numbers) there's extra effort needed with hex so I definitely prefer decimal. Hex is good for bit masks but these are not such.
It's a fixed-point, so you can introduce a simple macro like
#define TONE_FREQ_SHIFT 16 #define TONE_FREQ(x, y) ((x) << TONE_FREQ_SHIFT | (y))
then TONE_FREQUENCY_DEFAULT will be TONE_FREQ(997, 0), and so on. If you need to change the base shift, just change the TONE_FREQ_SHIFT and the rest remains as is.
Yes, that works for integers and it is readable but the needed frequencies are often decimal numbers in Hz. E.g. C5 note is 523.251 Hz or if a sweep is started from 4th third octave band the frequency would be 31.5 Hz.
Yeah, the decimal point is tricky.
So I myself do with Octave that I use as pocket calculator
printf('%d\n',round(523.251*2^16))
34291778
printf('%d\n',round(31.5*2^16))
2064384
printf('%d\n',round(10^(-60/20)*2^31))
2147484
I thought C compiler (not preprocessor) should optimize the conversion and handle as an integer constant.
#define TONE_FREQ(f) (int)((f) * (2 << 16)) /* f = float */
and int x = TONE_FREQ(523.251);
should be converted to
int x = 34291778;Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing does the conversion to fixed without increase of the firmware image size. I also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro since compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
Sorry, the rounded version overflows and depending on the way 20000 vs. 20000.0 as the parameter the compiler may even pass it silently. However this way the rounded Q16.16 conversion seems to work:
#define TONE_FREQ(f) (int)(((int64_t)((f) * 131072.0 + 1.0)) >> 1) /* f = float */
If there's no concern from other people who know better the compilers I'm OK to make this change to increase readability and help avoiding mistakes with frequencies.
Seppo
Takashi
On Thu, 2017-06-22 at 15:22 +0300, Seppo Ingalsuo wrote:
Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing
does
the conversion to fixed without increase of the firmware image size.
I
also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro
since
compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
Sorry, the rounded version overflows and depending on the way 20000 vs. 20000.0 as the parameter the compiler may even pass it silently. However this way the rounded Q16.16 conversion seems to work:
#define TONE_FREQ(f) (int)(((int64_t)((f) * 131072.0 + 1.0)) >> 1) /* f = float */
Best to make 131072.0 a macro too so easy to understand what it's doing.
We should also have this as our non optimal "generic" version and we should look to create arch specific macros (with inline assembler) later on (that take take of overflow or larger types).
Liam
--------------------------------------------------------------------- Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On 6/26/17 5:28 AM, Liam Girdwood wrote:
On Thu, 2017-06-22 at 15:22 +0300, Seppo Ingalsuo wrote:
Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing
does
the conversion to fixed without increase of the firmware image size.
I
also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro
since
compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
Sorry, the rounded version overflows and depending on the way 20000 vs. 20000.0 as the parameter the compiler may even pass it silently. However this way the rounded Q16.16 conversion seems to work:
#define TONE_FREQ(f) (int)(((int64_t)((f) * 131072.0 + 1.0)) >> 1) /* f = float */
Best to make 131072.0 a macro too so easy to understand what it's doing.
Hehe, I've done things like this in 1999...we should be able to compile all this DSP code in both fixed and floating point formats, with macros the tables can be generated in the two modes and the low-level code can be maintained with two different implementations of the baseline. I used a 'Fract' typedef to represent both fixed point and floating point representations.
It helps if all your floating point constants are represented as [-1.0,1.0[ since it's a direct map with the fractional representation. If you want more bits for the integer part then you can add this as an argument to the macro. You can also add a MAXVAL if you want to quantize with less than 32 bits total.
We should also have this as our non optimal "generic" version and we should look to create arch specific macros (with inline assembler) later on (that take take of overflow or larger types).
Liam
Intel Corporation (UK) Limited Registered No. 1134945 (England) Registered Office: Pipers Way, Swindon SN3 1RJ VAT No: 860 2173 47
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ Sound-open-firmware mailing list Sound-open-firmware@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
On Thu, 2017-06-22 at 14:50 +0300, Seppo Ingalsuo wrote:
Thanks, that works. At least with the compiler that I use (http://alsa-project.org/main/index.php/Firmware) the optimizing does the conversion to fixed without increase of the firmware image size. I also checked from disassembly that this part of the code remains identical. I did a minor fix and added rounding into the macro since compiler doesn't round the cast to int:
#define TONE_FREQ(f) (int)((f) * (1 << 16)) /* f = float */
#define TONE_FREQ(f) (((int)((f) * (1 << 17)) + 1) >> 1) /* f = float */
If there's no concern from other people who know better the compilers I'm OK to make this change to increase readability and help avoiding mistakes with frequencies.
Ok, by me.
Liam
A free of null pointer is fixed with this change. Also the possibility of nondefined number of channels in EQ config or switch command is eliminated by by initializing the FIR for max channels.
Some trace output is added for FIR setup.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/eq_fir.c | 28 ++++++++++++++++++++-------- src/audio/eq_fir.h | 2 +- src/include/uapi/ipc.h | 6 ++++++ 3 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/src/audio/eq_fir.c b/src/audio/eq_fir.c index 5617313..d5f9738 100644 --- a/src/audio/eq_fir.c +++ b/src/audio/eq_fir.c @@ -175,6 +175,9 @@ static int eq_fir_setup(struct fir_state_32x16 fir[], } }
+ /* Free existing FIR channels data if it was allocated */ + eq_fir_free_delaylines(fir); + /* Initialize 1st phase */ for (i = 0; i < nch; i++) { resp = config->assign_response[i]; @@ -191,8 +194,6 @@ static int eq_fir_setup(struct fir_state_32x16 fir[], }
} - /* Free existing FIR channels data if it was allocated */ - eq_fir_free_delaylines(fir);
/* Allocate all FIR channels data in a big chunk and clear it */ fir_data = rballoc(RZONE_SYS, RFLAGS_NONE, @@ -306,11 +307,9 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data) { trace_src("ECm"); struct comp_data *cd = comp_get_drvdata(dev); - struct comp_buffer *source = list_first_item(&dev->bsource_list, - struct comp_buffer, sink_list); struct sof_ipc_eq_fir_blob *blob; struct sof_ipc_eq_fir_switch *assign; - + struct eq_fir_update *fir_update; int i; size_t bs;
@@ -318,9 +317,15 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data) case COMP_CMD_EQ_FIR_SWITCH: trace_src("EFx"); assign = (struct sof_ipc_eq_fir_switch *) data; + fir_update = (struct eq_fir_update *) assign->data; eq_fir_switch_response(cd->fir, cd->config, - (struct eq_fir_update *) assign->data, - source->params.pcm->channels); + fir_update, PLATFORM_MAX_CHANNELS); + + /* Print trace information */ + tracev_value(iir_update->stream_max_channels); + for (i = 0; i < fir_update->stream_max_channels; i++) + tracev_value(iir_update->assign_response[i]); + break; case COMP_CMD_EQ_FIR_CONFIG: trace_src("EFc"); @@ -338,7 +343,14 @@ static int eq_fir_cmd(struct comp_dev *dev, int cmd, void *data) return -EINVAL;
memcpy(cd->config, blob->data, bs); - eq_fir_setup(cd->fir, cd->config, source->params.pcm->channels); + eq_fir_setup(cd->fir, cd->config, PLATFORM_MAX_CHANNELS); + + /* Print trace information */ + tracev_value(cd->config->stream_max_channels); + tracev_value(cd->config->number_of_responses_defined); + for (i = 0; i < cd->config->stream_max_channels; i++) + tracev_value(cd->config->assign_response[i]); + break; case COMP_CMD_MUTE: trace_src("EFm"); diff --git a/src/audio/eq_fir.h b/src/audio/eq_fir.h index 0b5dbb1..7579ba4 100644 --- a/src/audio/eq_fir.h +++ b/src/audio/eq_fir.h @@ -52,7 +52,7 @@
#define NHEADER_EQ_FIR_BLOB 2 /* Header is two words plus assigns plus coef */
-#define EQ_FIR_MAX_BLOB_SIZE 4096 /* Max size allowed for blob */ +#define EQ_FIR_MAX_BLOB_SIZE 4096 /* Max size allowed for blob in bytes */
struct eq_fir_configuration { uint16_t stream_max_channels; diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index d89039e..db7a8bb 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -511,6 +511,12 @@ struct sof_ipc_comp_tone { int32_t ramp_step; } __attribute__((packed));
+/* FIR equalizer component */ +struct sof_ipc_comp_eq_fir { + struct sof_ipc_comp comp; + struct sof_ipc_pcm_comp pcm; +} __attribute__((packed)); + /* IPC to pass configuration blobs to equalizers and re-assign responses */ struct sof_ipc_eq_fir_blob { struct sof_ipc_comp comp;
Initialize EQ for max number of channels in configuration or switch commands to avoid not set number of channels.
Some trace output is added into EQ setup and some comments cleaned and added.
Signed-off-by: Seppo Ingalsuo seppo.ingalsuo@linux.intel.com --- src/audio/eq_iir.c | 35 +++++++++++++++++++++++++---------- src/audio/eq_iir.h | 2 +- src/include/uapi/ipc.h | 6 ++++++ 3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/src/audio/eq_iir.c b/src/audio/eq_iir.c index 8d12895..bf66000 100644 --- a/src/audio/eq_iir.c +++ b/src/audio/eq_iir.c @@ -312,19 +312,24 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data) { trace_eq_iir("ECm"); struct comp_data *cd = comp_get_drvdata(dev); - struct comp_buffer *source = list_first_item(&dev->bsource_list, - struct comp_buffer, sink_list); struct sof_ipc_eq_iir_switch *assign; struct sof_ipc_eq_iir_blob *blob; + struct eq_iir_update *iir_update; int i; size_t bs; switch (cmd) { case COMP_CMD_EQ_IIR_SWITCH: trace_eq_iir("EFx"); assign = (struct sof_ipc_eq_iir_switch *) data; + iir_update = (struct eq_iir_update *) assign->data; eq_iir_switch_response(cd->iir, cd->config, - (struct eq_iir_update *) assign->data, - source->params.pcm->channels); + iir_update, PLATFORM_MAX_CHANNELS); + + /* Print trace information */ + tracev_value(iir_update->stream_max_channels); + for (i = 0; i < iir_update->stream_max_channels; i++) + tracev_value(iir_update->assign_response[i]); + break; case COMP_CMD_EQ_IIR_CONFIG: trace_eq_iir("EFc"); @@ -333,7 +338,8 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data)
/* Copy new config, need to decode data to know the size */ blob = (struct sof_ipc_eq_iir_blob *) data; - bs = blob->comp.hdr.size - sizeof(struct sof_ipc_hdr); + bs = blob->comp.hdr.size - sizeof(struct sof_ipc_hdr) + - sizeof(struct sof_ipc_host_buffer); if (bs > EQ_IIR_MAX_BLOB_SIZE) return -EINVAL;
@@ -343,7 +349,16 @@ static int eq_iir_cmd(struct comp_dev *dev, int cmd, void *data) return -EINVAL;
memcpy(cd->config, blob->data, bs); - eq_iir_setup(cd->iir, cd->config, source->params.pcm->channels); + /* Initialize all channels, the actual number of channels may + * not be set yet. */ + eq_iir_setup(cd->iir, cd->config, PLATFORM_MAX_CHANNELS); + + /* Print trace information */ + tracev_value(cd->config->stream_max_channels); + tracev_value(cd->config->number_of_responses_defined); + for (i = 0; i < cd->config->stream_max_channels; i++) + tracev_value(cd->config->assign_response[i]); + break; case COMP_CMD_MUTE: trace_eq_iir("EFm"); @@ -409,9 +424,6 @@ static int eq_iir_copy(struct comp_dev *dev) /* Check that source has enough frames available and that sink has * enough frames free. */ - //frames = source->params.period_frames; - //need_source = frames * source->params.frame_size; - //need_sink = frames * sink->params.frame_size; frames = source->params.pcm->period_count; need_source = frames * source->params.pcm->frame_size; need_sink = frames * sink->params.pcm->frame_size; @@ -432,7 +444,10 @@ static int eq_iir_prepare(struct comp_dev *dev)
cd->eq_iir_func = eq_iir_s32_default;
- /* Initialize EQ */ + /* Initialize EQ. Note that if EQ has not received command to + * configure the response the EQ prepare returns an error that + * interrupts pipeline prepare for downstream. + */ if (cd->config == NULL) return -EINVAL;
diff --git a/src/audio/eq_iir.h b/src/audio/eq_iir.h index 2634986..42d95c1 100644 --- a/src/audio/eq_iir.h +++ b/src/audio/eq_iir.h @@ -64,7 +64,7 @@ * {0, 0, 0, 0, 1073741824, 0, 16484} */
-#define EQ_IIR_MAX_BLOB_SIZE 1024 +#define EQ_IIR_MAX_BLOB_SIZE 1024 /* In bytes or size_t */
#define NHEADER_EQ_IIR_BLOB 2 /* Blob is two words plus asssigns plus coef */
diff --git a/src/include/uapi/ipc.h b/src/include/uapi/ipc.h index db7a8bb..ea9d1c6 100644 --- a/src/include/uapi/ipc.h +++ b/src/include/uapi/ipc.h @@ -517,6 +517,12 @@ struct sof_ipc_comp_eq_fir { struct sof_ipc_pcm_comp pcm; } __attribute__((packed));
+/* IIR equalizer component */ +struct sof_ipc_comp_eq_iir { + struct sof_ipc_comp comp; + struct sof_ipc_pcm_comp pcm; +} __attribute__((packed)); + /* IPC to pass configuration blobs to equalizers and re-assign responses */ struct sof_ipc_eq_fir_blob { struct sof_ipc_comp comp;
participants (6)
-
Keyon Jie -
Liam Girdwood -
Liam Girdwood -
Pierre-Louis Bossart -
Seppo Ingalsuo -
Takashi Iwai