module: waves: rework module to use sink/source api

[softwarecki]

Rework the waves module to only use the sink/source api to prepare sof for the full transition to pipeline 2.0.

[Copilot]

Pull request overview

Reworks the WAVES module adapter processing path to use the sink/source API (ring-buffer based) instead of the legacy raw input/output buffer API, aligning the module with the pipeline 2.0 transition.

Changes:

• Switch waves_codec_process() to the sink/source process callback signature.
• Read input via source_get_data() (with wrap handling) into the module’s internal input buffer.
• Write output via sink_get_buffer() + sink_commit_buffer() (with wrap handling) from the module’s internal output buffer.

Comments suppressed due to low confidence (3)

src/audio/module_adapter/module/waves/waves.c:766

• source_get_data() expects void const **buffer_start, but src_buf_start is declared as void * and passed by address, which is an incompatible pointer type and can trigger build warnings/errors. Declare src_buf_start as const void * (or uint8_t const *) and keep the pointer types consistent with source_api.h.

	const void *src_ptr;
	void *src_buf_start;
	size_t src_buf_size;

src/audio/module_adapter/module/waves/waves.c:796

• Input is released from the source (source_release_data(..., n_bytes)) immediately after copying, before MaxxEffect_Process() and before ensuring the output can be written. If processing fails or sink_get_buffer() returns an error, the input has already been consumed and will be lost (previous legacy path kept consumed=0 on errors). Delay releasing the source until after successful processing/output commit, and on failure paths release 0 bytes to drop the reservation without consuming.

	ret = source_get_data(sources[0], n_bytes, &src_ptr, &src_buf_start, &src_buf_size);
	if (ret)
		return ret;

	/* src_buf_size is the total ring buffer size; handle wrap when copying to in_buff */
	size_to_wrap = (const uint8_t *)src_buf_start + src_buf_size - (const uint8_t *)src_ptr;
	if (n_bytes <= size_to_wrap) {
		memcpy_s(codec->mpd.in_buff, n_bytes, src_ptr, n_bytes);
	} else {
		memcpy_s(codec->mpd.in_buff, n_bytes, src_ptr, size_to_wrap);
		memcpy_s((uint8_t *)codec->mpd.in_buff + size_to_wrap, n_bytes - size_to_wrap,
			 src_buf_start, n_bytes - size_to_wrap);
	}
	source_release_data(sources[0], n_bytes);
	codec->mpd.avail = n_bytes;

src/audio/module_adapter/module/waves/waves.c:836

• There is no check that the sink has enough free space before running the effect, and sink_get_buffer() errors are handled after the input has already been released. If the sink is full, this can cause dropped samples or re-processing on retry (depending on how source release is fixed). Add an early sink_get_free_size() check (e.g., against codec->mpd.out_buff_size/expected produced bytes) and return -ENOSPC without consuming input when there isn’t enough space.

		codec->mpd.produced = waves_codec->o_stream.numAvailableSamples *
			waves_codec->o_format.numChannels * waves_codec->sample_size_in_bytes;
		codec->mpd.consumed = codec->mpd.produced;
		ret = sink_get_buffer(sinks[0], codec->mpd.produced,
				      &snk_ptr, &snk_buf_start, &snk_buf_size);
		if (!ret) {

[lgirdwood]

@softwarecki do we have an init check for this when buffers are setup ? Seems valid.

[softwarecki]

@lgirdwood Number of sources and sinks is checked in module_adapter_prepare(). It returns an error if any of these values is zero. So process() cannot be called without at least one source and sink.

sof/src/audio/module_adapter/module_adapter.c

Lines 488 to 515 in c351193

	/* compute number of input buffers */
	mod->num_of_sources = 0;
	list_for_item(blist, &dev->bsource_list)
	mod->num_of_sources++;
	/* compute number of output buffers */
	mod->num_of_sinks = 0;
	list_for_item(blist, &dev->bsink_list)
	mod->num_of_sinks++;
	if (!mod->num_of_sources && !mod->num_of_sinks) {
	comp_err(dev, "no source and sink buffers connected!");
	return -EINVAL;
	}
	if (mod->num_of_sources > CONFIG_MODULE_MAX_CONNECTIONS ||
	mod->num_of_sinks > CONFIG_MODULE_MAX_CONNECTIONS) {
	comp_err(dev, "too many connected sinks %d or sources %d!",
	mod->num_of_sinks, mod->num_of_sources);
	return -EINVAL;
	}
	if (mod->num_of_sources > mod->max_sources ||
	mod->num_of_sinks > mod->max_sinks) {
	comp_err(dev, "connected sources %d (max %d) or sinks %d (max %d) exceed module capacity",
	mod->num_of_sources, mod->max_sources,
	mod->num_of_sinks, mod->max_sinks);
	return -EINVAL;
	}

[wjablon1]

I don't fully understand why you didn’t use codec->mpd.in_buff_size as a destination size. I guess it would require another check due to the subtraction in the 2nd memcpy_s under "else"

[softwarecki]

size_t n_bytes = codec->mpd.in_buff_size; at line 770.

[kv2019i]

A couple of smaller notes, see comments inline.

[kv2019i]

This is a slight change in function. source_release_data() consumes the data before call to MaxxEffect_Process(). If process fails, it's not possibly to retry. The old raw-buffer path seemed to handle this case.

[kv2019i]

Also, should the consumed amount be set based on how much was actually consumed by process (like done in old code)?