Skip to content

Audio: Fix the default alignment setting #8351

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
kv2019i merged 1 commit into from
Oct 26, 2023
Merged

Audio: Fix the default alignment setting #8351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 0 additions & 6 deletions src/audio/audio_stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,12 +141,6 @@ void audio_stream_init(struct audio_stream *audio_stream, void *buff_addr, uint3
audio_stream->addr = buff_addr;
audio_stream->end_addr = (char *)audio_stream->addr + size;

/* set the default alignment info.
* set byte_align as 1 means no alignment limit on byte.
* set frame_align as 1 means no alignment limit on frame.
*/
audio_stream_init_alignment_constants(1, 1, audio_stream);

source_init(audio_stream_get_source(audio_stream), &audio_stream_source_ops,
&audio_stream->runtime_stream_params);
sink_init(audio_stream_get_sink(audio_stream), &audio_stream_sink_ops,
Expand Down
44 changes: 25 additions & 19 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 @@ -348,25 +348,6 @@ static inline void audio_stream_set_buffer_fmt(struct audio_stream *buf,
#define audio_stream_get_frag(buffer, ptr, idx, sample_size) \
audio_stream_wrap(buffer, (char *)(ptr) + ((idx) * (sample_size)))

/**
* Applies parameters to the buffer.
* @param buffer Buffer.
* @param params Parameters (frame format, rate, number of channels).
* @return 0 if succeeded, error code otherwise.
*/
static inline int audio_stream_set_params(struct audio_stream *buffer,
struct sof_ipc_stream_params *params)
{
if (!params)
return -EINVAL;

buffer->runtime_stream_params.frame_fmt = params->frame_fmt;
buffer->runtime_stream_params.rate = params->rate;
buffer->runtime_stream_params.channels = params->channels;

return 0;
}

/**
* Calculates period size in bytes based on component stream's parameters.
* @param buf Component buffer.
Expand Down Expand Up @@ -433,6 +414,31 @@ static inline void audio_stream_init_alignment_constants(const uint32_t byte_ali
(is_power_of_2(process_size) ? 31 : 32) - clz(process_size);
}

/**
* Applies parameters to the buffer.
* @param buffer Audio stream buffer.
* @param params Parameters (frame format, rate, number of channels).
* @return 0 if succeeded, error code otherwise.
*/
static inline int audio_stream_set_params(struct audio_stream *buffer,
struct sof_ipc_stream_params *params)
{
if (!params)
return -EINVAL;

buffer->runtime_stream_params.frame_fmt = params->frame_fmt;
buffer->runtime_stream_params.rate = params->rate;
buffer->runtime_stream_params.channels = params->channels;

/* set the default alignment info.
* set byte_align as 1 means no alignment limit on byte.
* set frame_align as 1 means no alignment limit on frame.
*/
audio_stream_init_alignment_constants(1, 1, buffer);

@kv2019i kv2019i Oct 25, 2023

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.

Hmm, it's difficult to confirm this is safe to do. audio_stream_init() is only called once but audio_stream_set_params() is called from many places via buffer_set_params().

Have you checked that we are not overriding alignment info?

Not saying old code is correct. It would look we'd at least need to recalculate runtime_stream_params.align_frame_cnt and align_shift_idx if buffer parameters are changed.

@andrula-song andrula-song Oct 26, 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.

usually, we set alignment info in prepare function(after internal parameter set and verify) for specific component, I checked the src, eq-iir,eq-fir and volume,aria .etc components, they will change the default value to their own setting.
and yes, once the buffer parameters are changed, we do need to recalculate the align info.


return 0;
}

/**
* Calculates period size in bytes based on component stream's parameters.
* @param buf Component buffer.
Expand Down