Skip to content

[RFC] tools: add python blob generator#2536

Closed
juimonen wants to merge 1 commit intothesofproject:masterfrom
juimonen:blobgen
Closed

[RFC] tools: add python blob generator#2536
juimonen wants to merge 1 commit intothesofproject:masterfrom
juimonen:blobgen

Conversation

@juimonen
Copy link

Provide a general tool for creating binary blobs for sof process
components.

Signed-off-by: Jaska Uimonen jaska.uimonen@intel.com

@juimonen
Copy link
Author

just something as my python skills are not that good... based on python's "struct" module.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a cmd line tool that prepends ABI for coefficient blobs. I want to kill the mux/demux blobs, it should be using regular ALSA enum controls.

@juimonen
Copy link
Author

@lgirdwood, so this tool is generating the whole blob for you (not just abi header). So it is not for fir/iir where you need to use complex math design tools for getting the filter coeffs, unless you are very good doing math in your head :). It is more for those components where you build the blob from multiple structs having quite simple parameters in the end.

I'm not sure the mux/demux we have is usable with enums only. It is more complicated than mux/demux envisaged in asoc.

Anyway, @Keyon and @mmaka1 are suggesting to move totally to blobs in processing components:
https://github.com/orgs/thesofproject/teams/sof-developers/discussions/32, we would need someone else than me commenting on that.

Also, having this tool around would not close any options, but you could actually create the mux/demux blob (which was never possible before in public).

@lgirdwood
Copy link
Member

@juimonen I want rid of any blob that is used for runtime config unless it's loading coefficients

@plbossart
Copy link
Member

Even for coefficients we should have a header and a syntax that identifies blob type/versions/etc. There's nothing worse than a random set of bytes, it's unmanageable.

@juimonen
Copy link
Author

@plbossart so all the blobs we are sending have sof_abi_hdr with component specific data after it. That has abi versions etc. If you are talking about for example fir/iir specific structs, they are in sof includes and removed from kernel uapi. So if you'd like to use those in user space software, you need to somehow include sof headers.

Anyway I made this tool to help someone to create a blob, as it is requested from me all the time and as there is currently nothing more public than @singalsuo eq matlab scripts, and they are specific to eq.

I'm not trying to promote use of blobs here, I will try to modify the demux out of blob (and var array), the intent of you and @lgirdwood is now pretty clear to me. I've been just little bit hesitant to touch the demux on fw side as it is not written by me. But it seems nothing happens unless I try to do it myself :) people are busy...

@lgirdwood
Copy link
Member

I've been just little bit hesitant to touch the demux on fw side as it is not written by me. But it seems nothing happens unless I try to do it myself :) people are busy...

Please feel free to touch anything you like on FW side. :-)

@xiulipan
Copy link
Contributor

xiulipan commented Apr 8, 2020

@juimonen Would you mind add one more feature here to have another option to create a simple snd_ctl_tlv wrapper outside sof headers?

https://github.com/alsa-project/alsa-lib/blob/fb48ad9e4f6b84fd4ade689bd79e3a3c37d3e034/include/sound/uapi/asound.h#L1057-L1061

struct snd_ctl_tlv {
	unsigned int numid;	/* control element numeric identification */
	unsigned int length;	/* in bytes aligned to 4 */
	unsigned int tlv[0];	/* first TLV */
};

This will help to make the blob directly usable with alsaucm with cset-tlv

@juimonen
Copy link
Author

juimonen commented Apr 8, 2020

@xiulipan sure will do!

@juimonen
Copy link
Author

juimonen commented Apr 8, 2020

@xiulipan so you want this to replace the sof header, or as extra around sof header + blob?

@juimonen
Copy link
Author

juimonen commented Apr 8, 2020

@xiulipan and btw where is this cset-tlv? Can't seem to find it? is it some chromium specific thing?

@xiulipan
Copy link
Contributor

xiulipan commented Apr 9, 2020

@juimonen From my test using byte blob with UCM, I found the binary accepted by SOF should be looks like

