ucm: add binary configure file parse

From: "Lu, Han" <[email protected]>

with cset command, UCM set kcontrol parameters directly:

    cset "name='<KCONTROL_NAME>' 1<,2,3,...>"
This patch enables UCM to set kcontrol with parameters from
configure file:
    cset-bin-file "name='<KCONTROL_NAME>' <path/to/file>"

where "cset-bin-file" is a newly added keyword alongside of "cset", to indicate cset with binary data in file.
The binary data in file is parameter for audio DSPs, and it's just passed by UCM/ALSA as raw data. The data type of the parameter element must be byte, and the count is limited by the size of struct snd_ctl_elem_value and driver definition.

Signed-off-by Lu, Han <[email protected]>

src/ucm/main.c +20 -0

index 37ae4c8..818465a 100644
@@ -160,11 +160,62 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      snd_ctl_elem_info_t *info,
+			      const char *filepath)
+{
+	int err, fd;
+	struct stat st;
+	size_t sz;
+	char *res;
+	snd_ctl_elem_type_t type;
+	unsigned int idx, count;
+
+	type = snd_ctl_elem_info_get_type(info);
+	if (type != SND_CTL_ELEM_TYPE_BYTES) {
+		uc_error("only support byte type!");
+		err = -EINVAL;
+		return err;
+	}
+	fd = open(filepath, O_RDONLY);
+	if (fd < 0) {
+		err = -errno;

src/ucm/main.c +24 -3

+	if (res == NULL) {
+		err = -ENOMEM;
+		goto __fail;
+	}
+	if (read(fd, res, sz) != sz) {
+		err = -errno;
+		goto __fail_read;
+	}
+	for (idx = 0; idx < sz; idx++)
+		snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));
+      __fail_read:
+	free(res);
+      __fail:
+	close(fd);
+	return err;
+}
+
 extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst,
 					 const char *str,
 					 const char **ret_ptr);
 
-static int execute_cset(snd_ctl_t *ctl, const char *cset)
+static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type)
 {
 	const char *pos;
 	int err;

@@ -194,7 +245,10 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset)
 	err = snd_ctl_elem_info(ctl, info);
 	if (err < 0)
 		goto __fail;
-	err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
+	if (type == SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE)
+		err = binary_file_parse(value, info, pos);
+	else
+		err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
 	if (err < 0)
 		goto __fail;
 	err = snd_ctl_elem_write(ctl, value);

@@ -239,6 +293,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 				goto __fail_nomem;
 			break;
 		case SEQUENCE_ELEMENT_TYPE_CSET:
+		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
 			if (cdev == NULL) {
 				const char *cdev1 = NULL, *cdev2 = NULL;
 				err = get_value3(&cdev1, "PlaybackCTL",

@@ -274,7 +329,7 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 					goto __fail;
 				}
 			}
-			err = execute_cset(ctl, s->data.cset);
+			err = execute_cset(ctl, s->data.cset, s->type);
 			if (err < 0) {
 				uc_error("unable to execute cset '%s'\n", s->data.cset);
 				goto __fail;

src/ucm/parser.c +10 -0

index d7517f6..9e1cb41 100644
@@ -306,6 +306,16 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 			continue;
 		}
 
