control: enable octal and hexadecimal 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:
    bcsetf "name='<KCONTROL_NAME>' <path/to/file>"

where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).

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

src/ucm/main.c +41 -3

index 37ae4c8..1496b22 100644
@@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      const char *filepath)
+{
+	int err = 0;
+	FILE *in;
+	long len;
+	char *res;
+	unsigned int idx;
+
+	in = fopen(filepath, "r");
+	if (!in) {
+		err = -errno;
+		goto __fail;
+	}
+	fseek(in, 0L, SEEK_END);
+	len = ftell(in);
+	rewind(in);
+	if (len > 512)
+		len = 512;
+	res = calloc(1, (size_t)len);
+	if (res == NULL) {
+		err = -ENOMEM;
+		goto __fail_nomem;
+	}
+	fread(res, (size_t)len, 1, in);
+	for (idx = 0; idx < len; idx++)
+		snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));
+	free(res);
+      __fail_nomem:
+	fclose(in);
+      __fail:
+	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, int isbin)
 {
 	const char *pos;
 	int err;

@@ -194,7 +228,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 (isbin)
+		err = binary_file_parse(value, 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 +276,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_BCSETF:
 			if (cdev == NULL) {
 				const char *cdev1 = NULL, *cdev2 = NULL;
 				err = get_value3(&cdev1, "PlaybackCTL",

@@ -274,7 +312,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->isbin);
 			if (err < 0) {
 				uc_error("unable to execute cset '%s'\n", s->data.cset);
 				goto __fail;

src/ucm/parser.c +11 -0

index d7517f6..686c883 100644
@@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 			continue;
 		}
 
+		if (strcmp(cmd, "bcsetf") == 0) {
+			curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;
+			curr->isbin = 1;
+			err = parse_string(n, &curr->data.cset);
+			if (err < 0) {
+				uc_error("error: bcsetf 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 +2 -0

index 87f14a2..80d7335 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_BCSETF	5
 
 struct ucm_value {
         struct list_head list;

@@ -63,6 +64,7 @@ struct sequence_element {
 		char *cset;
 		char *exec;
 	} data;
+	int isbin;          /* Indicate cset is binary array or ascii array */
 };
 
 /*

Signature · 1 line

2.1.0

At Tue, 13 Jan 2015 11:00:39 +0800,

Quoted 4 lines · [email protected] wrote:

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

with cset command, UCM set kcontrol parameters directly:

Quoted 8 lines

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

where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).

Why binary? It's not portable. You can't carry it to a different architecture.

Takashi

Quoted 128 lines

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

src/ucm/main.c +41 -3

index 37ae4c8..1496b22 100644
@@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      const char *filepath)
+{
+	int err = 0;
+	FILE *in;
+	long len;
+	char *res;
+	unsigned int idx;
+
+	in = fopen(filepath, "r");
+	if (!in) {
+		err = -errno;
+		goto __fail;
+	}
+	fseek(in, 0L, SEEK_END);
+	len = ftell(in);
+	rewind(in);
+	if (len > 512)
+		len = 512;
+	res = calloc(1, (size_t)len);
+	if (res == NULL) {
+		err = -ENOMEM;
+		goto __fail_nomem;
+	}
+	fread(res, (size_t)len, 1, in);
+	for (idx = 0; idx < len; idx++)
+		snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));
+	free(res);
+      __fail_nomem:
+	fclose(in);
+      __fail:
+	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, int isbin)
 {
 	const char *pos;
 	int err;

@@ -194,7 +228,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 (isbin)
+		err = binary_file_parse(value, 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 +276,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_BCSETF:
 			if (cdev == NULL) {
 				const char *cdev1 = NULL, *cdev2 = NULL;
 				err = get_value3(&cdev1, "PlaybackCTL",

@@ -274,7 +312,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->isbin);
 			if (err < 0) {
 				uc_error("unable to execute cset '%s'\n", s->data.cset);
 				goto __fail;

src/ucm/parser.c +11 -0

index d7517f6..686c883 100644
@@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 			continue;
 		}
 
+		if (strcmp(cmd, "bcsetf") == 0) {
+			curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;
+			curr->isbin = 1;
+			err = parse_string(n, &curr->data.cset);
+			if (err < 0) {
+				uc_error("error: bcsetf 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 +2 -0

index 87f14a2..80d7335 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_BCSETF	5
 
 struct ucm_value {
         struct list_head list;

@@ -63,6 +64,7 @@ struct sequence_element {
 		char *cset;
 		char *exec;
 	} data;
+	int isbin;          /* Indicate cset is binary array or ascii array */
 };
 
 /*

Signature · 1 line

2.1.0

Quoted 15 lines · Takashi Iwai

At Tue, 13 Jan 2015 11:00:39 +0800,

Quoted 12 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:
    bcsetf "name='<KCONTROL_NAME>' <path/to/file>"

where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).

Quoted 3 lines

Why binary? It's not portable. You can't carry it to a different architecture.

The intention here is that the binary data is not meant for the host but for audio DSPs so it's just passed by UCM/ALSA as raw data.

We do have some DSP processing algos that have tuning tools where the tool will generate a binary file of coefficients (depending on various physical device properties etc). These coefficient files can then be uploaded to the DSP by UCM on use case change.

Liam

Quoted 131 lines

Takashi

Quoted 128 lines

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

src/ucm/main.c +41 -3

index 37ae4c8..1496b22 100644
@@ -160,11 +160,45 @@ static int open_ctl(snd_use_case_mgr_t *uc_mgr,
 	return 0;
 }
 
+static int binary_file_parse(snd_ctl_elem_value_t *dst,
+			      const char *filepath)
+{
+	int err = 0;
+	FILE *in;
+	long len;
+	char *res;
+	unsigned int idx;
+
+	in = fopen(filepath, "r");
+	if (!in) {
+		err = -errno;
+		goto __fail;
+	}
+	fseek(in, 0L, SEEK_END);
+	len = ftell(in);
+	rewind(in);
+	if (len > 512)
+		len = 512;
+	res = calloc(1, (size_t)len);
+	if (res == NULL) {
+		err = -ENOMEM;
+		goto __fail_nomem;
+	}
+	fread(res, (size_t)len, 1, in);
+	for (idx = 0; idx < len; idx++)
+		snd_ctl_elem_value_set_byte(dst, idx, *(res + idx));
+	free(res);
+      __fail_nomem:
+	fclose(in);
+      __fail:
+	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, int isbin)
 {
 	const char *pos;
 	int err;

@@ -194,7 +228,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 (isbin)
+		err = binary_file_parse(value, 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 +276,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_BCSETF:
 			if (cdev == NULL) {
 				const char *cdev1 = NULL, *cdev2 = NULL;
 				err = get_value3(&cdev1, "PlaybackCTL",

@@ -274,7 +312,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->isbin);
 			if (err < 0) {
 				uc_error("unable to execute cset '%s'\n", s->data.cset);
 				goto __fail;

src/ucm/parser.c +11 -0

index d7517f6..686c883 100644
@@ -306,6 +306,17 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 			continue;
 		}
 
+		if (strcmp(cmd, "bcsetf") == 0) {
+			curr->type = SEQUENCE_ELEMENT_TYPE_BCSETF;
+			curr->isbin = 1;
+			err = parse_string(n, &curr->data.cset);
+			if (err < 0) {
+				uc_error("error: bcsetf 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 +2 -0

index 87f14a2..80d7335 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_BCSETF	5
 
 struct ucm_value {
         struct list_head list;

@@ -63,6 +64,7 @@ struct sequence_element {
 		char *cset;
 		char *exec;
 	} data;
+	int isbin;          /* Indicate cset is binary array or ascii array */
 };
 
 /*

Signature · 1 line

2.1.0

At Tue, 13 Jan 2015 18:26:18 +0000,

Quoted 17 lines · Liam Girdwood

Quoted 15 lines · Takashi Iwai

At Tue, 13 Jan 2015 11:00:39 +0800,

Quoted 12 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:
    bcsetf "name='<KCONTROL_NAME>' <path/to/file>"

where "bcsetf" is a newly added keyword alongside of "cset", to indicate binary cset with file; and <path/to/file> is the configure file storing parameters in bytes array, up to 512 Bytes (the maxim value that struct snd_ctl_elem_value can hold).

Quoted 6 lines

Quoted 3 lines

Why binary? It's not portable. You can't carry it to a different architecture.

The intention here is that the binary data is not meant for the host but for audio DSPs so it's just passed by UCM/ALSA as raw data.

In that case, we should limit to certain element data types. Otherwise people would abuse it for passing data even to integer or enum ctls.

And of course it'd be better to clarify the reason in the patch description :)

BTW, I'm still not so convinced by bcsetf... Can't it be more verbose or readable?

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

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

src/control/ctlparse.c +4 -4

index 978977d..acaf734 100644
@@ -59,10 +59,10 @@ static long get_integer(const char **ptr, long min, long max)
 		goto out;
 
 	s = p;
-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);
 	}
 	if (*p == '%') {
 		val = (long)convert_prange1(strtod(s, NULL), min, max);

@@ -87,10 +87,10 @@ static long long get_integer64(const char **ptr, long long min, long long max)
 		goto out;
 
 	s = p;
-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);
 	}
 	if (*p == '%') {
 		val = (long long)convert_prange1(strtod(s, NULL), min, max);

Signature · 1 line

2.1.0

At Tue, 13 Jan 2015 11:00:38 +0800,

Quoted 4 lines · [email protected] wrote:

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

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

Looks good to me. Liam, any objection for this extension?

thanks,

Takashi

Quoted 34 lines

src/control/ctlparse.c +4 -4

index 978977d..acaf734 100644
@@ -59,10 +59,10 @@ static long get_integer(const char **ptr, long min, long max)
 		goto out;
 
 	s = p;
-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);
 	}
 	if (*p == '%') {
 		val = (long)convert_prange1(strtod(s, NULL), min, max);

@@ -87,10 +87,10 @@ static long long get_integer64(const char **ptr, long long min, long long max)
 		goto out;
 
 	s = p;
-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);
 	}
 	if (*p == '%') {
 		val = (long long)convert_prange1(strtod(s, NULL), min, max);

Signature · 1 line

2.1.0

At Tue, 13 Jan 2015 11:00:38 +0800,

Quoted 4 lines · [email protected] wrote:

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

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

Looks good to me. Liam, any objection for this extension?

src/control/ctlparse.c static long get_integer(const char **ptr, long min, long max) · line 65

Quoted 6 lines

src/control/ctlparse.c +2 -2

-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Wednesday, January 14, 2015 4:02 AM
To: Takashi Iwai
Cc: Lu, Han; Liam Girdwood; [email protected]
Subject: Re: [PATCH - UCM 1/2] control: enable octal and hexadecimal parse

At Tue, 13 Jan 2015 17:53:14 +0100,

Quoted 9 lines · Takashi Iwai

At Tue, 13 Jan 2015 11:00:38 +0800,

Quoted 4 lines · [email protected] wrote:

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

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

Looks good to me. Liam, any objection for this extension?

Erm, sorry, I correct my statement: this is buggy. Look at the code:

Quoted 6 lines

src/control/ctlparse.c static long get_integer(const char **ptr, long min, long max) · line 65

Quoted 6 lines

src/control/ctlparse.c +2 -2

-	val = strtol(s, &p, 10);
+	val = strtol(s, &p, 0);
 	if (*p == '.') {
 		p++;
-		strtol(p, &p, 10);
+		strtol(p, &p, 0);

The second strtol() is for skipping the decimals. So this has to be 10-based. That is, use zero-base only for the first strtol().

Takashi