Tools: Tune: Collection of patches to produce ALSA UCM friendly binary control blobs #9070

singalsu · 2024-04-22T15:09:13Z

First commit changes the export format to needed format with additional 8 bytes header that is required UCM's cset-tlv.

The other patches clean up the binary and ASCII text blobs exports for nicer directory structure and file names.

lgirdwood

@singalsu next steps are to

prefix all tools with sof- and make them executable
Move tool to appropriate module directory
Install them in make install Cmake rule - @marc-hb will know what to do here and can help.

lgirdwood · 2024-04-23T11:13:21Z

@wszypelt good to merge ? I dont think internal CI is testing these scripts yet.

ShriramShastry

I reviewed the changes, and they appear to be appropriate. I have a few suggestions I have added.

Thank you

ShriramShastry · 2024-04-23T12:04:55Z

tools/tune/common/sof_ucm_blob_write.m

+SOF_CTRL_CMD_BINARY = 3;
+nh = 8;
+nb = length(blob8);
+ublob8 = zeros(nb + nh, 1);


I think it would be better. Since ublob8 is made up of 8-bit values, writing it as 'uint8' accurately reflects its contents. Ensure that input blob8 contains no unintentional 8-bit unsigned integers.
ublob8 = zeros(nb + nh, 1, 'uint8');

I agree this would be good to do (Matlab compiler might require this). Please wait for new version for this easy improvement.

ShriramShastry · 2024-04-23T12:30:45Z

tools/tune/eq/example_fir_eq.m

@@ -10,17 +10,19 @@ function example_fir_eq()

 %% Common definitions
 fs = 48e3;
-fn.cpath3 = '../../ctl/ipc3';
-fn.cpath4 = '../../ctl/ipc4';
+fn.cpath3 = '../../ctl/ipc3/eq_fir';


There is no check to see if these directories exist before writing files into them, which may lead to errors. Can you consider adding checks and possibly creating directories that do not currently exist?

I think it's better to leave to another PR. There could be a function dir = sof_directory_create_check(dir);

ShriramShastry · 2024-04-23T12:31:23Z

tools/tune/eq/example_fir_eq.m

-fn.cpath3 = '../../ctl/ipc3';
-fn.cpath4 = '../../ctl/ipc4';
+fn.cpath3 = '../../ctl/ipc3/eq_fir';
+fn.cpath4 = '../../ctl/ipc4/eq_fir';


here - same as well?

ShriramShastry · 2024-04-23T12:43:27Z

tools/tune/eq/example_iir_eq.m

@@ -10,17 +10,19 @@ function example_iir_eq()

 %% Common definitions
 fs = 48e3;
-fn.cpath3 = '../../ctl/ipc3';
-fn.cpath4 = '../../ctl/ipc4';
+fn.cpath3 = '../../ctl/ipc3/eq_iir';


At line 168 and 169, unnecessary semicolon is added. I hope it is OK to remove

Yep, maybe leave to next PR since I'm not changing these lines with this topic.

ShriramShastry · 2024-04-23T12:48:24Z

tools/tune/eq/example_spk_eq.m

@@ -15,8 +15,10 @@ function example_spk_eq()

 %% Defaults
 fs = 48e3;
-fn.cpath3 = '../../ctl/ipc3';
-fn.cpath4 = '../../ctl/ipc4';
+iir.cpath3 = '../../ctl/ipc3/eq_iir';


There are few minor ; like eq.enable_iir; and additional [] braces are there like [ bq_fir ]) at line number 137 and 154. Please consider removing them.

Also better to put into other PR.

ShriramShastry · 2024-04-23T12:54:56Z

tools/tune/eq/example_spk_eq.m

 	eq_tplg2_write(fullfile(iir.tpath2, iir.tplg2), bp_iir, 'eq_iir', iir.comment);
 end

+rmpath ../common


There are numerous places in the addpath where rmpath is used.
Path management in MATLAB functions is less desirable due to global effects that can result in unexpected behaviour. I would suggest using helper functions or subfunctions to handle it.

I don't have clear idea how to do it. Also postpone to other PR. We need to also find a way to handle the common functions when the scripts are moved into modules (@lgirdwood).

