[singalsu]

The detailed description is in the commit texts. Matlab/Octave scripts can fetch the current version header with system command for:

"sof-ctl -g payloadsize -b -o fn"

to retrieve the header bytes for ext bytes control size of payloadsize into file fn. The sof-ctl tool must be installed somewhere to executable search path of computer that runs the EQ tool. And it must be kept up-to-date to get the correct ABI header.

Edit: Use of name payloadsize is better than blobsize to understand the size bytes do not include the size of the ABI header.

[singalsu]

@lgirdwood This patch implements feature issue #791 but since the changes for feature issue #1535 are not yet done I named this [RFC]. Please explain what should happen with the proposed option "-a x:y:z" present and absent so I can implement it.

[singalsu]

@keyonjie Does this command line option add work for demux component data blob generator?

[keyonjie]

Hi @singalsu we can already dump/print ABI header, if you don't specify '-r', it will dump the abi header:
-b set/get control in binary mode(e.g. for set, use binary input file, for get, dump out in hex format)
-r no abi header for the input file, or not dumping abi header for get.

please see details from the help:

Usage:	 ./sof-ctl [-D <device>] [-c <control name>] [-s <data>]
	 ./sof-ctl [-D <device>] [-n <control id>] [-s <data>]
	 ./sof-ctl -h

Where:
 -D device name (default is hw:0)
 -c control name e.g. numid=22,name=\"EQIIR1.0 EQIIR\""
 -n control id e.g. 22
 -s set data using ASCII CSV input file
 -b set/get control in binary mode(e.g. for set, use binary input file, for get, dump out in hex format)
 -r no abi header for the input file, or not dumping abi header for get.
 -o specify the output file.
 -t specify the component specified type.

such as:

root@test-cml:/home/test# ./sof-ctl -n 15
Control size is 300000.
Retrieving configuration for device hw:0 control numid=15.
Success.
hdr: magic 0x00464f53
hdr: type 1
hdr: size 64 bytes
hdr: abi 3:10:0
4607827,1,64,50372608,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0
root@test-cml:/home/test# 

[singalsu]

This is not the same thing. The usage for "-p" happens at audio tuning computer, e.g. computer where topology is defined. It needs to work with without (ssh) connection to a SOF device running a suitable topology. E.g. if I wan't a header for 128 bytes long ext bytes control:

$ sof-ctl -p 128
hdr: magic 0x00464f53
hdr: type 0
hdr: size 128 bytes
hdr: abi 3:9:0
4607827,0,128,50368512,0,0,0,0

You can see from the EQ tool commit how the command is used. This is generic and this 32 bytes header (currently) is not specific for EQs.

[singalsu]

I didn't want to change the code more than needed so the EQ tool uses the binary file output via "-b -f file.bin". The ASCII dump with the "hdr: " infos would be more complex to parse. So I just fread() as unsigned bytes the binary header blob. But a tool could as well parse this ASCII output if preferred.

[keyonjie]

@singalsu so do you mean on host Matlab, you need to send blob with different abi header(128B comparing with 32B)? do you have the struct definition for those 128 bytes? By the way, the hdr->size is for the payload, not for the header size.

[keyonjie]

if you want to generate different header, we need define how to generate it, if you only want to dump the first 128bytes of the tlv kcontrol, we can just dump all of it and select the first 128B?

[singalsu]

When we create hex blobs like SOF/tools/topology/eq*.m4 we need to know the generic binary ABI header that has changed every now and then when ABI is bumped for major version. That's currently the first 32 bytes. From EQ tools point-of-view the following EQ specific configuration request has been stable. This part is variable length (the longer the more challenging the acoustics is), so I needed the size parameter for "-p size" (with ABI header size excluded as you say). The tool can't have knowledge of length of this header and where the magic value, version, and size related bytes data need to be placed.

Therefore the EQ design tool that today can't parse C headers needs an utility to report the generic header part. The above example "sof-ctl -n 15" output is not of any help in this task. We don't know what control # the EQ will be assigned and we don't usually have SOF on the computer when Matlab/Octave runs. Also we should not depend on ssh connection to a device when some simple EQ shape like generic 50 Hz high-pass is designed. If we have an up-to-date sof-ctl utility in the computer where the EQ tool run it is able to give the correct blob header. If the created blob fails the ABI check the tuning engineer should update the sof-ctl executable that carries this information from SOF build.

Do you see problems with this idea?

[lgirdwood]

@keyonjie this option intends to help external tools to prepend the correct ABI data for tuning.

[singalsu]

We discussed this this today this with @keyonjie. He proposed to rename new switch to "-g" for "generate header use case". It's the third offline type use case in addition to set/get data. Is this OK change @lgirdwood ?

[lgirdwood]

@singalsu @keyonjie yes, as long as switch is unique and you guys agree on use case.

[singalsu]

I've now renamed the switch to -g, the features are now all implemented.

[singalsu]

The mentioned issue need to be fixed with PR update.

[lgirdwood]

@keyonjie this option intends to help external tools to prepend the correct ABI data for tuning.

[singalsu]

I renamed this without [RFC] since the required features are now present.

[jajanusz]

I renamed this without [RFC] since the required features are now present.

So we can merge, right?

[keyonjie]

@singalsu almost good, please revisit the error handling in the main() function, as we are changing the block sequence(open out_fd firstly now), we need to change the sequence of those labels also.

[keyonjie]

@singalsu since this is common for both binary and CSV txt, can we move this to read_done, then we can save one in the "if (ctl_data->binary) {}" case?

[singalsu]

Good find, thanks!

[keyonjie]

goto struct_free;

[singalsu]

Yep, thanks for finding this!

[keyonjie]

goto out_fd_close;

[keyonjie]

The sequence here is changed, so we need revisit them for each item below, e.g.

	/* The control need to be defined. */
	if (!ctl_data->cname) {
		fprintf(stderr, "Error: No control was requested.\n");
		usage(argv[0]);
--		goto struct_free;
++		goto out_fd_close;
	}

	/* open input file */
	if (input_file) {
		ctl_data->in_fd = open(input_file, O_RDONLY);
		if (ctl_data->in_fd <= 0) {
			fprintf(stderr, "error: %s\n", strerror(errno));
--			goto struct_free;
++		        goto out_fd_close;
		}
	}

	/* set up ctl_elem, allocate buffers */
	ret = ctl_setup(ctl_data);
	if (ret < 0) {
		fprintf(stderr, "Error: ctl_data setup failed, ret:%d", ret);
--		goto out_fd_close;
++		goto in_fd_close;
	}

and change at the end of the function like this:

data_free:
	ret = ctl_free(ctl_data);
in_fd_close:
	if (ctl_data->in_fd)
		close(ctl_data->in_fd);
out_fd_close:
	if (ctl_data->out_fd)
		close(ctl_data->out_fd);
struct_free:
	free(ctl_data);

[singalsu]

Thanks, I got it!

[lgirdwood]

CI failures unrelated to this PR, the usual suspect.