From 0c1f2ae39e896eda4446e31e587dda1e43ad122c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Thu, 24 Jul 2025 09:49:15 +0200 Subject: [PATCH] audio: buffer: don't overwrite global objects The 4 buffer operation sets: comp_buffer_source_ops, comp_buffer_sink_ops, ring_buffer_source_ops and ring_buffer_sink_ops are pointed to by every buffer. Yet when initialising each of those buffers attempts are made to overwrite those global objects with the same pointers. So in practive when the first buffer is registered, it overwrites those object members, and then all further buffers inherit those changes. This works because those values are always the same, still this isn't a good practice. Assign those members statically and make objects constant instead. Signed-off-by: Guennadi Liakhovetski --- src/audio/buffers/audio_buffer.c | 25 +------------------------ src/audio/buffers/comp_buffer.c | 10 ++++++++-- src/audio/buffers/ring_buffer.c | 10 ++++++++-- src/include/sof/audio/audio_buffer.h | 16 +++++++++++++++- 4 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/audio/buffers/audio_buffer.c b/src/audio/buffers/audio_buffer.c index f63a9a7c93b6..8ab0a711e7b1 100644 --- a/src/audio/buffers/audio_buffer.c +++ b/src/audio/buffers/audio_buffer.c @@ -122,7 +122,6 @@ int audio_buffer_source_set_ipc_params_default(struct sof_audio_buffer *buffer, return 0; } -static int audio_buffer_sink_set_ipc_params(struct sof_sink *sink, struct sof_ipc_stream_params *params, bool force_update) { @@ -133,7 +132,6 @@ int audio_buffer_sink_set_ipc_params(struct sof_sink *sink, struct sof_ipc_strea return audio_buffer_source_set_ipc_params_default(buffer, params, force_update); } -static int audio_buffer_sink_on_audio_format_set(struct sof_sink *sink) { struct sof_audio_buffer *buffer = sof_audio_buffer_from_sink(sink); @@ -143,7 +141,6 @@ int audio_buffer_sink_on_audio_format_set(struct sof_sink *sink) return 0; } -static int audio_buffer_sink_set_alignment_constants(struct sof_sink *sink, const uint32_t byte_align, const uint32_t frame_align_req) @@ -155,7 +152,6 @@ int audio_buffer_sink_set_alignment_constants(struct sof_sink *sink, return 0; } -static int audio_buffer_source_set_ipc_params(struct sof_source *source, struct sof_ipc_stream_params *params, bool force_update) { @@ -166,7 +162,6 @@ int audio_buffer_source_set_ipc_params(struct sof_source *source, return audio_buffer_source_set_ipc_params_default(buffer, params, force_update); } -static int audio_buffer_source_on_audio_format_set(struct sof_source *source) { struct sof_audio_buffer *buffer = sof_audio_buffer_from_source(source); @@ -176,7 +171,6 @@ int audio_buffer_source_on_audio_format_set(struct sof_source *source) return 0; } -static int audio_buffer_source_set_alignment_constants(struct sof_source *source, const uint32_t byte_align, const uint32_t frame_align_req) @@ -189,7 +183,7 @@ int audio_buffer_source_set_alignment_constants(struct sof_source *source, } void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bool is_shared, - struct source_ops *source_ops, struct sink_ops *sink_ops, + const struct source_ops *source_ops, const struct sink_ops *sink_ops, const struct audio_buffer_ops *audio_buffer_ops, struct sof_audio_stream_params *audio_stream_params) { @@ -200,23 +194,6 @@ void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bo buffer->audio_stream_params = audio_stream_params; buffer->is_shared = is_shared; - /* set default implementations of sink/source methods, if there's no - * specific implementation provided - */ - if (!sink_ops->audio_set_ipc_params) - sink_ops->audio_set_ipc_params = audio_buffer_sink_set_ipc_params; - if (!sink_ops->on_audio_format_set && buffer->ops->on_audio_format_set) - sink_ops->on_audio_format_set = audio_buffer_sink_on_audio_format_set; - if (!sink_ops->set_alignment_constants && buffer->ops->set_alignment_constants) - sink_ops->set_alignment_constants = audio_buffer_sink_set_alignment_constants; - - if (!source_ops->audio_set_ipc_params) - source_ops->audio_set_ipc_params = audio_buffer_source_set_ipc_params; - if (!source_ops->on_audio_format_set && buffer->ops->on_audio_format_set) - source_ops->on_audio_format_set = audio_buffer_source_on_audio_format_set; - if (!source_ops->set_alignment_constants && buffer->ops->set_alignment_constants) - source_ops->set_alignment_constants = audio_buffer_source_set_alignment_constants; - source_init(audio_buffer_get_source(buffer), source_ops, audio_buffer_get_stream_params(buffer)); sink_init(audio_buffer_get_sink(buffer), sink_ops, diff --git a/src/audio/buffers/comp_buffer.c b/src/audio/buffers/comp_buffer.c index 454f5f0a49e3..bf6fc8a65659 100644 --- a/src/audio/buffers/comp_buffer.c +++ b/src/audio/buffers/comp_buffer.c @@ -161,16 +161,22 @@ static void comp_buffer_free(struct sof_audio_buffer *audio_buffer) rfree(buffer); } -static struct source_ops comp_buffer_source_ops = { +static const struct source_ops comp_buffer_source_ops = { .get_data_available = comp_buffer_get_data_available, .get_data = comp_buffer_get_data, .release_data = comp_buffer_release_data, + .audio_set_ipc_params = audio_buffer_source_set_ipc_params, + .on_audio_format_set = audio_buffer_source_on_audio_format_set, + .set_alignment_constants = audio_buffer_source_set_alignment_constants, }; -static struct sink_ops comp_buffer_sink_ops = { +static const struct sink_ops comp_buffer_sink_ops = { .get_free_size = comp_buffer_get_free_size, .get_buffer = comp_buffer_get_buffer, .commit_buffer = comp_buffer_commit_buffer, + .audio_set_ipc_params = audio_buffer_sink_set_ipc_params, + .on_audio_format_set = audio_buffer_sink_on_audio_format_set, + .set_alignment_constants = audio_buffer_sink_set_alignment_constants, }; static const struct audio_buffer_ops audio_buffer_ops = { diff --git a/src/audio/buffers/ring_buffer.c b/src/audio/buffers/ring_buffer.c index c245dec42d6f..b6cbc23060e2 100644 --- a/src/audio/buffers/ring_buffer.c +++ b/src/audio/buffers/ring_buffer.c @@ -255,17 +255,23 @@ int ring_buffer_module_unbind(struct sof_sink *sink) return 0; } -static struct source_ops ring_buffer_source_ops = { +static const struct source_ops ring_buffer_source_ops = { .get_data_available = ring_buffer_get_data_available, .get_data = ring_buffer_get_data, .release_data = ring_buffer_release_data, + .audio_set_ipc_params = audio_buffer_source_set_ipc_params, + .on_audio_format_set = audio_buffer_source_on_audio_format_set, + .set_alignment_constants = audio_buffer_source_set_alignment_constants, }; -static struct sink_ops ring_buffer_sink_ops = { +static const struct sink_ops ring_buffer_sink_ops = { .get_free_size = ring_buffer_get_free_size, .get_buffer = ring_buffer_get_buffer, .commit_buffer = ring_buffer_commit_buffer, .on_unbind = ring_buffer_module_unbind, + .audio_set_ipc_params = audio_buffer_sink_set_ipc_params, + .on_audio_format_set = audio_buffer_sink_on_audio_format_set, + .set_alignment_constants = audio_buffer_sink_set_alignment_constants, }; static const struct audio_buffer_ops audio_buffer_ops = { diff --git a/src/include/sof/audio/audio_buffer.h b/src/include/sof/audio/audio_buffer.h index e7fc6e7e1c5d..a4b52248ce59 100644 --- a/src/include/sof/audio/audio_buffer.h +++ b/src/include/sof/audio/audio_buffer.h @@ -287,7 +287,7 @@ static inline struct sof_audio_buffer *sof_audio_buffer_from_source(struct sof_s * audio_buffer_ops operations */ void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bool is_shared, - struct source_ops *source_ops, struct sink_ops *sink_ops, + const struct source_ops *source_ops, const struct sink_ops *sink_ops, const struct audio_buffer_ops *audio_buffer_ops, struct sof_audio_stream_params *audio_stream_params); @@ -307,4 +307,18 @@ void audio_buffer_reset(struct sof_audio_buffer *buffer) buffer->ops->reset(buffer); } +/* Audio-buffer wrappers for the source-sink API */ +int audio_buffer_source_set_ipc_params(struct sof_source *source, + struct sof_ipc_stream_params *params, bool force_update); +int audio_buffer_source_on_audio_format_set(struct sof_source *source); +int audio_buffer_source_set_alignment_constants(struct sof_source *source, + const uint32_t byte_align, + const uint32_t frame_align_req); +int audio_buffer_sink_set_ipc_params(struct sof_sink *sink, struct sof_ipc_stream_params *params, + bool force_update); +int audio_buffer_sink_on_audio_format_set(struct sof_sink *sink); +int audio_buffer_sink_set_alignment_constants(struct sof_sink *sink, + const uint32_t byte_align, + const uint32_t frame_align_req); + #endif /* __SOF_AUDIO_BUFFER__ */