All binaries are usually installed in ${prefix}/bin and any common module would normally end up in ${prefix}/share/sof/ - pls see how ALSA installs tools and common files, we should adopt the same approach.
@singalsu @ShriramShastry the install prefix will come from Cmake, so can matlab take a relative path to {prefix} ?

ShriramShastry · 2024-04-23T13:09:10Z

tools/tune/multiband_drc/example_multiband_drc.m

@@ -140,9 +140,9 @@ function export_multiband_drc(prm)

 tplg_write(tplg1_fn, blob8, "MULTIBAND_DRC");
 tplg2_write(tplg2_fn, blob8_ipc4, "multiband_drc_config", "Exported with script example_multiband_drc.m");
-blob_write(blob3_fn, blob8);
+sof_ucm_blob_write(blob3_fn, blob8);


Adding checks to the beginning of the export_multiband_drc function can improve the code's robustness.

if prm.sample_rate <= 0 error('Sample rate must be positive.'); end if isempty(prm.enable_bands) || length(prm.enable_bands) < prm.num_bands error('Enabled bands array does not match the number of bands.'); end

Also perform similar checks for band_lower_freq and other crucial parameters

Yep, also into other PR. I'm not resolving these to keep these reminders for changes visible.

wszypelt · 2024-04-23T13:27:17Z

@lgirdwood the current CI passes correctly

marc-hb · 2024-04-23T23:11:07Z

@lgirdwood the current CI passes correctly

current = https://sof-ci.01.org/sof-pr-viewer/#/build/PR9070/build13904370. These references are lost on every force push, so always save them in comments (in my dreams CIs would post that comment...)

The sof_ucm_blob_write.m is an updated copy of blob_write that exports binary blobs with header that is suitable for ALSA UCM's cset-tlv command. The UCM requires binary files so the default binary export is changed for every component setup script to this format. The ASCII decimal numbers .txt format export remains suitable for sof-ctl tool. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

There were no bytes control binary blobs for highpass filters. This patch adds export of them and adds a 100 Hz filter option. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The replacement is sof_ucm_blob_write.m. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

We have both .bin and .blob file name suffixes in use for similar byte controls initialize files. The scripts those generate files with .bin are changed to .blob. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

To remove clutter from upper level this patch changes generated files naming from e.g. "ipc4/eq_fir_loudness.blob" to "ipc4/eq_fir/loudness.blob". It helps to find the blob files from directory that has same name as the target component. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

This patch moves blobs for crossover component into sub-directory "crossover" to clean up clutter from ctl/ipc3 and ipc4 level. The patch also adds export of blobs for IPC4 mode that was missing. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu · 2024-04-24T15:40:43Z

I updated the Matlab/Octave zeros() function to use uint8 type as @ShriramShastry suggested. I added also the missing license text comment to the new function. There are no other changes.

lgirdwood · 2024-04-25T14:32:26Z

@wszypelt good to merge ? I dont think Internal CI is testing matlab atm. Thanks !

plbossart · 2024-04-25T14:51:21Z

tools/tune/crossover/example_crossover.m

@@ -39,7 +39,8 @@ function export_crossover(cr)
 endian = "little";
 tpath1 = '../../topology/topology1/m4/crossover';
 tpath2 = '../../topology/topology2/include/components/crossover';
-ctlpath = '../../ctl/ipc3';
+ctlpath3 = '../../ctl/ipc3/crossover';
+ctlpath4 = '../../ctl/ipc4/crossover';


@singalsu presumably an OEM or integrator will have multiple SKUs to tune. Should there be some sort of prefix for the blobs so that the storage can be specified as SKU-specific?

And while I am at it, can those tuning scripts actually be used in a script that generates blobs for multiple platforms, just taking input parameters for each SKU?

marc-hb · 2024-04-25T21:29:04Z

tools/tune/eq/example_fir_eq.m

@@ -21,7 +21,7 @@ function example_fir_eq()
 %% -------------------
 %% Example 1: Loudness
 %% -------------------
-fn.bin = 'eq_fir_loudness.bin';
+fn.bin = 'eq_fir_loudness.blob';


Thanks, I was a bit confused by .bin. It's much clearer now with .blob!

:-)))

wszypelt · 2024-04-29T08:47:41Z

@lgirdwood I've been away for a few days, I'll try this build again. results should be available within an hour

singalsu marked this pull request as ready for review April 22, 2024 15:09

singalsu requested review from a team, lgirdwood, plbossart, mmaka1, lbetlej, dbaluta and kv2019i as code owners April 22, 2024 15:09

singalsu requested a review from ShriramShastry April 22, 2024 15:09

kv2019i approved these changes Apr 22, 2024

View reviewed changes

lgirdwood approved these changes Apr 23, 2024

View reviewed changes

ShriramShastry approved these changes Apr 23, 2024

View reviewed changes

singalsu added 6 commits April 24, 2024 18:35

Tools: Tune: EQ: Create highpass blobs and add 100 Hz version

616765b

There were no bytes control binary blobs for highpass filters. This patch adds export of them and adds a 100 Hz filter option. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Tools: Tune: EQ: The function eq_blob_write.m is deleted

8b698a4

The replacement is sof_ucm_blob_write.m. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Tools: Tune: EQ: Rename configuration blobs to use .blob

44ba747

We have both .bin and .blob file name suffixes in use for similar byte controls initialize files. The scripts those generate files with .bin are changed to .blob. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

singalsu force-pushed the change_blob_format_for_ucm branch from 218ed71 to 6908a5b Compare April 24, 2024 15:38

lgirdwood approved these changes Apr 25, 2024

View reviewed changes

plbossart reviewed Apr 25, 2024

View reviewed changes

marc-hb reviewed Apr 25, 2024

View reviewed changes

lgirdwood merged commit e8fcaf6 into thesofproject:main Apr 29, 2024
44 of 45 checks passed

singalsu mentioned this pull request May 2, 2024

[BUG] sof-ctl blob format is not the same as with ALSA UCM, should align the sof-ctl format #9092

Open

singalsu deleted the change_blob_format_for_ucm branch August 21, 2024 08:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tools: Tune: Collection of patches to produce ALSA UCM friendly binary control blobs #9070

Tools: Tune: Collection of patches to produce ALSA UCM friendly binary control blobs #9070

singalsu commented Apr 22, 2024

lgirdwood left a comment •

edited

Loading

lgirdwood commented Apr 23, 2024

ShriramShastry left a comment

ShriramShastry Apr 23, 2024

singalsu Apr 24, 2024

ShriramShastry Apr 23, 2024

singalsu Apr 24, 2024

ShriramShastry Apr 23, 2024

ShriramShastry Apr 23, 2024

singalsu Apr 24, 2024

ShriramShastry Apr 23, 2024 •

edited

Loading

singalsu Apr 24, 2024

ShriramShastry Apr 23, 2024

singalsu Apr 24, 2024

lgirdwood Apr 25, 2024

ShriramShastry Apr 23, 2024

singalsu Apr 24, 2024

wszypelt commented Apr 23, 2024

marc-hb commented Apr 23, 2024 •

edited

Loading

singalsu commented Apr 24, 2024

lgirdwood commented Apr 25, 2024

plbossart Apr 25, 2024

marc-hb Apr 25, 2024

wszypelt commented Apr 29, 2024

Tools: Tune: Collection of patches to produce ALSA UCM friendly binary control blobs #9070

Tools: Tune: Collection of patches to produce ALSA UCM friendly binary control blobs #9070

Conversation

singalsu commented Apr 22, 2024

lgirdwood left a comment • edited Loading

Choose a reason for hiding this comment

lgirdwood commented Apr 23, 2024

ShriramShastry left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ShriramShastry Apr 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wszypelt commented Apr 23, 2024

marc-hb commented Apr 23, 2024 • edited Loading

singalsu commented Apr 24, 2024

lgirdwood commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wszypelt commented Apr 29, 2024

lgirdwood left a comment •

edited

Loading

ShriramShastry Apr 23, 2024 •

edited

Loading

marc-hb commented Apr 23, 2024 •

edited

Loading