Skip to content

kpb: add support 24/24bit buffering for 1-6 channels #6785

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mwasko merged 4 commits into from
Jan 30, 2023
Merged

kpb: add support 24/24bit buffering for 1-6 channels #6785

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a590867
memory: expand HEAPMEM_SIZE
fkwasowi Dec 13, 2022
5a072ac
kpb: set sync_draining_mode to true
fkwasowi Dec 13, 2022
57edd8b
kpb: add support for 1-6 channels
fkwasowi Dec 13, 2022
b93d091
kpb: add functions supporting 24-bit format
fkwasowi Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
225 changes: 215 additions & 10 deletions src/audio/kpb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ static void kpb_free_history_buffer(struct history_buffer *buff);
static inline bool kpb_is_sample_width_supported(uint32_t sampling_width);
static void kpb_copy_samples(struct comp_buffer __sparse_cache *sink,
struct comp_buffer __sparse_cache *source, size_t size,
size_t sample_width);
size_t sample_width, uint32_t channels);
static void kpb_drain_samples(void *source, struct audio_stream __sparse_cache *sink,
size_t size, size_t sample_width);
static void kpb_buffer_samples(const struct audio_stream __sparse_cache *source,
Expand Down Expand Up @@ -224,7 +224,7 @@ static void kpb_set_params(struct comp_dev *dev,
params->sample_valid_bytes =
kpb->ipc4_cfg.base_cfg.audio_fmt.valid_bit_depth / 8;
params->buffer_fmt = kpb->ipc4_cfg.base_cfg.audio_fmt.interleaving_style;
params->buffer.size = kpb->ipc4_cfg.base_cfg.obs * KPB_MAX_BUFF_TIME * 2;
params->buffer.size = kpb->ipc4_cfg.base_cfg.obs * KPB_MAX_BUFF_TIME * params->channels;

params->host_period_bytes = params->channels *
params->sample_container_bytes *
Expand Down Expand Up @@ -704,7 +704,7 @@ static int kpb_prepare(struct comp_dev *dev)
struct comp_data *kpb = comp_get_drvdata(dev);
int ret = 0;
int i;
size_t hb_size_req = KPB_MAX_BUFFER_SIZE(kpb->config.sampling_width);
size_t hb_size_req = KPB_MAX_BUFFER_SIZE(kpb->config.sampling_width, kpb->config.channels);

comp_dbg(dev, "kpb_prepare()");

Expand Down Expand Up @@ -814,8 +814,7 @@ static int kpb_prepare(struct comp_dev *dev)
ret = -EIO;
}

/* Disallow sync_draining_mode for now */
kpb->sync_draining_mode = false;
kpb->sync_draining_mode = true;
Comment thread
fkwasowi marked this conversation as resolved.
Outdated

kpb_change_state(kpb, KPB_STATE_RUN);

Expand Down Expand Up @@ -915,6 +914,7 @@ static int kpb_copy(struct comp_dev *dev)
size_t sample_width = kpb->config.sampling_width;
struct draining_data *dd = &kpb->draining_task_data;
uint32_t avail_bytes;
uint32_t channels = kpb->config.channels;

comp_dbg(dev, "kpb_copy()");

Expand Down Expand Up @@ -966,7 +966,7 @@ static int kpb_copy(struct comp_dev *dev)
break;
}

kpb_copy_samples(sink_c, source_c, copy_bytes, sample_width);
kpb_copy_samples(sink_c, source_c, copy_bytes, sample_width, channels);

