From ec5775d1aac6d1b11c224066b986411dad19faf3 Mon Sep 17 00:00:00 2001 From: Ben Zhang Date: Fri, 25 Mar 2022 15:50:43 -0400 Subject: [PATCH] google_rtc_audio_processing: Make control write reliable Userspace may write the config control repeatedly in quick succession. Accept the new config in cmd callback to avoid -EBUSY error and ensure the last successful control write before prepare/copy is applied. This patch also fixes an edge case during init where the first non-empty control write happens after prepare. This patch ensures the config is applied during copy. Tested: 1) Writes in quick succession for i in $(seq 1 100); do ./sof-ctl -n 92 -t 0 -b -r -s /root/test_blob0.txt & ./sof-ctl -n 92 -t 0 -b -r -s /root/test_blob1.txt & done 2) Large config requiring multiple ipcs for i in $(seq 1 1000); do ./sof-ctl -n 92 -t 0 -b -r -s /root/large_blob.txt ./sof-ctl -n 92 -t 0 -b -r -s /root/test_blob1.txt done 3) Reboot, write before and after arecord starts 4) Suspend/resume Signed-off-by: Ben Zhang --- src/audio/google_rtc_audio_processing.c | 30 +++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/audio/google_rtc_audio_processing.c b/src/audio/google_rtc_audio_processing.c index 01a3d906791e..249b5d5f6726 100644 --- a/src/audio/google_rtc_audio_processing.c +++ b/src/audio/google_rtc_audio_processing.c @@ -56,6 +56,7 @@ struct google_rtc_audio_processing_comp_data { int16_t *output_buffer; int16_t output_buffer_index; struct comp_data_blob_handler *tuning_handler; + bool reconfigure; }; void *GoogleRtcMalloc(size_t size) @@ -107,6 +108,7 @@ static int google_rtc_audio_processing_reconfigure(struct comp_dev *dev) comp_info(dev, "google_rtc_audio_processing_reconfigure(): New tuning config %p (%zu bytes)", config, size); + cd->reconfigure = false; ret = GoogleRtcAudioProcessingReconfigure(cd->state, config, size); if (ret) { comp_err(dev, "GoogleRtcAudioProcessingReconfigure failed: %d", @@ -122,10 +124,27 @@ static int google_rtc_audio_processing_cmd_set_data( struct sof_ipc_ctrl_data *cdata) { struct google_rtc_audio_processing_comp_data *cd = comp_get_drvdata(dev); + int ret; switch (cdata->cmd) { case SOF_CTRL_CMD_BINARY: - return comp_data_blob_set_cmd(cd->tuning_handler, cdata); + ret = comp_data_blob_set_cmd(cd->tuning_handler, cdata); + if (ret) + return ret; + /* Accept the new blob immediately so that userspace can write + * the control in quick succession without error. + * This ensures the last successful control write from userspace + * before prepare/copy is applied. + * The config blob is not referenced after reconfigure() returns + * so it is safe to call comp_get_data_blob here which frees the + * old blob. This assumes cmd() and prepare()/copy() cannot run + * concurrently which is the case when there is no preemption. + */ + if (comp_is_new_data_blob_available(cd->tuning_handler)) { + comp_get_data_blob(cd->tuning_handler, NULL, NULL); + cd->reconfigure = true; + } + return 0; default: comp_err(dev, "google_rtc_audio_processing_ctrl_set_data(): Only binary controls supported %d", @@ -236,6 +255,13 @@ static struct comp_dev *google_rtc_audio_processing_create( bzero(cd->output_buffer, cd->num_frames * sizeof(cd->output_buffer[0])); cd->output_buffer_index = 0; + /* comp_is_new_data_blob_available always returns false for the first + * control write with non-empty config. The first non-empty write may + * happen after prepare (e.g. during copy). Default to true so that + * copy keeps checking until a non-empty config is applied. + */ + cd->reconfigure = true; + comp_set_drvdata(dev, cd); dev->state = COMP_STATE_READY; comp_dbg(dev, "google_rtc_audio_processing_create(): Ready"); @@ -358,7 +384,7 @@ static int google_rtc_audio_processing_copy(struct comp_dev *dev) int channel; int ret; - if (comp_is_new_data_blob_available(cd->tuning_handler)) { + if (cd->reconfigure) { ret = google_rtc_audio_processing_reconfigure(dev); if (ret) return ret;