From 6535e7c09d32b9ea8fdba83cd9e80d7ac069bfa1 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Sun, 10 Dec 2023 19:42:39 -0800 Subject: [PATCH] google_rtc_audio_processing: Clean up prepare() method The prepare step was missing some checks, and doing others in a non-orthogonal way. Clean it up so we can be sure it validates the sample format, channel count and rate of each of the three connected streams. Signed-off-by: Andy Ross --- .../google/google_rtc_audio_processing.c | 117 ++++++++++-------- 1 file changed, 64 insertions(+), 53 deletions(-) diff --git a/src/audio/google/google_rtc_audio_processing.c b/src/audio/google/google_rtc_audio_processing.c index f3f84d2d627f..d40dc352ac1f 100644 --- a/src/audio/google/google_rtc_audio_processing.c +++ b/src/audio/google/google_rtc_audio_processing.c @@ -518,6 +518,22 @@ static int google_rtc_audio_processing_free(struct processing_module *mod) return 0; } +static bool is_ref_buffer(struct comp_dev *dev, struct comp_buffer *b) +{ +#if CONFIG_IPC_MAJOR_4 + return IPC4_SINK_QUEUE_ID(buf_get_id(b)) == SOF_AEC_FEEDBACK_QUEUE_ID; +#else + return b->source->pipeline->pipeline_id != dev->pipeline->pipeline_id; +#endif +} + +static enum sof_ipc_frame bits_fmt(int bits) +{ + if (bits == 32) + return SOF_IPC_FRAME_S32_LE; + return SOF_IPC_FRAME_S16_LE; +} + static int google_rtc_audio_processing_prepare(struct processing_module *mod, struct sof_source **sources, int num_of_sources, @@ -527,12 +543,7 @@ static int google_rtc_audio_processing_prepare(struct processing_module *mod, struct comp_dev *dev = mod->dev; struct google_rtc_audio_processing_comp_data *cd = module_get_private_data(mod); struct list_item *source_buffer_list_item; - struct comp_buffer *output; - unsigned int aec_channels = 0, frame_fmt, rate; - int microphone_stream_channels = 0; - int output_stream_channels; - int ret; - int i = 0; + int ret = 0, i = 0; comp_info(dev, "google_rtc_audio_processing_prepare()"); @@ -540,75 +551,75 @@ static int google_rtc_audio_processing_prepare(struct processing_module *mod, list_for_item(source_buffer_list_item, &dev->bsource_list) { struct comp_buffer *source = container_of(source_buffer_list_item, struct comp_buffer, sink_list); -#if CONFIG_IPC_MAJOR_4 - if (IPC4_SINK_QUEUE_ID(buf_get_id(source)) == - SOF_AEC_FEEDBACK_QUEUE_ID) { -#else - if (source->source->pipeline->pipeline_id != dev->pipeline->pipeline_id) { -#endif + if (is_ref_buffer(dev, source)) { cd->aec_reference_source = i; cd->ref_comp_buffer = source; - aec_channels = audio_stream_get_channels(&source->stream); - comp_dbg(dev, "reference index = %d, channels = %d", i, aec_channels); } else { cd->raw_microphone_source = i; - microphone_stream_channels = audio_stream_get_channels(&source->stream); - comp_dbg(dev, "microphone index = %d, channels = %d", i, - microphone_stream_channels); } - - audio_stream_init_alignment_constants(1, 1, &source->stream); i++; } - output = list_first_item(&dev->bsink_list, struct comp_buffer, source_list); - - /* On some platform the playback output is left right left right due to a crossover - * later on the signal processing chain. That makes the aec_reference be 4 channels - * and the AEC should only use the 2 first. + /* Validate channel, format and rate on each of our three + * inputs. All much match our build-time configuration, AEC + * does not handle dynamic stream formats. */ - if (cd->num_aec_reference_channels > aec_channels) { - comp_err(dev, "unsupported number of AEC reference channels: %d", - aec_channels); - return -EINVAL; + int ref_fmt = source_get_frm_fmt(sources[cd->aec_reference_source]); + int ref_chan = source_get_channels(sources[cd->aec_reference_source]); + int ref_rate = source_get_rate(sources[cd->aec_reference_source]); + + int mic_fmt = source_get_frm_fmt(sources[cd->raw_microphone_source]); + int mic_chan = source_get_channels(sources[cd->raw_microphone_source]); + int mic_rate = source_get_rate(sources[cd->raw_microphone_source]); + + int out_fmt = sink_get_frm_fmt(sinks[0]); + int out_chan = sink_get_channels(sinks[0]); + int out_rate = sink_get_rate(sinks[0]); + + /* Too many channels is a soft failure, AEC treats only the first N */ + if (ref_chan > REF_CHAN_MAX) + comp_warn(dev, "Too many ref channels: %d, truncating to %d", + ref_chan, REF_CHAN_MAX); + if (mic_chan > MIC_CHAN_MAX) + comp_warn(dev, "Too many mic channels: %d, truncating to %d", + mic_chan, MIC_CHAN_MAX); + + if (out_chan != mic_chan) { + comp_err(dev, "Input/output mic channel mismatch"); + ret = -EINVAL; } - audio_stream_init_alignment_constants(1, 1, &output->stream); - frame_fmt = audio_stream_get_frm_fmt(&output->stream); - rate = audio_stream_get_rate(&output->stream); - output_stream_channels = audio_stream_get_channels(&output->stream); - - if (microphone_stream_channels != output_stream_channels) - return -EINVAL; + if (ref_rate != mic_rate || ref_rate != out_rate || + ref_rate != CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING_SAMPLE_RATE_HZ) { + comp_err(dev, "Incorrect source/sink sample rate, expect %d\n", + CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING_SAMPLE_RATE_HZ); + ret = -EINVAL; + } - if (microphone_stream_channels > MIC_CHAN_MAX) { - comp_warn(dev, "Too many mic channels: %d (max %d), truncating", - microphone_stream_channels, MIC_CHAN_MAX); + if (ref_fmt != bits_fmt(CONFIG_GOOGLE_RTC_AUDIO_PROCESSING_REF_BITS)) { + comp_err(dev, "Reference stream must be %d bit samples\n", + CONFIG_GOOGLE_RTC_AUDIO_PROCESSING_REF_BITS); + ret = -EINVAL; } - switch (frame_fmt) { -#if CONFIG_FORMAT_S16LE - case SOF_IPC_FRAME_S16_LE: - break; -#endif /* CONFIG_FORMAT_S16LE */ - default: - comp_err(dev, "unsupported data format: %d", frame_fmt); - return -EINVAL; + if (mic_fmt != bits_fmt(CONFIG_GOOGLE_RTC_AUDIO_PROCESSING_MIC_BITS)) { + comp_err(dev, "Mic stream must be %d bit samples\n", + CONFIG_GOOGLE_RTC_AUDIO_PROCESSING_MIC_BITS); + ret = -EINVAL; } - if (rate != CONFIG_COMP_GOOGLE_RTC_AUDIO_PROCESSING_SAMPLE_RATE_HZ) { - comp_err(dev, "unsupported samplerate: %d", rate); - return -EINVAL; + if (mic_fmt != out_fmt) { + comp_err(dev, "Mismatched in/out frame format"); + ret = -EINVAL; } /* Blobs sent during COMP_STATE_READY is assigned to blob_handler->data * directly, so comp_is_new_data_blob_available always returns false. */ - ret = google_rtc_audio_processing_reconfigure(mod); - if (ret) - return ret; + if (ret == 0) + ret = google_rtc_audio_processing_reconfigure(mod); - return 0; + return ret; } static int trigger_handler(struct processing_module *mod, int cmd)