/* Buffer source data internally in history buffer for future
* use by clients.
Expand Down Expand Up @@ -1026,7 +1026,7 @@ static int kpb_copy(struct comp_dev *dev)
break;
}

kpb_copy_samples(sink_c, source_c, copy_bytes, sample_width);
kpb_copy_samples(sink_c, source_c, copy_bytes, sample_width, channels);

comp_update_buffer_produce(sink_c, copy_bytes);
comp_update_buffer_consume(source_c, copy_bytes);
Expand Down Expand Up @@ -1594,6 +1594,77 @@ static enum task_state kpb_draining_task(void *arg)
return SOF_TASK_STATE_COMPLETED;
}

#ifdef KPB_HIFI3
static void kpb_convert_24b_to_32b(const void *linear_source, int ioffset,
struct audio_stream __sparse_cache *sink, int ooffset,
unsigned int n_samples)
{
int ssize = audio_stream_sample_bytes(sink);
uint8_t *in = (uint8_t *)linear_source + ioffset * ssize;
uint8_t *out = audio_stream_wrap(sink, (uint8_t *)sink->w_ptr + ooffset * ssize);
ae_int32x2 *buf_end;
ae_int32x2 *buf;

buf = (ae_int32x2 *)(sink->addr);
buf_end = (ae_int32x2 *)(sink->end_addr);
ae_int32x2 *out_ptr = (ae_int32x2 *)buf;

AE_SETCBEGIN0(buf);
AE_SETCEND0(buf_end);
out_ptr = (ae_int32x2 *)out;

ae_valign align_in = AE_LA64_PP(in);
int i = 0;
ae_int24x2 d24 = AE_ZERO24();

if (!IS_ALIGNED((uintptr_t)out_ptr, 8)) {
AE_LA24_IP(d24, align_in, in);
ae_int32x2 d320 = d24;
int higher = AE_MOVAD32_H(d320);
*(ae_int32 *)(out_ptr) = higher << 8;
out_ptr = (ae_int32x2 *)(out + 4);

@andrula-song andrula-song Jan 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and for those instructions,
" ae_int32x2 d320 = d24;
int higher = AE_MOVAD32_H(d320);
*(ae_int32 *)(out_ptr) = higher << 8;
out_ptr = (ae_int32x2 *)(out + 4); "
we can use AE_S32F24_L_IP(d24, out_ptr, 4); to replace that.

@fkwasowi fkwasowi Jan 16, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After such a change kpb stopped working

@andrula-song andrula-song Jan 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the H part is the same with L part of d24, so both AE_S32F24_L_IP(d24, out_ptr, 4); or AE_S32F24_H_IP(d24, out_ptr, 4); is work.

@fkwasowi fkwasowi Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does not compile because the arguments are not appropriate for this function. When changing argument types, the function does not work.

++i;
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
}
/* process two samples in single iteration to increase performance */
while (i < (int)n_samples - 1) {
AE_LA24X2_IP(d24, align_in, in);
ae_int32x2 d320 = d24;

d320 = AE_SLAI32(d320, 8);
AE_S32X2_XC(d320, out_ptr, 8);
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
i += 2;
}

@andrula-song andrula-song Jan 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too, use AE_S32X2F24_IP or AE_S32X2F24_XC

@fkwasowi fkwasowi Jan 16, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, such a change makes kpb not work properly

@andrula-song andrula-song Jan 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while use AE_S32X2F24*, please set parameter 3 as 8 instead of 4, for example, AE_S32X2F24_IP(d24, out_ptr, 8);

@fkwasowi fkwasowi Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change does not compile because the arguments are not appropriate for this function. When changing argument types, the function does not work.

if (i != (int)n_samples) {
AE_LA24X2_IP(d24, align_in, in);
ae_int32x2 d320 = d24;
int higher = AE_MOVAD32_H(d320);
*(ae_int32 *)(out_ptr) = higher << 8;

@andrula-song andrula-song Jan 12, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here too, use AE_S32F24_H_IP(d24, out_ptr, 4)

@fkwasowi fkwasowi Jan 16, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no such function AE_S32F24_H_IP - the code does not compile

@andrula-song andrula-song Jan 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here use AE_S32F24_H_IP(d24, out_ptr, 4) is because you load 2 24bit (the L and H part of d24 is different)

@fkwasowi fkwasowi Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not compile with this change, as no such function exists.

@fkwasowi fkwasowi Jan 24, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

,,Implicit declaration of function AE_S32F24_H_IP "

}
}
#else
static void kpb_convert_24b_to_32b(const void *source, int ioffset,
struct audio_stream __sparse_cache *sink,
int ooffset, unsigned int samples)
{
int ssize = audio_stream_sample_bytes(sink);
uint8_t *src = (uint8_t *)source + ioffset * 3;
int32_t *dst = audio_stream_wrap(sink, (uint8_t *)sink->w_ptr + ooffset * ssize);
int processed;
int nmax, i, n;

for (processed = 0; processed < samples; processed += n) {
dst = audio_stream_wrap(sink, dst);
n = samples - processed;
nmax = KPB_BYTES_TO_S32_SAMPLES(audio_stream_bytes_without_wrap(sink, dst));
n = MIN(n, nmax);
for (i = 0; i < n; i += 1) {
*dst = (src[2] << 16) | (src[1] << 8) | src[0];
dst++;
src += 3;
}
}
}
#endif
/**
* \brief Drain data samples safe, according to configuration.
*
Expand All @@ -1617,6 +1688,9 @@ static void kpb_drain_samples(void *source, struct audio_stream __sparse_cache *
#endif /* CONFIG_FORMAT_S16LE */
#if CONFIG_FORMAT_S24LE || CONFIG_FORMAT_S32LE
case 24:
samples = size / ((sample_width >> 3) * sink->channels);
kpb_convert_24b_to_32b(source, 0, sink, 0, samples);
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
break;
case 32:
samples = KPB_BYTES_TO_S32_SAMPLES(size);
audio_stream_copy_from_linear(source, 0, sink, 0, samples);
Expand All @@ -1628,6 +1702,68 @@ static void kpb_drain_samples(void *source, struct audio_stream __sparse_cache *
}
}

