Skip to content

audio: use size_t for circular-buffer bytes counts #11208

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

Open
softwarecki wants to merge 3 commits into
base: main
Choose a base branch
from
+58 −65
Open

audio: use size_t for circular-buffer bytes counts #11208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0972efe
audio: audio_stream: return size_t from cir_buf_bytes_without_wrap he…
softwarecki Sep 16, 2026
4680f9e
audio: volume: Use size_t in functions searching for zero-crossing frame
softwarecki Sep 16, 2026
279949f
audio: audio_stream: Use uintptr_t for pointer comparisons in asserts
softwarecki Sep 16, 2026
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
44 changes: 20 additions & 24 deletions src/audio/volume/volume.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,15 @@ LOG_MODULE_REGISTER(volume, CONFIG_SOF_LOG_LEVEL);
* \param[in] frames Number of frames.
* \param[in,out] prev_sum Previous sum of channel samples.
*/
static uint32_t vol_zc_get_s16(struct cir_buf_source *source, const int channels,
uint32_t frames, int64_t *prev_sum)
static size_t vol_zc_get_s16(struct cir_buf_source *source, const unsigned int channels,
size_t frames, int64_t *prev_sum)
{
uint32_t curr_frames = frames;
size_t curr_frames = frames;
int32_t sum;
const int16_t *x = source->ptr;
int bytes;
int nmax;
int i, j, n;
int remaining_samples = frames * channels;
size_t bytes, nmax, i, n;
unsigned int j;
size_t remaining_samples = frames * channels;

/* Go to last channel */
x = source_cir_buf_wrap(x + remaining_samples - 1, source->buf_start, source->buf_end);
Expand Down Expand Up @@ -106,16 +105,15 @@ static uint32_t vol_zc_get_s16(struct cir_buf_source *source, const int channels
* \param[in] frames Number of frames.
* \param[in,out] prev_sum Previous sum of channel samples.
*/
static uint32_t vol_zc_get_s24(struct cir_buf_source *source, const int channels,
uint32_t frames, int64_t *prev_sum)
static size_t vol_zc_get_s24(struct cir_buf_source *source, const unsigned int channels,
size_t frames, int64_t *prev_sum)
{
int64_t sum;
uint32_t curr_frames = frames;
size_t curr_frames = frames;
const int32_t *x = source->ptr;
int bytes;
int nmax;
int i, j, n;
int remaining_samples = frames * channels;
size_t bytes, nmax, i, n;
unsigned int j;
size_t remaining_samples = frames * channels;

@lyakh lyakh Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hm, would all modules now be expected to change remaining_samples in all their processing methods to size_t? it would be a rather big change

@softwarecki softwarecki Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Ultimately, I'd like all byte, sample, and frame counts to use size_t for consistency and some unsigned type for channels count. That said, I'm not planning a project-wide conversion right now.

I'm only introducing these changes in modules that are already being touched, such as during sink/source API conversions, or when they naturally come up as part of other work (for example, when updating helper definitions). Over time, this should allow us to move towards a consistent use of size_t without requiring a large-scale refactoring effort.


/* Go to last channel */
x = source_cir_buf_wrap(x + remaining_samples - 1, source->buf_start, source->buf_end);
Expand Down Expand Up @@ -155,16 +153,15 @@ static uint32_t vol_zc_get_s24(struct cir_buf_source *source, const int channels
* \param[in] frames Number of frames.
* \param[in,out] prev_sum Previous sum of channel samples.
*/
static uint32_t vol_zc_get_s32(struct cir_buf_source *source, const int channels,
uint32_t frames, int64_t *prev_sum)
static size_t vol_zc_get_s32(struct cir_buf_source *source, const unsigned int channels,
size_t frames, int64_t *prev_sum)
{
int64_t sum;
uint32_t curr_frames = frames;
size_t curr_frames = frames;
const int32_t *x = source->ptr;
int bytes;
int nmax;
int i, j, n;
int remaining_samples = frames * channels;
size_t bytes, nmax, i, n;
unsigned int j;
size_t remaining_samples = frames * channels;

@lyakh lyakh Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

in general I like this change, it is very close to what types I'd also use for channel counters and byte numbers (perhaps not for frame / sample counts though). But we have many modules that follow a roughly the same pattern - int for channel counts etc. Should all be changed now? If yes, maybe it should be a dedicated effort - establish types for all these common variables and change them throughout the code base

@softwarecki softwarecki Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

For frame and sample counts, I also prefer size_t. For 8-bit formats, the byte count is identical to the sample count, so using the same type throughout feels natural.

I agree it would be beneficial to establish a set of preferred types and gradually converge towards them. However, I don't think this is important enough to justify a project-wide conversion right now. Also, I'm not a fan of introducing dedicated typedefs such as channels_no_t - I'd rather use standard types directly.


/* Go to last channel */
x = source_cir_buf_wrap(x + remaining_samples - 1, source->buf_start, source->buf_end);
Expand Down Expand Up @@ -564,12 +561,11 @@ static int volume_process(struct processing_module *mod,
struct sof_sink *sink = sinks[0];
struct cir_buf_source source_buf;
struct cir_buf_sink sink_buf;
const int nch = cd->channels;
const unsigned int nch = cd->channels;
size_t source_frame_bytes = source_get_frame_bytes(source);
size_t sink_frame_bytes = sink_get_frame_bytes(sink);
size_t source_bytes, sink_bytes, bytes;
uint32_t avail_frames;
uint32_t frames;
size_t avail_frames, frames;
int64_t prev_sum = 0;
int ret;

Expand Down
4 changes: 2 additions & 2 deletions src/audio/volume/volume.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ typedef void (*vol_scale_func)(struct processing_module *mod, struct cir_buf_sou
/**
* \brief volume interface for function getting nearest zero crossing frame
*/
typedef uint32_t (*vol_zc_func)(struct cir_buf_source *source, const int channels,
uint32_t frames, int64_t *prev_sum);
typedef size_t (*vol_zc_func)(struct cir_buf_source *source, const unsigned int channels,
size_t frames, int64_t *prev_sum);

/**
* \brief Function for volume ramp shape function
Expand Down
39 changes: 25 additions & 14 deletions src/include/module/audio/audio_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ static inline int cir_buf_samples_without_wrap_s16(const void *ptr, const void *
{
int to_end = (const int16_t *)buf_end - (const int16_t *)ptr;

assert((intptr_t)buf_end >= (intptr_t)ptr);
assert((uintptr_t)buf_end >= (uintptr_t)ptr);
return to_end;
}

Expand All @@ -183,14 +183,26 @@ static inline int cir_buf_samples_without_wrap_s32(const void *ptr, const void *
{
int to_end = (const int32_t *)buf_end - (const int32_t *)ptr;

assert((intptr_t)buf_end >= (intptr_t)ptr);
assert((uintptr_t)buf_end >= (uintptr_t)ptr);
return to_end;
}

/**
* Verifies the pointer and performs rollover when reached the end of
* the circular buffer.
* @param ptr Pointer
* @brief Calculates number of bytes to buffer wrap when reading a circular
* buffer forward from current pointer towards the buffer end.
* @param ptr Read or write pointer of circular buffer.
* @param buf_end End address of circular buffer.
* @return Number of bytes between the pointer and the buffer end.
*/
static inline size_t cir_buf_bytes_without_wrap(const void *ptr, const void *buf_end)
{
assert((uintptr_t)buf_end >= (uintptr_t)ptr);
return (uintptr_t)buf_end - (uintptr_t)ptr;
}

/**
* Verifies the pointer and performs rollover when reaching the end of the circular buffer.
* @param ptr Read pointer that may have moved past the buffer end.
* @param buf_addr Start address of the circular buffer.
* @param buf_end End address of the circular buffer.
* @return Pointer, adjusted if necessary.
Expand All @@ -201,27 +213,26 @@ static inline void *cir_buf_wrap(const void *ptr, const void *buf_addr, const vo
ptr = (const char *)buf_addr +
((const char *)ptr - (const char *)buf_end);

assert((intptr_t)ptr <= (intptr_t)buf_end);
assert((uintptr_t)ptr <= (uintptr_t)buf_end);

return (void *)ptr;
}

/**
* Verifies a read pointer and performs rollover when reached the end of the circular buffer.
* @param ptr Pointer
* Verifies a read pointer and performs rollover when reaching the end of the circular buffer.
* @param ptr Read pointer that may have moved past the buffer end.
* @param buf_addr Start address of the circular buffer.
* @param buf_end End address of the circular buffer.
* @return Pointer, adjusted if necessary.
*/

static inline const void *source_cir_buf_wrap(const void *ptr, const void *buf_addr,
const void *buf_end)
{
if (ptr >= buf_end)
ptr = (const char *)buf_addr +
((const char *)ptr - (const char *)buf_end);

assert((intptr_t)ptr <= (intptr_t)buf_end);
assert((uintptr_t)ptr <= (uintptr_t)buf_end);

return ptr;
}
Expand All @@ -233,11 +244,11 @@ static inline const void *source_cir_buf_wrap(const void *ptr, const void *buf_a
* @param buf_start Start address of circular buffer.
* @return Number of bytes between the buffer start and the pointer.
*/
static inline int cir_buf_bytes_without_wrap_rewind(const void *ptr, const void *buf_start)
static inline size_t cir_buf_bytes_without_wrap_rewind(const void *ptr, const void *buf_start)
{
assert((intptr_t)ptr >= (intptr_t)buf_start);
assert((uintptr_t)ptr >= (uintptr_t)buf_start);

return (intptr_t)ptr - (intptr_t)buf_start;
return (uintptr_t)ptr - (uintptr_t)buf_start;
}

/**
Expand All @@ -254,7 +265,7 @@ static inline const void *source_cir_buf_rewind_wrap(const void *ptr, const void
if (ptr < buf_start)
ptr = (const char *)buf_end - ((const char *)buf_start - (const char *)ptr);

assert((intptr_t)ptr >= (intptr_t)buf_start);
assert((uintptr_t)ptr >= (uintptr_t)buf_start);

return ptr;
}
Expand Down
36 changes: 11 additions & 25 deletions src/include/sof/audio/audio_stream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ static inline void *audio_stream_wrap(const struct audio_stream *buffer, void *p
ptr = (char *)buffer->addr +
((char *)ptr - (char *)buffer->end_addr);

assert((intptr_t)ptr <= (intptr_t)buffer->end_addr);
assert((uintptr_t)ptr <= (uintptr_t)buffer->end_addr);

return ptr;
}
Expand All @@ -422,7 +422,7 @@ static inline void *audio_stream_rewind_wrap(const struct audio_stream *buffer,
if (ptr < buffer->addr)
ptr = (char *)buffer->end_addr - ((char *)buffer->addr - (char *)ptr);

assert((intptr_t)ptr >= (intptr_t)buffer->addr);
assert((uintptr_t)ptr >= (uintptr_t)buffer->addr);

return ptr;
}
Expand Down Expand Up @@ -804,8 +804,8 @@ static inline void audio_stream_writeback(struct audio_stream *buffer, uint32_t
static inline int
audio_stream_bytes_without_wrap(const struct audio_stream *source, const void *ptr)
{
assert((intptr_t)source->end_addr >= (intptr_t)ptr);
return (intptr_t)source->end_addr - (intptr_t)ptr;
assert((uintptr_t)source->end_addr >= (uintptr_t)ptr);
return (uintptr_t)source->end_addr - (uintptr_t)ptr;
}

/**
Expand All @@ -819,8 +819,8 @@ audio_stream_bytes_without_wrap(const struct audio_stream *source, const void *p
static inline int
audio_stream_rewind_bytes_without_wrap(const struct audio_stream *source, const void *ptr)
{
assert((intptr_t)ptr >= (intptr_t)source->addr);
int to_begin = (intptr_t)ptr - (intptr_t)source->addr;
assert((uintptr_t)ptr >= (uintptr_t)source->addr);
size_t to_begin = (uintptr_t)ptr - (uintptr_t)source->addr;
return to_begin;
}

Expand All @@ -837,8 +837,8 @@ static inline uint32_t
void *wptr = audio_stream_get_wptr(source);
int to_begin = audio_stream_rewind_bytes_without_wrap(source, wptr);

assert((intptr_t)wptr >= (intptr_t)source->addr);
assert((intptr_t)source->end_addr > (intptr_t)wptr);
assert((uintptr_t)wptr >= (uintptr_t)source->addr);
assert((uintptr_t)source->end_addr > (uintptr_t)wptr);

if (to_begin > bytes)
return (uint32_t *)((intptr_t)wptr - bytes);
Expand All @@ -858,7 +858,7 @@ audio_stream_samples_without_wrap_s16(const struct audio_stream *source, const v
{
int to_end = (int16_t *)source->end_addr - (int16_t *)ptr;

assert((intptr_t)source->end_addr >= (intptr_t)ptr);
assert((uintptr_t)source->end_addr >= (uintptr_t)ptr);
return to_end;
}

Expand All @@ -874,7 +874,7 @@ audio_stream_samples_without_wrap_s24(const struct audio_stream *source, const v
{
int to_end = (int32_t *)source->end_addr - (int32_t *)ptr;

assert((intptr_t)source->end_addr >= (intptr_t)ptr);
assert((uintptr_t)source->end_addr >= (uintptr_t)ptr);
return to_end;
}

Expand All @@ -890,24 +890,10 @@ audio_stream_samples_without_wrap_s32(const struct audio_stream *source, const v
{
int to_end = (int32_t *)source->end_addr - (int32_t *)ptr;

assert((intptr_t)source->end_addr >= (intptr_t)ptr);
assert((uintptr_t)source->end_addr >= (uintptr_t)ptr);
return to_end;
}

/**
* @brief Calculates numbers of bytes to buffer wrap when reading stream
* backwards from current sample pointed by ptr towards begin.
* @param ptr Read or write pointer og circular buffer.
* @param buf_end End address of circular buffer.
* @return Number of bytes to buffer wrap. For number of samples calculate
* need to add size of sample to returned bytes count.
*/
static inline int cir_buf_bytes_without_wrap(const void *ptr, const void *buf_end)
{
assert((intptr_t)buf_end >= (intptr_t)ptr);
return (intptr_t)buf_end - (intptr_t)ptr;
}

/**
* @brief Calculates numbers of frames to buffer wrap and return
* minimum of calculated value.
Expand Down
Loading