+		if (strcmp(cmd, "cset-bin-file") == 0) {
+			curr->type = SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE;
+			err = parse_string(n, &curr->data.cset);
+			if (err < 0) {
+				uc_error("error: cset-bin-file requires a string!");
+				return err;
+			}
+			continue;
+		}
+
 		if (strcmp(cmd, "usleep") == 0) {
 			curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
 			err = snd_config_get_integer(n, &curr->data.sleep);

src/ucm/ucm_local.h +1 -0

index 87f14a2..c1655c7 100644
@@ -47,6 +47,7 @@
 #define SEQUENCE_ELEMENT_TYPE_CSET	2
 #define SEQUENCE_ELEMENT_TYPE_SLEEP	3
 #define SEQUENCE_ELEMENT_TYPE_EXEC	4
+#define SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE	5
 
 struct ucm_value {
         struct list_head list;

Signature · 1 line

2.1.0

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Tuesday, January 20, 2015 5:19 PM
To: Lu, Han
Cc: Girdwood, Liam R; [email protected]
Subject: Re: [PATCH - UCM 1/1] ucm: add binary configure file parse

At Tue, 20 Jan 2015 16:33:09 +0800,

Quoted 51 lines · [email protected] wrote:

From: "Lu, Han" <[email protected]>

with cset command, UCM set kcontrol parameters directly:

    cset "name='<KCONTROL_NAME>' 1<,2,3,...>"
This patch enables UCM to set kcontrol with parameters from configure
file:
    cset-bin-file "name='<KCONTROL_NAME>' <path/to/file>"

where "cset-bin-file" is a newly added keyword alongside of "cset", to indicate cset with binary data in file.
The binary data in file is parameter for audio DSPs, and it's just passed by UCM/ALSA as raw data. The data type of the parameter element must be byte, and the count is limited by the size of struct snd_ctl_elem_value and driver definition.

Signed-off-by Lu, Han <[email protected]>

src/ucm/main.c index 37ae4c8..818465a renamed from src/ucm/main.c +0 -0

100644

src/ucm/main.c +26 -0

@@ -160,11 +160,62 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      snd_ctl_elem_info_t *info,
+			      const char *filepath)
+{
+	int err, fd;
+	struct stat st;
+	size_t sz;
+	char *res;
+	snd_ctl_elem_type_t type;
+	unsigned int idx, count;
+
+	type = snd_ctl_elem_info_get_type(info);
+	if (type != SND_CTL_ELEM_TYPE_BYTES) {
+		uc_error("only support byte type!");
+		err = -EINVAL;
+		return err;
+	}
+	fd = open(filepath, O_RDONLY);
+	if (fd < 0) {
+		err = -errno;
+		return err;
+	}
+	if (stat(filepath, &st) == -1) {
+		err = -errno;
+		goto __fail;
+	}

src/ucm/main.c static int open_ctl(snd_use_case_mgr_t *uc_mgr, · line 192

Quoted 4 lines

src/ucm/main.c +4 -0

+	sz = st.st_size;
+	count = snd_ctl_elem_info_get_count(info);
+	if (sz == 0 || sz > count || sz > sizeof(dst->value.bytes)) {
+		uc_error("invalid parameter size!");

A question here is what if the file size is smaller than the expected data size. Should we allow this explicitly? I thought it'd be safer to bail out, but I'd like to know more about the scenarios behind this patch.

src/ucm/main.c static int open_ctl(snd_use_case_mgr_t *uc_mgr, · line 196

Quoted 4 lines

src/ucm/main.c +4 -0

+		err = -EINVAL;
+		goto __fail;
+	}
+	res = calloc(sz, 1);

This can be malloc() as we'll overwrite the whole data in anyway.

thanks,

Takashi

src/ucm/main.c static int execute_cset(snd_ctl_t *ctl, const char

Quoted 28 lines

src/ucm/main.c +19 -1

+	if (res == NULL) {
+		err = -ENOMEM;
+		goto __fail;
+	}
+	if (read(fd, res, sz) != sz) {
+		err = -errno;
+		goto __fail_read;
+	}
+	for (idx = 0; idx < sz; idx++)
+		snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));
+      __fail_read:
+	free(res);
+      __fail:
+	close(fd);
+	return err;
+}
+
 extern int __snd_ctl_ascii_elem_id_parse(snd_ctl_elem_id_t *dst,
 					 const char *str,
 					 const char **ret_ptr);
 
-static int execute_cset(snd_ctl_t *ctl, const char *cset)
+static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned
+int type)
 {
 	const char *pos;
 	int err;

@@ -194,7 +245,10 @@ static int execute_cset(snd_ctl_t *ctl, const char

*cset)

Quoted 12 lines

src/ucm/main.c +4 -1

 	err = snd_ctl_elem_info(ctl, info);
 	if (err < 0)
 		goto __fail;
-	err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
+	if (type == SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE)
+		err = binary_file_parse(value, info, pos);
+	else
+		err = snd_ctl_ascii_value_parse(ctl, value, info, pos);
 	if (err < 0)
 		goto __fail;
 	err = snd_ctl_elem_write(ctl, value); @@ -239,6 +293,7 @@ static int

execute_sequence(snd_use_case_mgr_t *uc_mgr,

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr,

Quoted 7 lines

src/ucm/main.c +1 -0

 				goto __fail_nomem;
 			break;
 		case SEQUENCE_ELEMENT_TYPE_CSET:
+		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
 			if (cdev == NULL) {
 				const char *cdev1 = NULL, *cdev2 = NULL;
 				err = get_value3(&cdev1, "PlaybackCTL", @@

-274,7 +329,7 @@

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t

Quoted 15 lines

static int execute_sequence(snd_use_case_mgr_t *uc_mgr,

src/ucm/main.c +1 -1

 					goto __fail;
 				}
 			}
-			err = execute_cset(ctl, s->data.cset);
+			err = execute_cset(ctl, s->data.cset, s->type);
 			if (err < 0) {
 				uc_error("unable to execute cset '%s'\n", s-

data.cset);

src/ucm/main.c +0 -0

 				goto __fail;

src/ucm/parser.c index renamed from src/ucm/parser.c +0 -0

d7517f6..9e1cb41 100644

src/ucm/parser.c +0 -0

@@ -306,6 +306,16 @@ static int parse_sequence(snd_use_case_mgr_t

*uc_mgr ATTRIBUTE_UNUSED,

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,

Quoted 5 lines

src/ucm/parser.c +2 -0

 			continue;
 		}
 
+		if (strcmp(cmd, "cset-bin-file") == 0) {
+			curr->type =

SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE;

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,

Quoted 3 lines

src/ucm/parser.c +3 -0

+			err = parse_string(n, &curr->data.cset);
+			if (err < 0) {
+				uc_error("error: cset-bin-file requires a

string!");

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, · line 321

Quoted 8 lines

src/ucm/parser.c +5 -0

+				return err;
+			}
+			continue;
+		}
+
 		if (strcmp(cmd, "usleep") == 0) {
 			curr->type = SEQUENCE_ELEMENT_TYPE_SLEEP;
 			err = snd_config_get_integer(n, &curr->data.sleep);

diff --git

Quoted 15 lines

a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h index 87f14a2..c1655c7 100644

src/ucm/ucm_local.h +1 -1

@@ -47,6 +47,7 @@
 #define SEQUENCE_ELEMENT_TYPE_CSET	2
 #define SEQUENCE_ELEMENT_TYPE_SLEEP	3
 #define SEQUENCE_ELEMENT_TYPE_EXEC	4
+#define SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE	5
 
 struct ucm_value {
         struct list_head list;
--

2.1.0

Hi Takashi,

BR,
Han Lu

Quoted 60 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Tuesday, January 20, 2015 5:19 PM
To: Lu, Han
Cc: Girdwood, Liam R; [email protected]
Subject: Re: [PATCH - UCM 1/1] ucm: add binary configure file parse

At Tue, 20 Jan 2015 16:33:09 +0800,

Quoted 51 lines · [email protected] wrote:

From: "Lu, Han" <[email protected]>

with cset command, UCM set kcontrol parameters directly:

    cset "name='<KCONTROL_NAME>' 1<,2,3,...>"
This patch enables UCM to set kcontrol with parameters from configure
file:
    cset-bin-file "name='<KCONTROL_NAME>' <path/to/file>"

where "cset-bin-file" is a newly added keyword alongside of "cset", to indicate cset with binary data in file.
The binary data in file is parameter for audio DSPs, and it's just passed by UCM/ALSA as raw data. The data type of the parameter element must be byte, and the count is limited by the size of struct snd_ctl_elem_value and driver definition.

Signed-off-by Lu, Han <[email protected]>

src/ucm/main.c index 37ae4c8..818465a renamed from src/ucm/main.c +0 -0

100644

src/ucm/main.c +26 -0

@@ -160,11 +160,62 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      snd_ctl_elem_info_t *info,
+			      const char *filepath)
+{
+	int err, fd;
+	struct stat st;
+	size_t sz;
+	char *res;
+	snd_ctl_elem_type_t type;
+	unsigned int idx, count;
+
+	type = snd_ctl_elem_info_get_type(info);
+	if (type != SND_CTL_ELEM_TYPE_BYTES) {
+		uc_error("only support byte type!");
+		err = -EINVAL;
+		return err;
+	}
+	fd = open(filepath, O_RDONLY);
+	if (fd < 0) {
+		err = -errno;
+		return err;
+	}
+	if (stat(filepath, &st) == -1) {
+		err = -errno;
+		goto __fail;
+	}

Quoted 9 lines

src/ucm/main.c static int open_ctl(snd_use_case_mgr_t *uc_mgr, · line 192

Quoted 4 lines

src/ucm/main.c +4 -0

+	sz = st.st_size;
+	count = snd_ctl_elem_info_get_count(info);
+	if (sz == 0 || sz > count || sz > sizeof(dst->value.bytes)) {
+		uc_error("invalid parameter size!");

A question here is what if the file size is smaller than the expected data size. Should we allow this explicitly? I thought it'd be safer to bail out, but I'd like to know more about the scenarios behind this patch.

The scenario here is to load multiple (~160) binary configure files, but their length may be variable from about 10 to 512 bytes. (most of them is about 10 to 100 bytes)
In this case, the count defined in driver have to be 512. If I return when size != 512, all configure files must be extended to 512 bytes, it will take about 80 Kbytes. So I used 512 as a range. But Liam and I agreed that it will be safer to bail out in this case. I will change the patch. Thanks.