Audio: Volume: Fix the ramping and ZC mode align by singalsu · Pull Request #10704 · thesofproject/sof

singalsu · 2026-04-15T14:09:19Z

No description provided.

lyakh · 2026-04-16T07:11:38Z

+{
+	uint16_t align = stream->runtime_stream_params.align_frame_cnt;
+
+	return (frames / align) * align;


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

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.

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.

@lyakh I implemented all align round functions with the available ROUND_DOWN(). Please re-review!

Copilot

Pull request overview

This PR updates audio stream alignment handling across several components, with a focus on fixing volume ramping behavior and making alignment defaults consistent (including HiFi/ZC-related constraints).

Changes:

Introduces a default frame-count alignment constant and applies it broadly across stream initialization and component prepare paths.
Refactors aligned-frames calculation helpers and adds new frame-rounding utilities.
Updates volume ramp/ZC processing to align ramp step sizes to stream constraints and adjusts alignment setup in multiple audio components.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
src/include/sof/common.h	Adds `SOF_FRAME_COUNT_ALIGN` default constant.
src/include/sof/audio/audio_stream.h	Refactors `audio_stream_avail_frames_aligned()` and adds round up/down helpers.
src/audio/audio_stream.c	Changes default stream alignment initialization to use new defaults.
src/audio/volume/volume.c	Aligns ramp/ZC processing frame counts and refines volume alignment requirements.
src/audio/tdfb/tdfb.c	Updates alignment constants usage for TDFB streams.
src/audio/src/src_common.h	Updates `src_set_alignment()` signature.
src/audio/src/src_common.c	Updates SRC alignment defaults and removes sink-side alignment setting.
src/audio/src/src_ipc3.c	Adjusts SRC prepare to new `src_set_alignment()` signature.
src/audio/src/src_ipc4.c	Adjusts SRC prepare to new `src_set_alignment()` signature.
src/audio/selector/selector.c	Switches selector alignment to use global defaults.
src/audio/mixer/mixer.h	Adds 6ch-specific byte alignment constant for mixer.
src/audio/mixer/mixer.c	Updates mixer alignment logic and removes sink alignment setup.
src/audio/igo_nr/igo_nr.c	Switches IGO NR alignment to use global byte-align default and removes sink alignment.
src/audio/eq_iir/eq_iir.c	Switches EQ IIR byte alignment to `SOF_FRAME_BYTE_ALIGN`.
src/audio/eq_fir/eq_fir.c	Switches EQ FIR byte alignment to `SOF_FRAME_BYTE_ALIGN` and drops sink alignment set.
src/audio/aria/aria.c	Simplifies ARIA alignment setup to use global defaults (source side).

Copilot · 2026-04-21T12:44:10Z

@@ -369,11 +369,10 @@ static int eq_fir_process(struct processing_module *mod,
 static void eq_fir_set_alignment(struct audio_stream *source,
 				 struct audio_stream *sink)
 {


eq_fir_set_alignment() still takes a sink parameter but no longer uses it, which can trigger -Wunused-parameter (often treated as an error). Either remove the unused parameter (and adjust callers) or explicitly mark it unused.

Suggested change

{

{

(void)sink;

Copilot · 2026-04-21T12:44:10Z

@@ -216,7 +216,6 @@ static int mixer_prepare(struct processing_module *mod,
 	}



mixer_prepare() no longer sets alignment requirements on the sink stream, but mixer_set_frame_alignment() contains special handling for 5.1ch (16-byte) that is likely relevant to the output pointer alignment as well (not just the inputs). If the sink isn’t configured consistently, the sink write pointer can become misaligned over time depending on processed frame counts. Consider applying the same alignment setup to sink->stream (or otherwise ensuring the output alignment requirement is enforced).

Suggested change

mixer_set_frame_alignment(&sink->stream);

I don't understand how the sink write could become misaligned since it follows the source align with same amount of frames and samples.

I added a comment why the sink align constraints are not set.

Copilot · 2026-04-21T12:44:11Z

+	return (uint32_t)((n * audio_stream_frame_bytes(source) >>
+			   source->runtime_stream_params.align_shift_idx) *
+			  source->runtime_stream_params.align_frame_cnt);


audio_stream_avail_frames_aligned() now computes the aligned frame count via (n * frame_bytes) >> align_shift_idx instead of rounding n down to a multiple of align_frame_cnt. For non-power-of-two process_size (common with 5.1ch where frame_bytes is 12/24), align_shift_idx is the ceil log2, so this expression can return 0 even when n already contains one full aligned block. That can stall processing unnecessarily. Consider using source->runtime_stream_params.align_frame_cnt directly (e.g., round n down with the new audio_stream_align_frames_round_down() helper) so one aligned block is always returned when available.

Suggested change

return (uint32_t)((n * audio_stream_frame_bytes(source) >>

source->runtime_stream_params.align_shift_idx) *

source->runtime_stream_params.align_frame_cnt);

return audio_stream_align_frames_round_down(source, (uint32_t)n);

The copilot suggestion omits align_shift_idx. Both the bytes and frames align need to be applied, and I'm not sure only the align_frame_cnt apply is enough. Also the original code does (bytes >> align_shift_idx) * align_frame_cnt so I don't think the returned frames count would become zero in this version. And if it theoretically does, it probably should, and in next copy() there should be enough free/avail to return a non-zero aligned frames count value.

Hmm, copilot's suggestion may be OK, I'll try. It's simpler than this PR version.

Copilot · 2026-04-21T12:44:11Z

+			/* Align frames count to audio stream constraints */
+			frames = audio_stream_align_frames_round_up(source, frames);


In ZC ramp mode, zc_get() returns the frame count up to the detected zero-crossing point. Rounding this value up to the stream alignment (audio_stream_align_frames_round_up()) can overshoot the zero crossing and defeat the purpose of ZC ramping (potentially reintroducing clicks). Consider rounding down for the ZC path, or reworking zc_get()/search logic so the returned ZC point is already alignment-compliant.

Suggested change

/* Align frames count to audio stream constraints */

frames = audio_stream_align_frames_round_up(source, frames);

/* Align frames count to audio stream constraints without

* overshooting the detected zero-crossing point.

*/

frames = audio_stream_align_frames_round_down(source, frames);

Round down systematically could be as off as round up. I think it should be round to nearest that can be up or down. But round down needs attention if value becomes zero. If zero, it should round up for the ramping logic to work and not get stuck to the same frame position.

kv2019i · 2026-04-21T14:52:51Z

@@ -591,9 +591,9 @@ audio_stream_avail_frames(const struct audio_stream *source,
 }


nit/typo: "caused by logic int the function" -> "in the function"

This patch adds to audio_stream.h inline functions: - audio_stream_align_frames_round_down() - audio_stream_align_frames_round_up() - audio_stream_align_frames_round_nearest() They are useful when an algorithm needs to process a number of frames that is smaller then data size than offered by module API. E.g. when volume component ramps the volume, the block with same gain is smaller than a normal processing period. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch fixes an issue where processing smaller parts of the period can break the data align constraints. The frames count value from zc_get() function or from cd->vol_ramp_frames is rounded up to next align compatible frames count. The failure with align constraints appeared as glitches in audio with some 44.1 kHz family sample rates. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

lgirdwood · 2026-04-21T15:37:21Z

@singalsu are you able to respond to all copilot comments, then we can go ahead and merge.

This patch modifies the frame align request to match the gain loop samples unroll number. In HiFi5 four s32 samples or eight s16 are processed per iteration. In HiFi3, HiFi4 and generic C two s32 samples or four s16 samples are processed per iteration. The equation frame_align_req = n / gcd(n, channels) with n = 8 or n = 4 for HiFi5 for s16 or s32 n = 4 or 4 = 2 for other builds for s16 or s32 ensures that every aligned frame count is multiple of gain loop unroll amount. The equation for byte_align is same as earlier but the unnecessary SOF_USE_HIFI() are removed. The code wasn't wrong for HiFi5 but it was confusing. The check for 6ch wasn't done because the SOF_FRAME_BYTE_ALIGN value was the same as VOLUME_HIFI3_HIFI4_FRAME_BYTE_ALIGN_6CH. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch changes the default align from 1 bytes to SOF_FRAME_BYTE_ALIGN that depends on build architecture. It defaults to 4 in generic build and to values 8 and 16 for Hifi3/4 and HiFi5 builds. A macro is defined for frame count align value 1 with name SOF_FRAME_COUNT_ALIGN. It can be used instead of numeric value 1. Modules can use it when there is no specific requirement for frames count constraint. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This change fixes an issue that is caused by the logic in the function that calculates the aligned frames counts separately for source and sink and then returns the smaller number of them. The fixed logic checks the maximum number of frames that could be processed from source to sink and adjusts the the number down with source align constraints. It prevents an issue where a downstream module in pipeline modifies align constraint for it's source that is previous module's sink. A more relaxed constraint could then break source align criteria if the number of frames to process is limited by sink. The proposed logic for align follows only source align criteria to avoid conflicting constraints. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch mofidies modules ARIA, EQFIR, EQIIR, IGO NR, Mixer, Selector, SRC, and TDFB by removing sink align constraints set. Also the frame byte align of 1 is changed to SOF_FRAME_BYTE_ALIGN. When the sink align is not set the default values from audio_stream_init() is used, or the downstream modules sets the align for the source. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu · 2026-04-21T17:18:17Z

@singalsu are you able to respond to all copilot comments, then we can go ahead and merge.

Yep, I will address them all properly. Seems there is a mandatory CI test fail that I need to check also.

Copilot

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.

lyakh requested changes Apr 16, 2026

View reviewed changes

singalsu force-pushed the volume_ramp_align_fix branch from f4a3c0c to eaf7d8a Compare April 21, 2026 11:53

singalsu commented Apr 21, 2026

View reviewed changes

Comment thread src/audio/volume/volume.c

singalsu marked this pull request as ready for review April 21, 2026 12:33

Copilot AI review requested due to automatic review settings April 21, 2026 12:33

singalsu requested review from abonislawski, dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and tlissows as code owners April 21, 2026 12:33

Copilot started reviewing on behalf of singalsu April 21, 2026 12:34 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

kv2019i approved these changes Apr 21, 2026

View reviewed changes

singalsu added 2 commits April 21, 2026 18:14

singalsu added 4 commits April 21, 2026 19:38

singalsu force-pushed the volume_ramp_align_fix branch from eaf7d8a to 40eac58 Compare April 21, 2026 17:19

singalsu requested review from Copilot and lyakh April 21, 2026 17:20

Copilot started reviewing on behalf of singalsu April 21, 2026 17:24 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

		@@ -216,7 +216,6 @@ static int mixer_prepare(struct processing_module *mod,
		}

		/* Align frames count to audio stream constraints */
		frames = audio_stream_align_frames_round_up(source, frames);

-			/* Align frames count to audio stream constraints */
-			frames = audio_stream_align_frames_round_up(source, frames);
+			/* Align frames count to audio stream constraints without
+			 * overshooting the detected zero-crossing point.
+			 */
+			frames = audio_stream_align_frames_round_down(source, frames);

		@@ -591,9 +591,9 @@ audio_stream_avail_frames(const struct audio_stream *source,
		}

Conversation

singalsu commented Apr 15, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lgirdwood commented Apr 21, 2026

Uh oh!

singalsu commented Apr 21, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants