Skip to content

Audio: Volume: Fix the ramping and ZC mode align #10704

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

Draft
singalsu wants to merge 2 commits into
base: main
Choose a base branch
from
+47 −3
Draft

Audio: Volume: Fix the ramping and ZC mode align #10704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
263f85e
Audio: Audio Stream: Add function to calculate aligned number of frames
singalsu Apr 15, 2026
f4a3c0c
Audio: Volume: Fix the ramping and ZC mode align
singalsu Apr 15, 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
18 changes: 15 additions & 3 deletions src/audio/volume/volume.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,7 @@ static int volume_process(struct processing_module *mod,
struct output_stream_buffer *output_buffers, int num_output_buffers)
{
struct vol_data *cd = module_get_private_data(mod);
struct audio_stream *source = input_buffers[0].data;
uint32_t avail_frames = input_buffers[0].size;
uint32_t frames;
int64_t prev_sum = 0;
Expand All @@ -571,12 +572,23 @@ static int volume_process(struct processing_module *mod,
frames = avail_frames;
} else if (cd->ramp_type == SOF_VOLUME_LINEAR_ZC) {
/* with ZC ramping look for next ZC offset */
frames = cd->zc_get(input_buffers[0].data, cd->vol_ramp_frames, &prev_sum);
frames = cd->zc_get(source, cd->vol_ramp_frames, &prev_sum);
/* Align frames count to audio stream constraints */
frames = audio_stream_align_frames_round_up(source, frames);
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Apr 16, 2026

Choose a reason for hiding this comment

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

is rounding up correct? Wouldn't that grab incomplete frames? Same below. And if you round down then you presumably wouldn't need to add lines 589-590

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

A round to nearest (up or down) would be best for ZC but need to check if zero frames need more handling.

} else {
/* without ZC process max ramp chunk */
frames = cd->vol_ramp_frames;
/* During volume ramp align the number of frames used in this
* gain step. Align up since with low rates this would typically
* become zero.
*/
frames = audio_stream_align_frames_round_up(source, cd->vol_ramp_frames);
}

/* Cancel the gain step for ZC or smaller ramp step if it exceeds
* the available frames.
*/
if (frames > avail_frames)
frames = avail_frames;

if (!cd->ramp_finished) {
volume_ramp(mod);
cd->vol_ramp_elapsed_frames += frames;
Expand Down
32 changes: 32 additions & 0 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 @@ -612,6 +612,38 @@ audio_stream_avail_frames_aligned(const struct audio_stream *source,
return MIN(src_frames, sink_frames);
}

/**
* Rounds down a frame count to meet the alignment constraint of the stream.
* @param stream Audio stream with alignment requirements set.
* @param frames Frame count to round down.
* @return Largest aligned frame count less than or equal to frames.
*/
static inline uint32_t audio_stream_align_frames_round_down(const struct audio_stream *stream,
uint32_t frames)
{
uint16_t align = stream->runtime_stream_params.align_frame_cnt;

return (frames / align) * align;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Apr 16, 2026

Choose a reason for hiding this comment

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

can alignment be not a power of 2? We have multiple locations in SOF where we enforce that assumption and then use bit-wise operations apply such power-of-2 alignment. Either way you can use ROUND_DOWN() for the generic case or ALIGN_DOWN() for the power-of-2 case

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

The align can be any positive integer number. ROUND_DOWN is some operation with modulo and ALIGN_DOWN is for power of 2. I prefer this simple equation.

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

ROUND_DOWN() in Zephyr seems to be same as this but I need to check. See https://docs.zephyrproject.org/latest/doxygen/html/group__sys-util.html#gad8d2389dbe7ea135eccf237dbafb76dd

But in SOF numbers.h it's not done (with modulo) like in Zephyr documentation with basically same equation that I used.

}

/**
* Rounds up a frame count to meet the alignment constraint of the stream.
* @param stream Audio stream with alignment requirements set.
* @param frames Frame count to round up.
* @return Smallest aligned frame count greater than or equal to frames.
*/
static inline uint32_t audio_stream_align_frames_round_up(const struct audio_stream *stream,
uint32_t frames)
{
uint16_t align = stream->runtime_stream_params.align_frame_cnt;
uint32_t aligned_frames = (frames / align) * align;

if (aligned_frames < frames)
aligned_frames += align;
Copy link
Copy Markdown
Collaborator

@lyakh lyakh Apr 16, 2026

Choose a reason for hiding this comment

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

ROUND_UP() or ALIGN_UP()

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

I prefer this simple equation to know exactly what it does.

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

So, I reject your reject.

Copy link
Copy Markdown
Collaborator Author

@singalsu singalsu Apr 16, 2026

Choose a reason for hiding this comment

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

OK, ROUND_UP() looks the the same but it's not in available headers for testbench build (no Zephyr headers), would need to add it.


return aligned_frames;
}

/**
* Updates the buffer state after writing to the buffer.
* @param buffer Buffer to update.
Expand Down
Loading