snd_ctl_tlv Header
sof blob Header
blob binary

So I would like to have another option to append a snd_ctl_tlv header.

The cset-tlv is something from alsaucm.
https://github.com/alsa-project/alsa-lib/blob/fb48ad9e4f6b84fd4ade689bd79e3a3c37d3e034/src/ucm/parser.c#L537-L545

		if (strcmp(cmd, "cset-tlv") == 0) {
			curr->type = SEQUENCE_ELEMENT_TYPE_CSET_TLV;
			err = parse_string(n, &curr->data.cset);
			if (err < 0) {
				uc_error("error: cset-tlv requires a string!");
				return err;
			}
			continue;
		}

Provide a general tool for creating binary blobs for sof process
components.

Signed-off-by: Jaska Uimonen <jaska.uimonen@intel.com>
Copy link

@monocodus monocodus bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is autogenerated code-style review, new suggestions: 16


items_per_line = 8

def conv_and_get_type(val):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def conv_and_get_type(val):
def conv_and_get_type(val):

This comment was generated by: AutoPEP8

except ValueError:
pass

parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.', epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.', epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)

This comment was generated by: AutoPEP8

pass

parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.', epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
parser.add_argument("--abiver", "-a", type=int, nargs=3, help='sof abi version major minor patch')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parser.add_argument("--abiver", "-a", type=int, nargs=3, help='sof abi version major minor patch')
parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.',

This comment was generated by: AutoPEP8


parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.', epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
parser.add_argument("--abiver", "-a", type=int, nargs=3, help='sof abi version major minor patch')
parser.add_argument('--controlid', "-c", type=int, nargs=1, help='define controlid and wrap the blob around snd_ctl_tlv header')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parser.add_argument('--controlid', "-c", type=int, nargs=1, help='define controlid and wrap the blob around snd_ctl_tlv header')
epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)

This comment was generated by: AutoPEP8

parser = argparse.ArgumentParser(description='Print C struct as hex blob for alsa conf.', epilog=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
parser.add_argument("--abiver", "-a", type=int, nargs=3, help='sof abi version major minor patch')
parser.add_argument('--controlid', "-c", type=int, nargs=1, help='define controlid and wrap the blob around snd_ctl_tlv header')
parser.add_argument("--members", "-m", nargs='+', help='char values specifying the types of the struct members')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
parser.add_argument("--members", "-m", nargs='+', help='char values specifying the types of the struct members')
parser.add_argument("--abiver", "-a", type=int, nargs=3,
help='sof abi version major minor patch')
parser.add_argument('--controlid', "-c", type=int, nargs=1,
help='define controlid and wrap the blob around snd_ctl_tlv header')
parser.add_argument("--members", "-m", nargs='+',
help='char values specifying the types of the struct members')

This comment was generated by: AutoPEP8

if args.controlid:
topo_hdr_extra = struct.pack(topo_ctl_members, args.controlid[0], size_extra)

#pack header
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#pack header
# pack header

This comment was generated by: AutoPEP8

#pack header
topo_hdr = struct.pack(topo_hdr_members, magic, comp_type, size, abi, reserved)

#pack payload
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#pack payload
# pack payload

This comment was generated by: AutoPEP8

#pack payload
topo_data = struct.pack(topo_data_members, *(values_t))

#full blob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#full blob
# full blob

This comment was generated by: AutoPEP8

else:
topo_full = topo_hdr + topo_data

#for sof topology m4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#for sof topology m4
# for sof topology m4

This comment was generated by: AutoPEP8


print("'\n")

#for sof ctl tool
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#for sof ctl tool
# for sof ctl tool

This comment was generated by: AutoPEP8

@juimonen
Copy link
Author

juimonen commented Apr 9, 2020

@xiuli new version updated. with specifying argument -c "control_id", the tool should create snd_ctl_tlv around the blob.

Copy link
Member

@lgirdwood lgirdwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the difference between this and the C version ?

@paulstelian97
Copy link
Collaborator

Please resubmit with "main" as PR base branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants