Pipeline2.0 STEP2 - move hybrid buffering logic from comp_buffer to audio_buffer and partially place comp_buffer on top of audio_buffer #9299
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| # SPDX-License-Identifier: BSD-3-Clause | ||
|
|
||
| add_local_sources(sof audio_buffer.c) | ||
| add_local_sources(sof comp_buffer.c) | ||
|
|
||
| if(CONFIG_PIPELINE_2_0) | ||
| add_local_sources(sof ring_buffer.c) | ||
| endif() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
|
There was a problem hiding this comment. 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->ops->free) | ||
| buffer->ops->free(buffer); | ||
| rfree(buffer); | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can do this in the next PR if needed (as CI is good), but does it make sense to rename like other similar files to legacy-buffer.c ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
legacy-buffer.c
looks as a good idea
also struct comp_buffer ==> struct legacy_buffer
it will be a clear sign that an API is depreciated