#ifdef KPB_HIFI3
static void kpb_convert_32b_to_24b(const struct audio_stream __sparse_cache *source, int ioffset,
void *linear_sink, int ooffset, unsigned int n_samples)
{
int ssize = audio_stream_sample_bytes(source);
uint8_t *in = audio_stream_wrap(source, (uint8_t *)source->r_ptr + ioffset * ssize);
uint8_t *out = (uint8_t *)linear_sink + ooffset * ssize;

const ae_f24x2 *sin = (const ae_f24x2 *)in;
ae_f24x2 *sout = (ae_f24x2 *)out;

ae_f24x2 vs = AE_ZERO24();
ae_valign align_out = AE_ZALIGN64();

if (!IS_ALIGNED((uintptr_t)sin, 8)) {
AE_L32F24_XC(vs, (const ae_f24 *)sin, 4);
AE_SA24_IP(vs, align_out, sout);
n_samples--;
}

unsigned int size = n_samples >> 1;
size_t i;

for (i = 0; i < size; i++) {
AE_L32X2F24_XC(vs, sin, 8);
AE_SA24X2_IP(vs, align_out, sout);
}
AE_SA64POS_FP(align_out, sout);

if (n_samples & 1) {
AE_L32X2F24_XC(vs, sin, 4);
ae_f24 tmp = AE_MOVAD32_H(AE_MOVINT24X2_FROMF24X2(vs));

AE_SA24_IP(tmp, align_out, sout);
AE_SA64POS_FP(align_out, sout);
}
}
#else
static void kpb_convert_32b_to_24b(const struct audio_stream __sparse_cache *source, int ioffset,
void *sink, int ooffset, unsigned int samples)
{
int ssize = audio_stream_sample_bytes(source);
int32_t *src = audio_stream_wrap(source, (uint8_t *)source->r_ptr + ioffset * ssize);
uint8_t *dst = (uint8_t *)sink + ooffset * 3;
int processed;
int nmax, i, n;

for (processed = 0; processed < samples; processed += n) {
src = audio_stream_wrap(source, src);
n = samples - processed;
nmax = KPB_BYTES_TO_S32_SAMPLES(audio_stream_bytes_without_wrap(source, src));
n = MIN(n, nmax);
for (i = 0; i < n; i += 1) {
dst[0] = *src & 0xFF;
dst[1] = (*src >> 8) & 0xFF;
dst[2] = (*src >> 16) & 0xFF;
dst += 3;
src++;
}
}
}
#endif
/**
* \brief Buffers data samples safe, according to configuration.
* \param[in,out] source Pointer to source buffer.
Expand All @@ -1654,6 +1790,11 @@ static void kpb_buffer_samples(const struct audio_stream __sparse_cache *source,
#endif
#if CONFIG_FORMAT_S24LE || CONFIG_FORMAT_S32LE
case 24:
samples_count = size / ((sample_width >> 3) * source->channels);
samples_offset = offset / ((sample_width >> 3) * source->channels);
kpb_convert_32b_to_24b(source, samples_offset,
sink, 0, samples_count);
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
break;
case 32:
samples_count = KPB_BYTES_TO_S32_SAMPLES(size);
samples_offset = KPB_BYTES_TO_S32_SAMPLES(offset);
Expand Down Expand Up @@ -1717,6 +1858,66 @@ static inline bool kpb_is_sample_width_supported(uint32_t sampling_width)
return ret;
}

#ifdef KPB_HIFI3
static void kpb_copy_24b_in_32b(const struct audio_stream __sparse_cache *source, uint32_t ioffset,
struct audio_stream __sparse_cache *sink, uint32_t ooffset,
uint32_t n_samples)
{
int ssize = audio_stream_sample_bytes(source); /* src fmt == sink fmt */
uint8_t *in = audio_stream_wrap(source, (uint8_t *)source->r_ptr + ioffset * ssize);
uint8_t *out = audio_stream_wrap(sink, (uint8_t *)sink->w_ptr + ooffset * ssize);

