Skip to content
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.

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

Pipeline2.0 STEP2 - move hybrid buffering logic from comp_buffer to audio_buffer and partially place comp_buffer on top of audio_buffer #9299

Merged
merged 6 commits into from
Sep 2, 2024
1 change: 1 addition & 0 deletions src/audio/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ add_local_sources(sof
component.c
data_blob.c
buffer.c
buffers/audio_buffer.c
source_api_helper.c
sink_api_helper.c
sink_source_utils.c
Expand Down
97 changes: 97 additions & 0 deletions src/audio/buffers/audio_buffer.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// SPDX-License-Identifier: BSD-3-Clause
//
// Copyright(c) 2024 Intel Corporation. All rights reserved.
//
// Author: Marcin Szkudlinski <[email protected]>

#include <stdint.h>
#include <stddef.h>
#include <errno.h>
#include <rtos/alloc.h>
#include <sof/audio/audio_buffer.h>
#include <sof/audio/sink_api.h>
#include <sof/audio/source_api.h>
#include <sof/audio/sink_source_utils.h>

#if CONFIG_PIPELINE_2_0

int audio_buffer_attach_secondary_buffer(struct sof_audio_buffer *buffer, bool at_input,
struct sof_audio_buffer *secondary_buffer)
{
if (buffer->secondary_buffer_sink || buffer->secondary_buffer_source)
return -EINVAL;

/* secondary buffer must share audio params with the primary buffer */
secondary_buffer->audio_stream_params = buffer->audio_stream_params;
/* for performance reasons pointers to params are also kept in sink/src structures */
secondary_buffer->_sink_api.audio_stream_params = buffer->audio_stream_params;
secondary_buffer->_source_api.audio_stream_params = buffer->audio_stream_params;
Copy link
Collaborator

@lyakh lyakh Sep 3, 2024

Choose a reason for hiding this comment

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

this seems to make 3 copies of stream parameters, so now we have 4 of them on the total? This seems really redundant. Is this a temporary situation until buffers are migrated to the new API? Maybe make one of them where it finally should be and use pointers at all other locations? TBH I'm a bit confused about the roadmap of the migration, how far we're into it and what is still left to do. @marcinszkudlinski maybe you could make a "feature" to explain the steps made so far, the current configuration (what types of buffers we currently have, what copying is done, etc.) and the roadmap?


if (at_input)
buffer->secondary_buffer_sink = secondary_buffer;
else
buffer->secondary_buffer_source = secondary_buffer;

return 0;
}

int audio_buffer_sync_secondary_buffer(struct sof_audio_buffer *buffer, size_t limit)
{
int err;

struct sof_source *data_src;
struct sof_sink *data_dst;

if (buffer->secondary_buffer_sink) {
/*
* audio_buffer sink API is shadowed, that means there's a secondary_buffer
* at data input
* get data from secondary_buffer (use source API)
* copy to primary buffer (use sink API)
* note! can't use audio_buffer_get_sink because it will provide a shadowed
* sink handler (to a secondary buffer).
*/
data_src = audio_buffer_get_source(buffer->secondary_buffer_sink);
data_dst = &buffer->_sink_api; /* primary buffer's sink API */
} else if (buffer->secondary_buffer_source) {
/*
* comp_buffer source API is shadowed, that means there's a secondary_buffer
* at data output
* get data from comp_buffer (use source API)
* copy to secondary_buffer (use sink API)
*/
data_src = &buffer->_source_api;
data_dst = audio_buffer_get_sink(buffer->secondary_buffer_source);

} else {
return -EINVAL;
}

/*
* keep data_available and free_size in local variables to avoid check_time/use_time
* race in MIN macro
*/
size_t data_available = source_get_data_available(data_src);
size_t free_size = sink_get_free_size(data_dst);
size_t to_copy = MIN(MIN(data_available, free_size), limit);

err = source_to_sink_copy(data_src, data_dst, true, to_copy);
return err;
}

#endif /* CONFIG_PIPELINE_2_0 */

void audio_buffer_free(struct sof_audio_buffer *buffer)
{
if (!buffer)
return;

CORE_CHECK_STRUCT(buffer);
#if CONFIG_PIPELINE_2_0
audio_buffer_free(buffer->secondary_buffer_sink);
audio_buffer_free(buffer->secondary_buffer_source);
#endif /* CONFIG_PIPELINE_2_0 */
if (buffer->free)
buffer->free(buffer);
rfree(buffer);
}
1 change: 1 addition & 0 deletions src/audio/source_api_helper.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright(c) 2023 Intel Corporation. All rights reserved.
*/

#include <sof/audio/sink_api.h>
#include <sof/audio/source_api.h>
#include <sof/audio/audio_stream.h>

Expand Down
149 changes: 136 additions & 13 deletions src/include/sof/audio/audio_buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
#include <sof/common.h>
#include <ipc/topology.h>
#include <sof/coherent.h>
#include <sof/audio/sink_api.h>
#include <sof/audio/source_api.h>

#define BUFFER_TYPE_LEGACY_BUFFER 1
#define BUFFER_TYPE_LEGACY_RING_HYBRID 2
#define BUFFER_TYPE_RING_BUFFER 3
#define BUFFER_TYPE_RING_BUFFER 2

/* base class for all buffers, all buffers must inherit from it */
struct sof_audio_buffer {
Expand All @@ -21,8 +22,34 @@ struct sof_audio_buffer {
/* type of the buffer BUFFER_TYPE_* */
uint32_t buffer_type;

/* runtime stream params */
struct sof_audio_stream_params audio_stream_params;
#if CONFIG_PIPELINE_2_0
/**
* sink API of an additional buffer
* of any type at data input
*
* to be removed when hybrid buffers are no longer needed
*/
struct sof_audio_buffer *secondary_buffer_sink;

/**
* source API of an additional buffer
* at data output
*/
struct sof_audio_buffer *secondary_buffer_source;

#endif /* CONFIG_PIPELINE_2_0 */

/* effective runtime stream params
* before pipelin2.0 is ready, stream params may be kept in audio_stream structure
* also for hybrid buffering audio params need to be shared between primary and secondary
* buffers
*
* So currently only a pointer to effective stream params is here.
* Note that the same pointer MUST be set in source and sink api (kept there for
* performance reasons)
*/
struct sof_audio_stream_params *audio_stream_params;


/* private: */
struct sof_source _source_api; /**< src api handler */
Expand All @@ -36,6 +63,77 @@ struct sof_audio_buffer {
void (*free)(struct sof_audio_buffer *buffer);
};

#if CONFIG_PIPELINE_2_0
/*
* attach a secondary buffer (any type) before buffer (when at_input == true) or behind a buffer
*
* before buffer (at_input == true):
* 2.0 mod ==> (sink_API) secondary buffer ==>
* ==> comp_buffer (audio_stream or source API) ==> 1.0 mod
*
* after buffer (at_input == false):
* 1.0 mod ==> (audio_stream or sink API) ==> comp_buffer ==>
* ==> secondary buffer(source API) == 2.0 mod
*
* If a secondary buffer is attached, it replaces source or sink interface of audio_stream
* allowing the module connected to it using all properties of secondary buffer (like
* lockless cross-core connection in case of ring_buffer etc.) keeping legacy interface
* to other modules
*
* buffer_sync_secondary_buffer must be called every 1 ms to move data to/from
* secondary buffer to comp_buffer
*
* @param buffer pointer to a buffer
* @param at_input true indicates that a secondary buffer is located at data input, replacing
* sink API of audio_stream
* false indicates that a secondary buffer is located at data output, replacing
* source API of audio_stream
* @param secondary_buffer pointer to a buffer to be attached
*
* to be removed when hybrid buffers are no longer needed
*/
int audio_buffer_attach_secondary_buffer(struct sof_audio_buffer *buffer, bool at_input,
struct sof_audio_buffer *secondary_buffer);

/*
* move data from/to secondary buffer, must be called periodically as described above
*
* @param buffer pointer to a buffer
* @param limit data copy limit. Indicates maximum amount of data that will be moved from/to
* secondary buffer in an operation
*
* to be removed when hybrid buffers are no longer needed
*/
int audio_buffer_sync_secondary_buffer(struct sof_audio_buffer *buffer, size_t limit);

/**
* @brief return a handler to sink API of audio_buffer.
* the handler may be used by helper functions defined in sink_api.h
*/
static inline
struct sof_sink *audio_buffer_get_sink(struct sof_audio_buffer *buffer)
{
CORE_CHECK_STRUCT(buffer);
return buffer->secondary_buffer_sink ?
audio_buffer_get_sink(buffer->secondary_buffer_sink) :
&buffer->_sink_api;
}

/**
* @brief return a handler to source API of audio_buffer
* the handler may be used by helper functions defined in source_api.h
*/
static inline
struct sof_source *audio_buffer_get_source(struct sof_audio_buffer *buffer)
{
CORE_CHECK_STRUCT(buffer);
return buffer->secondary_buffer_source ?
audio_buffer_get_source(buffer->secondary_buffer_source) :
&buffer->_source_api;
}

#else /* CONFIG_PIPELINE_2_0 */

/**
* @brief return a handler to sink API of audio_buffer.
* the handler may be used by helper functions defined in sink_api.h
Expand All @@ -58,6 +156,17 @@ struct sof_source *audio_buffer_get_source(struct sof_audio_buffer *buffer)
return &buffer->_source_api;
}

#endif /* CONFIG_PIPELINE_2_0 */

/**
* @brief return a handler to stream params structure
*/
static inline
struct sof_audio_stream_params *audio_buffer_get_stream_params(struct sof_audio_buffer *buffer)
{
return buffer->audio_stream_params;
}

/**
* @brief return a pointer to struct sof_audio_buffer from sink pointer
* NOTE! ensure that sink is really provided by sof_audio_buffer
Expand All @@ -79,18 +188,32 @@ static inline struct sof_audio_buffer *sof_audo_buffer_from_source(struct sof_so
}

/**
* @brief free buffer and all allocated resources
* @brief initialize audio buffer structures
*
* @param buffer pointer to the audio_buffer
* @param buffer_type a type of the buffer, BUFFER_TYPE_*
* @param is_shared indicates if the buffer will be shared between cores
* @param source_ops pointer to virtual methods table for source API
* @param sink_ops pointer to virtual methods table for sink API
* @param audio_stream_params pointer to audio stream (currently kept in buffer implementation)
*/
static inline
void audio_buffer_free(struct sof_audio_buffer *buffer)
void audio_buffer_init(struct sof_audio_buffer *buffer, uint32_t buffer_type, bool is_shared,
const struct source_ops *source_ops, const struct sink_ops *sink_ops,
struct sof_audio_stream_params *audio_stream_params)
{
if (!buffer)
return;

CORE_CHECK_STRUCT(buffer);
if (buffer->free)
buffer->free(buffer);
rfree(buffer);
CORE_CHECK_STRUCT_INIT(&buffer, is_shared);
buffer->buffer_type = buffer_type;
buffer->audio_stream_params = audio_stream_params;
source_init(audio_buffer_get_source(buffer), source_ops,
audio_buffer_get_stream_params(buffer));
sink_init(audio_buffer_get_sink(buffer), sink_ops,
audio_buffer_get_stream_params(buffer));
}

/**
* @brief free buffer and all allocated resources
*/
void audio_buffer_free(struct sof_audio_buffer *buffer);

#endif /* __SOF_AUDIO_BUFFER__ */
1 change: 1 addition & 0 deletions zephyr/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,7 @@ zephyr_library_sources(
# SOF mandatory audio processing
${SOF_AUDIO_PATH}/channel_map.c
${SOF_AUDIO_PATH}/buffer.c
${SOF_AUDIO_PATH}/buffers/audio_buffer.c
${SOF_AUDIO_PATH}/source_api_helper.c
${SOF_AUDIO_PATH}/sink_api_helper.c
${SOF_AUDIO_PATH}/sink_source_utils.c
Expand Down