how do we know this is the right change?

IIRC the code was tested precisely on capture by @bardliao, so why wasn't this caught earlier?

Lost in space and it's only Monday 9:30 am.

is there an issue filed to show what was failing without this @RanderWang

@plbossart @ranj063 When I use buffer copier to convert format from S32_LE to S16_LE in DMIC capture, I found that the source module can't match up with this buffer copier since it use output format S16_LE as reference, not input. We didn't get error since we just use buffer copier as a simple copy, not format conversion.

@plbossart @ranj063 I never changed audio format in the buffer type copier. i.e. The input and output format are always the same in my test. The issue @RanderWang met is that the output format of DMIC is S32_LE and he want to convert it to S16_LE in the buffer copier, and the existing code search S16_LE from available_fmt->out_audio_fmt and there is no S16_LE in the buffer copier's output available formats.
IMHO, the buffer type copier should always search audio format from input available format just like other modules like gain, mixer, etc. And pipeline_params should be changed to the output audio format. Note that pipeline_params is used to pass the audio format to the next widget, therefore it should always be the same as the previous widget's output format.

Looks like people need a PhD in IPC4 to figure out how to deal with formats?

I for one have zero understanding of what these concepts are, nor what matching means or what the 'reference' is. can we get an explanation of what this is:

available_fmt->base_config->audio_fmt
available_fmt->out_audio_fmt
available_fmt->in_audio_fmt

@plbossart available_fmt is the available format sets of a widget. available_fmt->base_config->audio_fmt is the input format, available_fmt->out_audio_fmt is the output format and the input format is included in available_fmt->base_config, so there is no available_fmt->in_audio_fmt.
available_fmt->ref_audio_fmt is either available_fmt->base_config->audio_fmt or available_fmt->out_audio_fmt which is used to match the runtime audio format. For example, if hw_params is S32_LE, 48000Hz, 2ch, and available_fmt->ref_audio_fmt = available_fmt->base_config->audio_fmt, sof_ipc4_init_audio_fmt will try to find the S32_LE, 48000Hz, 2ch format from available_fmt->base_config->audio_fmt. and copy the matched available_fmt->base_config to the widget's base_config.

The idea is that we list all available formats in topology and select the one that match the runtime audio format.

IMHO, the buffer type copier should always search audio format from input available format just like other modules like gain, mixer, etc. And pipeline_params should be changed to the output audio format. Note that pipeline_params is used to pass the audio format to the next widget, therefore it should always be the same as the previous widget's output format.

@bardliao @RanderWang this patch only changes the criterion for matching. Where aer we doing the part where we update the pipeline_params to reflect what the output format of the buffer copier is?

@ranj063 yes, it is fixed by #4097 ? I didn't get this issue since the sink module is in another pipeline.

@ranj063 I added some code to deal with output format but finally found we have processed this case in this function.

static int
sof_ipc4_prepare_copier_module(struct snd_sof_widget *swidget,
			       struct snd_pcm_hw_params *fe_params,
			       struct snd_sof_platform_stream_params *platform_params,
			       struct snd_pcm_hw_params *pipeline_params, int dir)
{
...
	/* modify the input params for the next widget */
	fmt = hw_param_mask(pipeline_params, SNDRV_PCM_HW_PARAM_FORMAT);
	out_sample_valid_bits =
		SOF_IPC4_AUDIO_FORMAT_CFG_V_BIT_DEPTH(copier_data->out_format.fmt_cfg);
	snd_mask_none(fmt);
	switch (out_sample_valid_bits) {
	case 16:
		snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S16_LE);
		break;
	case 24:
		snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S24_LE);
		break;
	case 32:
		snd_mask_set_format(fmt, SNDRV_PCM_FORMAT_S32_LE);
		break;
	default:
		dev_err(sdev->dev, "invalid sample frame format %d\n",
			params_format(pipeline_params));
		return -EINVAL;
	}
...
}

I designed a pipeline with two buffer copiers to do format conversion from 32 to 16 then 16 to 32, everything works with this PR.