-
Notifications
You must be signed in to change notification settings - Fork 349
Description
Describe the bug
(1)
audio_stream_avail_frames_aligned() uses shifts instead of divisions and so it returns wrong result for formats with frame size not equal to power of 2, e.g. for streams with 3, 5 or 7 channels.
(2)
Default alignment initialization in audio_stream_init() does not work as stream frame format is not yet initialized: it is always set later somewhen after that function call.
(3)
Probably, to overcome (2), every module have to call audio_stream_init_alignment_constants() to initialize alignment. Modules usually do this in .prepare() handler. However, we support binding to a module on a running pipeline without pausing it. In this case .prepare() is not called after new binding as pipeline was not paused or reset and that might left new attached buffer stream with uninitialized alignment.
Another problem is that it is easy to forget to call audio_stream_init_alignment_constants() (must be added to every module!). I see audio_stream_init_alignment_constants() is missing at least in copier, mixin and mixout. It would be better that in this case some default 1 byte alignment initialization take place.
(4)
if alignment initialization is missing -- because of (2) or (3) -- audio_stream_avail_frames_aligned() returns wrong (higher) number of frames. That frames value is used by module_adapter to invalidate buffer memory and is also passed as parameter to .process() func. Luckily, if all connected modules are on same core, buffer invalidate is skipped. For cross-core case, invalidate with wrong (higher) amount of frames leads to memory corruption and weird bugs.
Environment
- Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
- SOF: main branch.
audio_stream_avail_frames_aligned() source;
static inline uint32_t
audio_stream_avail_frames_aligned(const struct audio_stream *source,
const struct audio_stream *sink)
{
uint32_t src_frames = (audio_stream_get_avail_bytes(source) >>
source->runtime_stream_params.align_shift_idx) *
source->runtime_stream_params.align_frame_cnt;
uint32_t sink_frames = (audio_stream_get_free_bytes(sink) >>
sink->runtime_stream_params.align_shift_idx) *
sink->runtime_stream_params.align_frame_cnt;
return MIN(src_frames, sink_frames);
}