const ae_int32x2 *sin = (const ae_int32x2 *)in;
ae_int32x2 *sout = (ae_int32x2 *)out;
ae_int32x2 vs = AE_ZERO32();

if (!IS_ALIGNED((uintptr_t)sin, 8)) {
AE_L32_IP(vs, (const ae_int32 *)sin, 4);
AE_S32_L_IP(vs, (ae_int32 *)sout, 4);
n_samples--;
}
ae_valign align_out = AE_ZALIGN64();
size_t size = n_samples >> 1;
size_t i;

for (i = 0; i < size; i++) {
AE_L32X2_IP(vs, sin, 8);
AE_SA32X2_IP(vs, align_out, sout);
}
AE_SA64POS_FP(align_out, sout);
if (n_samples & 1) {
vs = AE_L32_I((const ae_int32 *)sin, 0);
AE_S32_L_I(vs, (ae_int32 *)sout, 0);
}
}
#else
static void kpb_copy_24b_in_32b(const struct audio_stream __sparse_cache *source,
uint32_t ioffset, struct audio_stream __sparse_cache *sink,
uint32_t ooffset, uint32_t samples)
{
int32_t *src = source->r_ptr;
int32_t *dst = sink->w_ptr;
int processed;
int nmax, i, n;

src += ioffset;
dst += ooffset;
for (processed = 0; processed < samples; processed += n) {
src = audio_stream_wrap(source, src);
dst = audio_stream_wrap(sink, dst);
n = samples - processed;
nmax = KPB_BYTES_TO_S32_SAMPLES(audio_stream_bytes_without_wrap(source, src));
n = MIN(n, nmax);
nmax = KPB_BYTES_TO_S32_SAMPLES(audio_stream_bytes_without_wrap(sink, dst));
n = MIN(n, nmax);
for (i = 0; i < n; i++) {
*dst = *src << 8;
src++;
dst++;
}
}
}
#endif
/**
* \brief Copy data samples safe, according to configuration.
*
Expand All @@ -1728,10 +1929,11 @@ static inline bool kpb_is_sample_width_supported(uint32_t sampling_width)
*/
static void kpb_copy_samples(struct comp_buffer __sparse_cache *sink,
struct comp_buffer __sparse_cache *source, size_t size,
size_t sample_width)
size_t sample_width, uint32_t channels)
{
struct audio_stream __sparse_cache *istream = &source->stream;
struct audio_stream __sparse_cache *ostream = &sink->stream;
unsigned int samples;

buffer_stream_invalidate(source, size);
switch (sample_width) {
Expand All @@ -1742,6 +1944,9 @@ static void kpb_copy_samples(struct comp_buffer __sparse_cache *sink,
#endif
#if CONFIG_FORMAT_S24LE || CONFIG_FORMAT_S32LE
case 24:
samples = size / ((sample_width >> 3) * channels);
kpb_copy_24b_in_32b(istream, 0, ostream, 0, samples);
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
break;
case 32:
audio_stream_copy(istream, 0, ostream, 0, KPB_BYTES_TO_S32_SAMPLES(size));
break;
Expand Down Expand Up @@ -1808,13 +2013,13 @@ static inline bool validate_host_params(struct comp_dev *dev,
comp_err(dev, "kpb: host_period_size (%d) cannot be 0 and host_buffer_size (%d) cannot be 0",
host_period_size, host_buffer_size);
return false;
} else if (HOST_BUFFER_MIN_SIZE(hb_size_req) >
} else if (HOST_BUFFER_MIN_SIZE(hb_size_req, kpb->config.channels) >
host_buffer_size) {
/* Host buffer size is too small - history data
* may get overwritten.
*/
comp_err(dev, "kpb: host_buffer_size (%d) must be at least %d",
host_buffer_size, HOST_BUFFER_MIN_SIZE(hb_size_req));
host_buffer_size, HOST_BUFFER_MIN_SIZE(hb_size_req, kpb->config.channels));
return false;
} else if (kpb->sync_draining_mode) {
/* Sync draining allowed. Check if we can perform draining
Expand Down
23 changes: 15 additions & 8 deletions src/include/sof/audio/kpb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,14 @@
#include <user/trace.h>
#include <stdint.h>

#if defined(__XCC__)

#include <xtensa/config/core-isa.h>
#if XCHAL_HAVE_HIFI3 || XCHAL_HAVE_HIFI4
#define KPB_HIFI3
#endif

#endif
struct comp_buffer;

/* KPB internal defines */
Expand All @@ -26,26 +34,25 @@ struct comp_buffer;
#endif

#define KPB_MAX_DRAINING_REQ (KPB_MAX_BUFF_TIME - HOST_WAKEUP_TIME)
#define KPB_MAX_SUPPORTED_CHANNELS 2 /**< number of supported channels */
#define KPB_MAX_SUPPORTED_CHANNELS 6 /**< number of supported channels */
/**< number of samples taken each milisecond */
#define KPB_SAMPLES_PER_MS (KPB_SAMPLNG_FREQUENCY / 1000)
#define KPB_SAMPLNG_FREQUENCY 16000 /**< supported sampling frequency in Hz */
#define KPB_NUM_OF_CHANNELS 2
#define KPB_SAMPLE_CONTAINER_SIZE(sw) ((sw == 16) ? 16 : 32)
#define KPB_MAX_BUFFER_SIZE(sw) ((KPB_SAMPLNG_FREQUENCY / 1000) * \
#define KPB_MAX_BUFFER_SIZE(sw, channels_number) ((KPB_SAMPLNG_FREQUENCY / 1000) * \
(KPB_SAMPLE_CONTAINER_SIZE(sw) / 8) * KPB_MAX_BUFF_TIME * \
KPB_NUM_OF_CHANNELS)
(channels_number))
#define KPB_MAX_NO_OF_CLIENTS 2
#define KPB_NO_OF_HISTORY_BUFFERS 2 /**< no of internal buffers */
#define KPB_ALLOCATION_STEP 0x100
#define KPB_NO_OF_MEM_POOLS 3
#define KPB_BYTES_TO_FRAMES(bytes, sample_width) \
(bytes / ((KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8) * \
KPB_NUM_OF_CHANNELS))
#define KPB_BYTES_TO_FRAMES(bytes, sample_width, channels_number) \
((bytes) / ((KPB_SAMPLE_CONTAINER_SIZE(sample_width) / 8) * \
(channels_number)))
/**< Defines how much faster draining is in comparison to pipeline copy. */
#define KPB_DRAIN_NUM_OF_PPL_PERIODS_AT_ONCE 2
/**< Host buffer shall be at least two times bigger than history buffer. */
#define HOST_BUFFER_MIN_SIZE(hb) (hb * 2)
#define HOST_BUFFER_MIN_SIZE(hb, channels_number) ((hb) * (channels_number))

/**< Convert with right shift a bytes count to samples count */
#define KPB_BYTES_TO_S16_SAMPLES(s) ((s) >> 1)
Expand Down
2 changes: 1 addition & 1 deletion src/platform/meteorlake/include/platform/lib/memory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
/**
* size of HPSRAM system heap
*/
#define HEAPMEM_SIZE 0x60000
#define HEAPMEM_SIZE 0xD0000
Comment thread
fkwasowi marked this conversation as resolved.
Outdated
Comment thread
fkwasowi marked this conversation as resolved.
Outdated

#endif /* __PLATFORM_LIB_MEMORY_H__ */

Expand Down