[PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[Shengjiu Wang]

When route_policy is average, src format is S24_3LE, the get_idx will exceed
length of table gets_label. So add items for S24_3LE/S20_3LE/S18_3LE in the
table.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
 src/pcm/plugin_ops.h |   27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h
index 21535c9..1213b69 100644
--- a/src/pcm/plugin_ops.h
+++ b/src/pcm/plugin_ops.h
@@ -670,7 +670,7 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;
 
 #ifdef GETS_LABELS
 /* width endswap sign_toggle */
-static void *const gets_labels[4 * 2 * 2] = {
+static void *const gets_labels[4 * 2 * 2 + 4 * 3] = {
 	&&gets_1_1,		/*  8h ->  8h */
 	&&gets_1_9,		/*  8h ^>  8h */
 	&&gets_1_1,		/*  8s ->  8h */
@@ -687,6 +687,19 @@ static void *const gets_labels[4 * 2 * 2] = {
 	&&gets_1234_9234,	/* 32h ^> 32h */
 	&&gets_1234_4321,	/* 32s -> 32h */
 	&&gets_1234_C321,	/* 32s ^> 32h */
+	/* 3bytes format */
+	&&gets_123_0123,	/* 24h -> 24h */
+	&&gets_123_0923,	/* 24h ^> 24h */
+	&&gets_123_0321,	/* 24s -> 24h */
+	&&gets_123_0B21,	/* 24s ^> 24h */
+	&&gets_123_0123_20,	/* 20h -> 24h */
+	&&gets_123_0923_20,	/* 20h ^> 24h */
+	&&gets_123_0321_20,	/* 20s -> 24h */
+	&&gets_123_0B21_20,	/* 20s ^> 24h */
+	&&gets_123_0123_18,	/* 18h -> 24h */
+	&&gets_123_0923_18,	/* 18h ^> 24h */
+	&&gets_123_0321_18,	/* 18s -> 24h */
+	&&gets_123_0B21_18,	/* 18s ^> 24h */
 };
 #endif
 
@@ -706,6 +719,18 @@ gets_1234_1234: sample = as_s32c(src); goto GETS_END;
 gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
 gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
 gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END;
+gets_123_0123: sample = sx24(_get_triple(src)); goto GETS_END;
+gets_123_0923: sample = sx24(_get_triple(src) ^ 0x800000); goto GETS_END;
+gets_123_0321: sample = sx24(_get_triple_s(src)); goto GETS_END;
+gets_123_0B21: sample = sx24(_get_triple_s(src) ^ 0x800000); goto GETS_END;
+gets_123_0123_20: sample = sx24(_get_triple(src) << 4); goto GETS_END;
+gets_123_0923_20: sample = sx24((_get_triple(src) << 4) ^ 0x800000); goto GETS_END;
+gets_123_0321_20: sample = sx24(_get_triple_s(src) << 4); goto GETS_END;
+gets_123_0B21_20: sample = sx24((_get_triple_s(src) << 4) ^ 0x800000); goto GETS_END;
+gets_123_0123_18: sample = sx24(_get_triple(src) << 6); goto GETS_END;
+gets_123_0923_18: sample = sx24((_get_triple(src) << 6) ^ 0x800000); goto GETS_END;
+gets_123_0321_18: sample = sx24(_get_triple_s(src) << 6); goto GETS_END;
+gets_123_0B21_18: sample = sx24((_get_triple_s(src) << 6) ^ 0x800000); goto GETS_END;
 }
 #endif
 
-- 
1.7.9.5

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[Takashi Iwai]

At Tue, 22 Jul 2014 10:30:41 +0800,
Shengjiu Wang wrote:
> 
> When route_policy is average, src format is S24_3LE, the get_idx will exceed
> length of table gets_label. So add items for S24_3LE/S20_3LE/S18_3LE in the
> table.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> ---
>  src/pcm/plugin_ops.h |   27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h
> index 21535c9..1213b69 100644
> --- a/src/pcm/plugin_ops.h
> +++ b/src/pcm/plugin_ops.h
> @@ -670,7 +670,7 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;
>  
>  #ifdef GETS_LABELS
>  /* width endswap sign_toggle */
> -static void *const gets_labels[4 * 2 * 2] = {
> +static void *const gets_labels[4 * 2 * 2 + 4 * 3] = {
>  	&&gets_1_1,		/*  8h ->  8h */
>  	&&gets_1_9,		/*  8h ^>  8h */
>  	&&gets_1_1,		/*  8s ->  8h */
> @@ -687,6 +687,19 @@ static void *const gets_labels[4 * 2 * 2] = {
>  	&&gets_1234_9234,	/* 32h ^> 32h */
>  	&&gets_1234_4321,	/* 32s -> 32h */
>  	&&gets_1234_C321,	/* 32s ^> 32h */
> +	/* 3bytes format */
> +	&&gets_123_0123,	/* 24h -> 24h */
> +	&&gets_123_0923,	/* 24h ^> 24h */
> +	&&gets_123_0321,	/* 24s -> 24h */
> +	&&gets_123_0B21,	/* 24s ^> 24h */
> +	&&gets_123_0123_20,	/* 20h -> 24h */
> +	&&gets_123_0923_20,	/* 20h ^> 24h */
> +	&&gets_123_0321_20,	/* 20s -> 24h */
> +	&&gets_123_0B21_20,	/* 20s ^> 24h */
> +	&&gets_123_0123_18,	/* 18h -> 24h */
> +	&&gets_123_0923_18,	/* 18h ^> 24h */
> +	&&gets_123_0321_18,	/* 18s -> 24h */
> +	&&gets_123_0B21_18,	/* 18s ^> 24h */
>  };
>  #endif
>  
> @@ -706,6 +719,18 @@ gets_1234_1234: sample = as_s32c(src); goto GETS_END;
>  gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
>  gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
>  gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END;
> +gets_123_0123: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923: sample = sx24(_get_triple(src) ^ 0x800000); goto GETS_END;
> +gets_123_0321: sample = sx24(_get_triple_s(src)); goto GETS_END;
> +gets_123_0B21: sample = sx24(_get_triple_s(src) ^ 0x800000); goto GETS_END;
> +gets_123_0123_20: sample = sx24(_get_triple(src) << 4); goto GETS_END;
> +gets_123_0923_20: sample = sx24((_get_triple(src) << 4) ^ 0x800000); goto GETS_END;

gets_* are just to convert the endianess and signedness, thus you must
not shift bits.  I think the code should be like:

> +gets_123_0123_20: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923_20: sample = sx24(_get_triple(src) ^ 0x80000); goto GETS_END;

... and...


> +gets_123_0123_18: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923_18: sample = sx24(_get_triple(src) ^ 0x20000); goto GETS_END;


thanks,

Takashi

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[shengjiu.wang]

Hi Iwai

   According to your comments, Should I define sx20 and sx18, because sx24 is for 24bit?

Best regards
Wang shengjiu

-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de] 
Sent: Tuesday, July 22, 2014 1:50 PM
To: Wang Shengjiu-B02247
Cc: perex@perex.cz; alsa-devel@alsa-project.org
Subject: Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

At Tue, 22 Jul 2014 10:30:41 +0800,
Shengjiu Wang wrote:
> 
> When route_policy is average, src format is S24_3LE, the get_idx will 
> exceed length of table gets_label. So add items for 
> S24_3LE/S20_3LE/S18_3LE in the table.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
> ---
>  src/pcm/plugin_ops.h |   27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 
> 21535c9..1213b69 100644
> --- a/src/pcm/plugin_ops.h
> +++ b/src/pcm/plugin_ops.h
> @@ -670,7 +670,7 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 
> 0x80); goto GETU_END;
>  
>  #ifdef GETS_LABELS
>  /* width endswap sign_toggle */
> -static void *const gets_labels[4 * 2 * 2] = {
> +static void *const gets_labels[4 * 2 * 2 + 4 * 3] = {
>  	&&gets_1_1,		/*  8h ->  8h */
>  	&&gets_1_9,		/*  8h ^>  8h */
>  	&&gets_1_1,		/*  8s ->  8h */
> @@ -687,6 +687,19 @@ static void *const gets_labels[4 * 2 * 2] = {
>  	&&gets_1234_9234,	/* 32h ^> 32h */
>  	&&gets_1234_4321,	/* 32s -> 32h */
>  	&&gets_1234_C321,	/* 32s ^> 32h */
> +	/* 3bytes format */
> +	&&gets_123_0123,	/* 24h -> 24h */
> +	&&gets_123_0923,	/* 24h ^> 24h */
> +	&&gets_123_0321,	/* 24s -> 24h */
> +	&&gets_123_0B21,	/* 24s ^> 24h */
> +	&&gets_123_0123_20,	/* 20h -> 24h */
> +	&&gets_123_0923_20,	/* 20h ^> 24h */
> +	&&gets_123_0321_20,	/* 20s -> 24h */
> +	&&gets_123_0B21_20,	/* 20s ^> 24h */
> +	&&gets_123_0123_18,	/* 18h -> 24h */
> +	&&gets_123_0923_18,	/* 18h ^> 24h */
> +	&&gets_123_0321_18,	/* 18s -> 24h */
> +	&&gets_123_0B21_18,	/* 18s ^> 24h */
>  };
>  #endif
>  
> @@ -706,6 +719,18 @@ gets_1234_1234: sample = as_s32c(src); goto 
> GETS_END;
>  gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto 
> GETS_END;
>  gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto 
> GETS_END;
>  gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto 
> GETS_END;
> +gets_123_0123: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923: sample = sx24(_get_triple(src) ^ 0x800000); goto 
> +GETS_END;
> +gets_123_0321: sample = sx24(_get_triple_s(src)); goto GETS_END;
> +gets_123_0B21: sample = sx24(_get_triple_s(src) ^ 0x800000); goto 
> +GETS_END;
> +gets_123_0123_20: sample = sx24(_get_triple(src) << 4); goto 
> +GETS_END;
> +gets_123_0923_20: sample = sx24((_get_triple(src) << 4) ^ 0x800000); 
> +goto GETS_END;

gets_* are just to convert the endianess and signedness, thus you must not shift bits.  I think the code should be like:

> +gets_123_0123_20: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923_20: sample = sx24(_get_triple(src) ^ 0x80000); goto 
> +GETS_END;

... and...


> +gets_123_0123_18: sample = sx24(_get_triple(src)); goto GETS_END;
> +gets_123_0923_18: sample = sx24(_get_triple(src) ^ 0x20000); goto 
> +GETS_END;


thanks,

Takashi

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[Takashi Iwai]

At Tue, 22 Jul 2014 07:59:03 +0000,
shengjiu.wang@freescale.com wrote:
> 
> Hi Iwai
> 
>    According to your comments, Should I define sx20 and sx18, because sx24 is for 24bit?

Right, they need the corresponding ones.

However, looking through the code again, it cannot work properly when
the format isn't 32bit and float isn't used.  It reads the value with
get label while it writes with put32 label.  The former reads the
value as is, but the latter assumes that the source value is
normalized to 32bit.

So, I guess the best way would be to use always 64bit int for route
multi-source case (unless float is available).  This fixes the bug and
simplifies a lot.  An untested patch is below.  Could you check
whether this works for you?


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] pcm: route: Use get32 for multi-source route calculation

The PCM route plugin can assign the destination value from average of
multiple sources with attenuation.  This requires the read of each
channel value, sums and writes the resultant value in the requested
format.

Currently, get_labels is used for reading source values while
put32_labels is used for writing the dest value.  This is, however,
a buggy implementation; get_labels gives the value as is only with
endianness and signedness conversions, but put32_labels assumes that
the value is normalized to 32bit int and it shifts down to the dest
format.  In addition, the current code lacks get_labels entries for
the 24bit formats, as Shengjiu Wang spotted out.

For fixing these bugs, this patch replaces the read with
get32_labels and use always 64bit int for sum.  This simplifies the
code a lot and drops many lines.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/pcm/pcm_route.c  | 62 +++++++++-------------------------------
 src/pcm/plugin_ops.h | 81 ----------------------------------------------------
 2 files changed, 14 insertions(+), 129 deletions(-)

diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c
index 72c198cb25d2..36a74ba661a9 100644
--- a/src/pcm/pcm_route.c
+++ b/src/pcm/pcm_route.c
@@ -60,7 +60,7 @@ typedef struct {
 typedef struct snd_pcm_route_ttable_dst snd_pcm_route_ttable_dst_t;
 
 typedef struct {
-	enum {UINT32=0, UINT64=1, FLOAT=2} sum_idx;
+	enum {UINT64, FLOAT} sum_idx;
 	unsigned int get_idx;
 	unsigned int put_idx;
 	unsigned int conv_idx;
@@ -233,35 +233,26 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 					const snd_pcm_route_ttable_dst_t* ttable,
 					const snd_pcm_route_params_t *params)
 {
-#define GETS_LABELS
+#define GET32_LABELS
 #define PUT32_LABELS
 #include "plugin_ops.h"
-#undef GETS_LABELS
+#undef GET32_LABELS
 #undef PUT32_LABELS
-	static void *const zero_labels[3] = {
-		&&zero_int32, &&zero_int64,
+	static void *const zero_labels[2] = {
+		&&zero_int64,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&zero_float
 #endif
 	};
 	/* sum_type att */
-	static void *const add_labels[3 * 2] = {
-		&&add_int32_noatt, &&add_int32_att,
+	static void *const add_labels[2 * 2] = {
 		&&add_int64_noatt, &&add_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&add_float_noatt, &&add_float_att
 #endif
 	};
 	/* sum_type att shift */
-	static void *const norm_labels[3 * 2 * 4] = {
-		0,
-		&&norm_int32_8_noatt,
-		&&norm_int32_16_noatt,
-		&&norm_int32_24_noatt,
-		0,
-		&&norm_int32_8_att,
-		&&norm_int32_16_att,
-		&&norm_int32_24_att,
+	static void *const norm_labels[2 * 2 * 4] = {
 		&&norm_int64_0_noatt,
 		&&norm_int64_8_noatt,
 		&&norm_int64_16_noatt,
@@ -281,7 +272,7 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		&&norm_float_24,
 #endif
 	};
-	void *zero, *get, *add, *norm, *put32;
+	void *zero, *get32, *add, *norm, *put32;
 	int nsrcs = ttable->nsrcs;
 	char *dst;
 	int dst_step;
@@ -323,7 +314,7 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 	}
 
 	zero = zero_labels[params->sum_idx];
-	get = gets_labels[params->get_idx];
+	get32 = get32_labels[params->get_idx];
 	add = add_labels[params->sum_idx * 2 + ttable->att];
 	norm = norm_labels[params->sum_idx * 8 + ttable->att * 4 + 4 - params->src_size];
 	put32 = put32_labels[params->put_idx];
@@ -336,9 +327,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 
 		/* Zero sum */
 		goto *zero;
-	zero_int32:
-		sum.as_sint32 = 0;
-		goto zero_end;
 	zero_int64: 
 		sum.as_sint64 = 0;
 		goto zero_end;
@@ -352,21 +340,14 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 			const char *src = srcs[srcidx];
 			
 			/* Get sample */
-			goto *get;
-#define GETS_END after_get
+			goto *get32;
+#define GET32_END after_get
 #include "plugin_ops.h"
-#undef GETS_END
+#undef GET32_END
 		after_get:
 
 			/* Sum */
 			goto *add;
-		add_int32_att:
-			sum.as_sint32 += sample * ttp->as_int;
-			goto after_sum;
-		add_int32_noatt:
-			if (ttp->as_int)
-				sum.as_sint32 += sample;
-			goto after_sum;
 		add_int64_att:
 			sum.as_sint64 += (int64_t) sample * ttp->as_int;
 			goto after_sum;
@@ -390,42 +371,30 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		
 		/* Normalization */
 		goto *norm;
-	norm_int32_8_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_8_att:
 		sum.as_sint64 <<= 8;
 	norm_int64_0_att:
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_16_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_16_att:
 		sum.as_sint64 <<= 16;
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_24_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_24_att:
 		sum.as_sint64 <<= 24;
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_8_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_8_noatt:
 		sum.as_sint64 <<= 8;
 		goto norm_int;
 
-	norm_int32_16_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_16_noatt:
 		sum.as_sint64 <<= 16;
 		goto norm_int;
 
-	norm_int32_24_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_24_noatt:
 		sum.as_sint64 <<= 24;
 		goto norm_int;
@@ -648,7 +617,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 	route->params.use_getput =
 		(snd_pcm_format_physical_width(src_format) + 7) / 3 == 3 ||
 		(snd_pcm_format_physical_width(dst_format) + 7) / 3 == 3;
-	route->params.get_idx = snd_pcm_linear_get_index(src_format, SND_PCM_FORMAT_S16);
+	route->params.get_idx = snd_pcm_linear_get32_index(src_format, SND_PCM_FORMAT_S32);
 	route->params.put_idx = snd_pcm_linear_put32_index(SND_PCM_FORMAT_S32, dst_format);
 	route->params.conv_idx = snd_pcm_linear_convert_index(src_format, dst_format);
 	route->params.src_size = snd_pcm_format_width(src_format) / 8;
@@ -656,10 +625,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 	route->params.sum_idx = FLOAT;
 #else
-	if (snd_pcm_format_width(src_format) == 32)
-		route->params.sum_idx = UINT64;
-	else
-		route->params.sum_idx = UINT32;
+	route->params.sum_idx = UINT64;
 #endif
 	return 0;
 }
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h
index 21535c9ca8d6..eb8c2c4f4dac 100644
--- a/src/pcm/plugin_ops.h
+++ b/src/pcm/plugin_ops.h
@@ -668,87 +668,6 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;
 }
 #endif
 
-#ifdef GETS_LABELS
-/* width endswap sign_toggle */
-static void *const gets_labels[4 * 2 * 2] = {
-	&&gets_1_1,		/*  8h ->  8h */
-	&&gets_1_9,		/*  8h ^>  8h */
-	&&gets_1_1,		/*  8s ->  8h */
-	&&gets_1_9,		/*  8s ^>  8h */
-	&&gets_12_12,		/* 16h -> 16h */
-	&&gets_12_92,		/* 16h ^> 16h */
-	&&gets_12_21,		/* 16s -> 16h */
-	&&gets_12_A1,		/* 16s ^> 16h */
-	&&gets_0123_0123,	/* 24h -> 24h */
-	&&gets_0123_0923,	/* 24h ^> 24h */
-	&&gets_1230_0321,	/* 24s -> 24h */
-	&&gets_1230_0B21,	/* 24s ^> 24h */
-	&&gets_1234_1234,	/* 32h -> 32h */
-	&&gets_1234_9234,	/* 32h ^> 32h */
-	&&gets_1234_4321,	/* 32s -> 32h */
-	&&gets_1234_C321,	/* 32s ^> 32h */
-};
-#endif
-
-#ifdef GETS_END
-while (0) {
-gets_1_1: sample = as_s8c(src); goto GETS_END;
-gets_1_9: sample = (int8_t)(as_s8c(src) ^ 0x80); goto GETS_END;
-gets_12_12: sample = as_s16c(src); goto GETS_END;
-gets_12_92: sample = (int16_t)(as_s16c(src) ^ 0x8000); goto GETS_END;
-gets_12_21: sample = (int16_t)bswap_16(as_s16c(src)); goto GETS_END;
-gets_12_A1: sample = (int16_t)bswap_16(as_s16c(src) ^ 0x80); goto GETS_END;
-gets_0123_0123: sample = sx24((int32_t)(as_s32c(src) << 8) >> 8); goto GETS_END;
-gets_0123_0923: sample = sx24((int32_t)((as_s32c(src) ^ 0x800000) << 8) >> 8); goto GETS_END;
-gets_1230_0321: sample = sx24((int32_t)(bswap_32(as_s32c(src)) << 8) >> 8); goto GETS_END;
-gets_1230_0B21: sample = sx24((int32_t)(bswap_32(as_s32c(src) ^ 0x8000) << 8) >> 8); goto GETS_END;
-gets_1234_1234: sample = as_s32c(src); goto GETS_END;
-gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
-gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
-gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END;
-}
-#endif
-
-#ifdef PUT_LABELS
-/* width endswap sign_toggle */
-static void *const put_labels[4 * 2 * 2] = {
-	&&put_1_1,		/*  8h ->  8h */
-	&&put_1_9,		/*  8h ^>  8h */
-	&&put_1_1,		/*  8h ->  8s */
-	&&put_1_9,		/*  8h ^>  8s */
-	&&put_12_12,		/* 16h -> 16h */
-	&&put_12_92,		/* 16h ^> 16h */
-	&&put_12_21,		/* 16h -> 16s */
-	&&put_12_29,		/* 16h ^> 16s */
-	&&put_0123_0123,	/* 24h -> 24h */
-	&&put_0123_0923,	/* 24h ^> 24h */
-	&&put_0123_3210,	/* 24h -> 24s */
-	&&put_0123_3290,	/* 24h ^> 24s */
-	&&put_1234_1234,	/* 32h -> 32h */
-	&&put_1234_9234,	/* 32h ^> 32h */
-	&&put_1234_4321,	/* 32h -> 32s */
-	&&put_1234_4329,	/* 32h ^> 32s */
-};
-#endif
-
-#ifdef PUT_END
-put_1_1: as_s8(dst) = sample; goto PUT_END;
-put_1_9: as_u8(dst) = sample ^ 0x80; goto PUT_END;
-put_12_12: as_s16(dst) = sample; goto PUT_END;
-put_12_92: as_u16(dst) = sample ^ 0x8000; goto PUT_END;
-put_12_21: as_s16(dst) = bswap_16(sample); goto PUT_END;
-put_12_29: as_u16(dst) = bswap_16(sample) ^ 0x80; goto PUT_END;
-/* this always writes the unused byte in 24-bit formats as 0x00 */
-put_0123_0123: as_s32(dst) = sx24(sample & 0x00ffffff); goto PUT_END;
-put_0123_0923: as_u32(dst) = sx24((sample & 0x00ffffff) ^ 0x800000); goto PUT_END;
-put_0123_3210: as_s32(dst) = sx24s(bswap_32(sample) & 0xffffff00); goto PUT_END;
-put_0123_3290: as_u32(dst) = sx24s((bswap_32(sample) & 0xffffff00) ^ 0x8000); goto PUT_END;
-put_1234_1234: as_s32(dst) = sample; goto PUT_END;
-put_1234_9234: as_u32(dst) = sample ^ 0x80000000; goto PUT_END;
-put_1234_4321: as_s32(dst) = bswap_32(sample); goto PUT_END;
-put_1234_4329: as_u32(dst) = bswap_32(sample) ^ 0x80; goto PUT_END;
-#endif
-
 #ifdef PUT32F_LABELS
 /* type (0 = float, 1 = float64), endswap */
 static void *const put32float_labels[2 * 2] = {
-- 
2.0.1

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[shengjiu.wang]

Hi Iwai

     Get32_labels has left shift the data to 32bit, but gets_labels don't shift bits. So the index for norm_labels need to be updated too.

     I am not sure which change is better, fix the params->src_size to 4, or remove the norm table ( but there is norm_xx_att, norm_xx_noatt choices)? 

     
Best regards
Wang shengjiu

-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de] 
Sent: Tuesday, July 22, 2014 6:11 PM
To: Wang Shengjiu-B02247
Cc: perex@perex.cz; alsa-devel@alsa-project.org
Subject: Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

At Tue, 22 Jul 2014 07:59:03 +0000,
shengjiu.wang@freescale.com wrote:
> 
> Hi Iwai
> 
>    According to your comments, Should I define sx20 and sx18, because sx24 is for 24bit?

Right, they need the corresponding ones.

However, looking through the code again, it cannot work properly when the format isn't 32bit and float isn't used.  It reads the value with get label while it writes with put32 label.  The former reads the value as is, but the latter assumes that the source value is normalized to 32bit.

So, I guess the best way would be to use always 64bit int for route multi-source case (unless float is available).  This fixes the bug and simplifies a lot.  An untested patch is below.  Could you check whether this works for you?


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] pcm: route: Use get32 for multi-source route calculation

The PCM route plugin can assign the destination value from average of multiple sources with attenuation.  This requires the read of each channel value, sums and writes the resultant value in the requested format.

Currently, get_labels is used for reading source values while put32_labels is used for writing the dest value.  This is, however, a buggy implementation; get_labels gives the value as is only with endianness and signedness conversions, but put32_labels assumes that the value is normalized to 32bit int and it shifts down to the dest format.  In addition, the current code lacks get_labels entries for the 24bit formats, as Shengjiu Wang spotted out.

For fixing these bugs, this patch replaces the read with get32_labels and use always 64bit int for sum.  This simplifies the code a lot and drops many lines.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/pcm/pcm_route.c  | 62 +++++++++-------------------------------
 src/pcm/plugin_ops.h | 81 ----------------------------------------------------
 2 files changed, 14 insertions(+), 129 deletions(-)

diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index 72c198cb25d2..36a74ba661a9 100644
--- a/src/pcm/pcm_route.c
+++ b/src/pcm/pcm_route.c
@@ -60,7 +60,7 @@ typedef struct {
 typedef struct snd_pcm_route_ttable_dst snd_pcm_route_ttable_dst_t;
 
 typedef struct {
-	enum {UINT32=0, UINT64=1, FLOAT=2} sum_idx;
+	enum {UINT64, FLOAT} sum_idx;
 	unsigned int get_idx;
 	unsigned int put_idx;
 	unsigned int conv_idx;
@@ -233,35 +233,26 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 					const snd_pcm_route_ttable_dst_t* ttable,
 					const snd_pcm_route_params_t *params)  { -#define GETS_LABELS
+#define GET32_LABELS
 #define PUT32_LABELS
 #include "plugin_ops.h"
-#undef GETS_LABELS
+#undef GET32_LABELS
 #undef PUT32_LABELS
-	static void *const zero_labels[3] = {
-		&&zero_int32, &&zero_int64,
+	static void *const zero_labels[2] = {
+		&&zero_int64,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&zero_float
 #endif
 	};
 	/* sum_type att */
-	static void *const add_labels[3 * 2] = {
-		&&add_int32_noatt, &&add_int32_att,
+	static void *const add_labels[2 * 2] = {
 		&&add_int64_noatt, &&add_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&add_float_noatt, &&add_float_att
 #endif
 	};
 	/* sum_type att shift */
-	static void *const norm_labels[3 * 2 * 4] = {
-		0,
-		&&norm_int32_8_noatt,
-		&&norm_int32_16_noatt,
-		&&norm_int32_24_noatt,
-		0,
-		&&norm_int32_8_att,
-		&&norm_int32_16_att,
-		&&norm_int32_24_att,
+	static void *const norm_labels[2 * 2 * 4] = {
 		&&norm_int64_0_noatt,
 		&&norm_int64_8_noatt,
 		&&norm_int64_16_noatt,
@@ -281,7 +272,7 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		&&norm_float_24,
 #endif
 	};
-	void *zero, *get, *add, *norm, *put32;
+	void *zero, *get32, *add, *norm, *put32;
 	int nsrcs = ttable->nsrcs;
 	char *dst;
 	int dst_step;
@@ -323,7 +314,7 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 	}
 
 	zero = zero_labels[params->sum_idx];
-	get = gets_labels[params->get_idx];
+	get32 = get32_labels[params->get_idx];
 	add = add_labels[params->sum_idx * 2 + ttable->att];
 	norm = norm_labels[params->sum_idx * 8 + ttable->att * 4 + 4 - params->src_size];
 	put32 = put32_labels[params->put_idx]; @@ -336,9 +327,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 
 		/* Zero sum */
 		goto *zero;
-	zero_int32:
-		sum.as_sint32 = 0;
-		goto zero_end;
 	zero_int64: 
 		sum.as_sint64 = 0;
 		goto zero_end;
@@ -352,21 +340,14 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 			const char *src = srcs[srcidx];
 			
 			/* Get sample */
-			goto *get;
-#define GETS_END after_get
+			goto *get32;
+#define GET32_END after_get
 #include "plugin_ops.h"
-#undef GETS_END
+#undef GET32_END
 		after_get:
 
 			/* Sum */
 			goto *add;
-		add_int32_att:
-			sum.as_sint32 += sample * ttp->as_int;
-			goto after_sum;
-		add_int32_noatt:
-			if (ttp->as_int)
-				sum.as_sint32 += sample;
-			goto after_sum;
 		add_int64_att:
 			sum.as_sint64 += (int64_t) sample * ttp->as_int;
 			goto after_sum;
@@ -390,42 +371,30 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		
 		/* Normalization */
 		goto *norm;
-	norm_int32_8_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_8_att:
 		sum.as_sint64 <<= 8;
 	norm_int64_0_att:
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_16_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_16_att:
 		sum.as_sint64 <<= 16;
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_24_att:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_24_att:
 		sum.as_sint64 <<= 24;
 		div(sum.as_sint64);
 		goto norm_int;
 
-	norm_int32_8_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_8_noatt:
 		sum.as_sint64 <<= 8;
 		goto norm_int;
 
-	norm_int32_16_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_16_noatt:
 		sum.as_sint64 <<= 16;
 		goto norm_int;
 
-	norm_int32_24_noatt:
-		sum.as_sint64 = sum.as_sint32;
 	norm_int64_24_noatt:
 		sum.as_sint64 <<= 24;
 		goto norm_int;
@@ -648,7 +617,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 	route->params.use_getput =
 		(snd_pcm_format_physical_width(src_format) + 7) / 3 == 3 ||
 		(snd_pcm_format_physical_width(dst_format) + 7) / 3 == 3;
-	route->params.get_idx = snd_pcm_linear_get_index(src_format, SND_PCM_FORMAT_S16);
+	route->params.get_idx = snd_pcm_linear_get32_index(src_format, 
+SND_PCM_FORMAT_S32);
 	route->params.put_idx = snd_pcm_linear_put32_index(SND_PCM_FORMAT_S32, dst_format);
 	route->params.conv_idx = snd_pcm_linear_convert_index(src_format, dst_format);
 	route->params.src_size = snd_pcm_format_width(src_format) / 8; @@ -656,10 +625,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)  #if SND_PCM_PLUGIN_ROUTE_FLOAT
 	route->params.sum_idx = FLOAT;
 #else
-	if (snd_pcm_format_width(src_format) == 32)
-		route->params.sum_idx = UINT64;
-	else
-		route->params.sum_idx = UINT32;
+	route->params.sum_idx = UINT64;
 #endif
 	return 0;
 }
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 21535c9ca8d6..eb8c2c4f4dac 100644
--- a/src/pcm/plugin_ops.h
+++ b/src/pcm/plugin_ops.h
@@ -668,87 +668,6 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;  }  #endif
 
-#ifdef GETS_LABELS
-/* width endswap sign_toggle */
-static void *const gets_labels[4 * 2 * 2] = {
-	&&gets_1_1,		/*  8h ->  8h */
-	&&gets_1_9,		/*  8h ^>  8h */
-	&&gets_1_1,		/*  8s ->  8h */
-	&&gets_1_9,		/*  8s ^>  8h */
-	&&gets_12_12,		/* 16h -> 16h */
-	&&gets_12_92,		/* 16h ^> 16h */
-	&&gets_12_21,		/* 16s -> 16h */
-	&&gets_12_A1,		/* 16s ^> 16h */
-	&&gets_0123_0123,	/* 24h -> 24h */
-	&&gets_0123_0923,	/* 24h ^> 24h */
-	&&gets_1230_0321,	/* 24s -> 24h */
-	&&gets_1230_0B21,	/* 24s ^> 24h */
-	&&gets_1234_1234,	/* 32h -> 32h */
-	&&gets_1234_9234,	/* 32h ^> 32h */
-	&&gets_1234_4321,	/* 32s -> 32h */
-	&&gets_1234_C321,	/* 32s ^> 32h */
-};
-#endif
-
-#ifdef GETS_END
-while (0) {
-gets_1_1: sample = as_s8c(src); goto GETS_END;
-gets_1_9: sample = (int8_t)(as_s8c(src) ^ 0x80); goto GETS_END;
-gets_12_12: sample = as_s16c(src); goto GETS_END;
-gets_12_92: sample = (int16_t)(as_s16c(src) ^ 0x8000); goto GETS_END;
-gets_12_21: sample = (int16_t)bswap_16(as_s16c(src)); goto GETS_END;
-gets_12_A1: sample = (int16_t)bswap_16(as_s16c(src) ^ 0x80); goto GETS_END;
-gets_0123_0123: sample = sx24((int32_t)(as_s32c(src) << 8) >> 8); goto GETS_END;
-gets_0123_0923: sample = sx24((int32_t)((as_s32c(src) ^ 0x800000) << 8) >> 8); goto GETS_END;
-gets_1230_0321: sample = sx24((int32_t)(bswap_32(as_s32c(src)) << 8) >> 8); goto GETS_END;
-gets_1230_0B21: sample = sx24((int32_t)(bswap_32(as_s32c(src) ^ 0x8000) << 8) >> 8); goto GETS_END;
-gets_1234_1234: sample = as_s32c(src); goto GETS_END;
-gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
-gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
-gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END; -} -#endif
-
-#ifdef PUT_LABELS
-/* width endswap sign_toggle */
-static void *const put_labels[4 * 2 * 2] = {
-	&&put_1_1,		/*  8h ->  8h */
-	&&put_1_9,		/*  8h ^>  8h */
-	&&put_1_1,		/*  8h ->  8s */
-	&&put_1_9,		/*  8h ^>  8s */
-	&&put_12_12,		/* 16h -> 16h */
-	&&put_12_92,		/* 16h ^> 16h */
-	&&put_12_21,		/* 16h -> 16s */
-	&&put_12_29,		/* 16h ^> 16s */
-	&&put_0123_0123,	/* 24h -> 24h */
-	&&put_0123_0923,	/* 24h ^> 24h */
-	&&put_0123_3210,	/* 24h -> 24s */
-	&&put_0123_3290,	/* 24h ^> 24s */
-	&&put_1234_1234,	/* 32h -> 32h */
-	&&put_1234_9234,	/* 32h ^> 32h */
-	&&put_1234_4321,	/* 32h -> 32s */
-	&&put_1234_4329,	/* 32h ^> 32s */
-};
-#endif
-
-#ifdef PUT_END
-put_1_1: as_s8(dst) = sample; goto PUT_END;
-put_1_9: as_u8(dst) = sample ^ 0x80; goto PUT_END;
-put_12_12: as_s16(dst) = sample; goto PUT_END;
-put_12_92: as_u16(dst) = sample ^ 0x8000; goto PUT_END;
-put_12_21: as_s16(dst) = bswap_16(sample); goto PUT_END;
-put_12_29: as_u16(dst) = bswap_16(sample) ^ 0x80; goto PUT_END;
-/* this always writes the unused byte in 24-bit formats as 0x00 */
-put_0123_0123: as_s32(dst) = sx24(sample & 0x00ffffff); goto PUT_END;
-put_0123_0923: as_u32(dst) = sx24((sample & 0x00ffffff) ^ 0x800000); goto PUT_END;
-put_0123_3210: as_s32(dst) = sx24s(bswap_32(sample) & 0xffffff00); goto PUT_END;
-put_0123_3290: as_u32(dst) = sx24s((bswap_32(sample) & 0xffffff00) ^ 0x8000); goto PUT_END;
-put_1234_1234: as_s32(dst) = sample; goto PUT_END;
-put_1234_9234: as_u32(dst) = sample ^ 0x80000000; goto PUT_END;
-put_1234_4321: as_s32(dst) = bswap_32(sample); goto PUT_END;
-put_1234_4329: as_u32(dst) = bswap_32(sample) ^ 0x80; goto PUT_END; -#endif
-
 #ifdef PUT32F_LABELS
 /* type (0 = float, 1 = float64), endswap */  static void *const put32float_labels[2 * 2] = {
--
2.0.1

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[Takashi Iwai]

At Tue, 22 Jul 2014 11:38:05 +0000,
shengjiu.wang@freescale.com wrote:
> 
> Hi Iwai
> 
>      Get32_labels has left shift the data to 32bit, but gets_labels don't shift bits. So the index for norm_labels need to be updated too.

Oh yes, this was forgotten.

>      I am not sure which change is better, fix the params->src_size to 4, or remove the norm table ( but there is norm_xx_att, norm_xx_noatt choices)? 

The norm table can be simply removed.  The att and noatt are needed
for integer case.  The revised patch is below.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] pcm: route: Use get32 for multi-source route calculation

The PCM route plugin can assign the destination value from average of
multiple sources with attenuation.  This requires the read of each
channel value, sums and writes the resultant value in the requested
format.

Currently, get_labels is used for reading source values while
put32_labels is used for writing the dest value.  This is, however,
a buggy implementation; get_labels gives the value as is only with
endianness and signedness conversions, but put32_labels assumes that
the value is normalized to 32bit int and it shifts down to the dest
format.  In addition, the current code lacks get_labels entries for
the 24bit formats, as Shengjiu Wang spotted out.

For fixing these bugs, this patch replaces the read with
get32_labels and use always 64bit int for sum.  This simplifies the
code a lot and drops many lines.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/pcm/pcm_route.c  | 128 +++++++++------------------------------------------
 src/pcm/plugin_ops.h |  81 --------------------------------
 2 files changed, 23 insertions(+), 186 deletions(-)

diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c
index 72c198cb25d2..5dac7ebcb7df 100644
--- a/src/pcm/pcm_route.c
+++ b/src/pcm/pcm_route.c
@@ -60,7 +60,7 @@ typedef struct {
 typedef struct snd_pcm_route_ttable_dst snd_pcm_route_ttable_dst_t;
 
 typedef struct {
-	enum {UINT32=0, UINT64=1, FLOAT=2} sum_idx;
+	enum {UINT64, FLOAT} sum_idx;
 	unsigned int get_idx;
 	unsigned int put_idx;
 	unsigned int conv_idx;
@@ -233,55 +233,34 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 					const snd_pcm_route_ttable_dst_t* ttable,
 					const snd_pcm_route_params_t *params)
 {
-#define GETS_LABELS
+#define GET32_LABELS
 #define PUT32_LABELS
 #include "plugin_ops.h"
-#undef GETS_LABELS
+#undef GET32_LABELS
 #undef PUT32_LABELS
-	static void *const zero_labels[3] = {
-		&&zero_int32, &&zero_int64,
+	static void *const zero_labels[2] = {
+		&&zero_int64,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&zero_float
 #endif
 	};
 	/* sum_type att */
-	static void *const add_labels[3 * 2] = {
-		&&add_int32_noatt, &&add_int32_att,
+	static void *const add_labels[2 * 2] = {
 		&&add_int64_noatt, &&add_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&add_float_noatt, &&add_float_att
 #endif
 	};
-	/* sum_type att shift */
-	static void *const norm_labels[3 * 2 * 4] = {
-		0,
-		&&norm_int32_8_noatt,
-		&&norm_int32_16_noatt,
-		&&norm_int32_24_noatt,
-		0,
-		&&norm_int32_8_att,
-		&&norm_int32_16_att,
-		&&norm_int32_24_att,
-		&&norm_int64_0_noatt,
-		&&norm_int64_8_noatt,
-		&&norm_int64_16_noatt,
-		&&norm_int64_24_noatt,
-		&&norm_int64_0_att,
-		&&norm_int64_8_att,
-		&&norm_int64_16_att,
-		&&norm_int64_24_att,
+	/* sum_type att */
+	static void *const norm_labels[2 * 2] = {
+		&&norm_int64_noatt,
+		&&norm_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
-		&&norm_float_0,
-		&&norm_float_8,
-		&&norm_float_16,
-		&&norm_float_24,
-		&&norm_float_0,
-		&&norm_float_8,
-		&&norm_float_16,
-		&&norm_float_24,
+		&&norm_float,
+		&&norm_float,
 #endif
 	};
-	void *zero, *get, *add, *norm, *put32;
+	void *zero, *get32, *add, *norm, *put32;
 	int nsrcs = ttable->nsrcs;
 	char *dst;
 	int dst_step;
@@ -323,9 +302,9 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 	}
 
 	zero = zero_labels[params->sum_idx];
-	get = gets_labels[params->get_idx];
+	get32 = get32_labels[params->get_idx];
 	add = add_labels[params->sum_idx * 2 + ttable->att];
-	norm = norm_labels[params->sum_idx * 8 + ttable->att * 4 + 4 - params->src_size];
+	norm = norm_labels[params->sum_idx * 2 + ttable->att];
 	put32 = put32_labels[params->put_idx];
 	dst = snd_pcm_channel_area_addr(dst_area, dst_offset);
 	dst_step = snd_pcm_channel_area_step(dst_area);
@@ -336,9 +315,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 
 		/* Zero sum */
 		goto *zero;
-	zero_int32:
-		sum.as_sint32 = 0;
-		goto zero_end;
 	zero_int64: 
 		sum.as_sint64 = 0;
 		goto zero_end;
@@ -352,21 +328,14 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 			const char *src = srcs[srcidx];
 			
 			/* Get sample */
-			goto *get;
-#define GETS_END after_get
+			goto *get32;
+#define GET32_END after_get
 #include "plugin_ops.h"
-#undef GETS_END
+#undef GET32_END
 		after_get:
 
 			/* Sum */
 			goto *add;
-		add_int32_att:
-			sum.as_sint32 += sample * ttp->as_int;
-			goto after_sum;
-		add_int32_noatt:
-			if (ttp->as_int)
-				sum.as_sint32 += sample;
-			goto after_sum;
 		add_int64_att:
 			sum.as_sint64 += (int64_t) sample * ttp->as_int;
 			goto after_sum;
@@ -390,48 +359,10 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		
 		/* Normalization */
 		goto *norm;
-	norm_int32_8_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_8_att:
-		sum.as_sint64 <<= 8;
-	norm_int64_0_att:
+	norm_int64_att:
 		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_16_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_16_att:
-		sum.as_sint64 <<= 16;
-		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_24_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_24_att:
-		sum.as_sint64 <<= 24;
-		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_8_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_8_noatt:
-		sum.as_sint64 <<= 8;
-		goto norm_int;
-
-	norm_int32_16_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_16_noatt:
-		sum.as_sint64 <<= 16;
-		goto norm_int;
-
-	norm_int32_24_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_24_noatt:
-		sum.as_sint64 <<= 24;
-		goto norm_int;
-
-	norm_int64_0_noatt:
-	norm_int:
+		/* fallthru */
+	norm_int64_noatt:
 		if (sum.as_sint64 > (int64_t)0x7fffffff)
 			sample = 0x7fffffff;	/* maximum positive value */
 		else if (sum.as_sint64 < -(int64_t)0x80000000)
@@ -441,16 +372,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		goto after_norm;
 
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
-	norm_float_8:
-		sum.as_float *= 1 << 8;
-		goto norm_float;
-	norm_float_16:
-		sum.as_float *= 1 << 16;
-		goto norm_float;
-	norm_float_24:
-		sum.as_float *= 1 << 24;
-		goto norm_float;
-	norm_float_0:
 	norm_float:
 		sum.as_float = rint(sum.as_float);
 		if (sum.as_float > (int64_t)0x7fffffff)
@@ -648,7 +569,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 	route->params.use_getput =
 		(snd_pcm_format_physical_width(src_format) + 7) / 3 == 3 ||
 		(snd_pcm_format_physical_width(dst_format) + 7) / 3 == 3;
-	route->params.get_idx = snd_pcm_linear_get_index(src_format, SND_PCM_FORMAT_S16);
+	route->params.get_idx = snd_pcm_linear_get32_index(src_format, SND_PCM_FORMAT_S32);
 	route->params.put_idx = snd_pcm_linear_put32_index(SND_PCM_FORMAT_S32, dst_format);
 	route->params.conv_idx = snd_pcm_linear_convert_index(src_format, dst_format);
 	route->params.src_size = snd_pcm_format_width(src_format) / 8;
@@ -656,10 +577,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 	route->params.sum_idx = FLOAT;
 #else
-	if (snd_pcm_format_width(src_format) == 32)
-		route->params.sum_idx = UINT64;
-	else
-		route->params.sum_idx = UINT32;
+	route->params.sum_idx = UINT64;
 #endif
 	return 0;
 }
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h
index 21535c9ca8d6..eb8c2c4f4dac 100644
--- a/src/pcm/plugin_ops.h
+++ b/src/pcm/plugin_ops.h
@@ -668,87 +668,6 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;
 }
 #endif
 
-#ifdef GETS_LABELS
-/* width endswap sign_toggle */
-static void *const gets_labels[4 * 2 * 2] = {
-	&&gets_1_1,		/*  8h ->  8h */
-	&&gets_1_9,		/*  8h ^>  8h */
-	&&gets_1_1,		/*  8s ->  8h */
-	&&gets_1_9,		/*  8s ^>  8h */
-	&&gets_12_12,		/* 16h -> 16h */
-	&&gets_12_92,		/* 16h ^> 16h */
-	&&gets_12_21,		/* 16s -> 16h */
-	&&gets_12_A1,		/* 16s ^> 16h */
-	&&gets_0123_0123,	/* 24h -> 24h */
-	&&gets_0123_0923,	/* 24h ^> 24h */
-	&&gets_1230_0321,	/* 24s -> 24h */
-	&&gets_1230_0B21,	/* 24s ^> 24h */
-	&&gets_1234_1234,	/* 32h -> 32h */
-	&&gets_1234_9234,	/* 32h ^> 32h */
-	&&gets_1234_4321,	/* 32s -> 32h */
-	&&gets_1234_C321,	/* 32s ^> 32h */
-};
-#endif
-
-#ifdef GETS_END
-while (0) {
-gets_1_1: sample = as_s8c(src); goto GETS_END;
-gets_1_9: sample = (int8_t)(as_s8c(src) ^ 0x80); goto GETS_END;
-gets_12_12: sample = as_s16c(src); goto GETS_END;
-gets_12_92: sample = (int16_t)(as_s16c(src) ^ 0x8000); goto GETS_END;
-gets_12_21: sample = (int16_t)bswap_16(as_s16c(src)); goto GETS_END;
-gets_12_A1: sample = (int16_t)bswap_16(as_s16c(src) ^ 0x80); goto GETS_END;
-gets_0123_0123: sample = sx24((int32_t)(as_s32c(src) << 8) >> 8); goto GETS_END;
-gets_0123_0923: sample = sx24((int32_t)((as_s32c(src) ^ 0x800000) << 8) >> 8); goto GETS_END;
-gets_1230_0321: sample = sx24((int32_t)(bswap_32(as_s32c(src)) << 8) >> 8); goto GETS_END;
-gets_1230_0B21: sample = sx24((int32_t)(bswap_32(as_s32c(src) ^ 0x8000) << 8) >> 8); goto GETS_END;
-gets_1234_1234: sample = as_s32c(src); goto GETS_END;
-gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
-gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
-gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END;
-}
-#endif
-
-#ifdef PUT_LABELS
-/* width endswap sign_toggle */
-static void *const put_labels[4 * 2 * 2] = {
-	&&put_1_1,		/*  8h ->  8h */
-	&&put_1_9,		/*  8h ^>  8h */
-	&&put_1_1,		/*  8h ->  8s */
-	&&put_1_9,		/*  8h ^>  8s */
-	&&put_12_12,		/* 16h -> 16h */
-	&&put_12_92,		/* 16h ^> 16h */
-	&&put_12_21,		/* 16h -> 16s */
-	&&put_12_29,		/* 16h ^> 16s */
-	&&put_0123_0123,	/* 24h -> 24h */
-	&&put_0123_0923,	/* 24h ^> 24h */
-	&&put_0123_3210,	/* 24h -> 24s */
-	&&put_0123_3290,	/* 24h ^> 24s */
-	&&put_1234_1234,	/* 32h -> 32h */
-	&&put_1234_9234,	/* 32h ^> 32h */
-	&&put_1234_4321,	/* 32h -> 32s */
-	&&put_1234_4329,	/* 32h ^> 32s */
-};
-#endif
-
-#ifdef PUT_END
-put_1_1: as_s8(dst) = sample; goto PUT_END;
-put_1_9: as_u8(dst) = sample ^ 0x80; goto PUT_END;
-put_12_12: as_s16(dst) = sample; goto PUT_END;
-put_12_92: as_u16(dst) = sample ^ 0x8000; goto PUT_END;
-put_12_21: as_s16(dst) = bswap_16(sample); goto PUT_END;
-put_12_29: as_u16(dst) = bswap_16(sample) ^ 0x80; goto PUT_END;
-/* this always writes the unused byte in 24-bit formats as 0x00 */
-put_0123_0123: as_s32(dst) = sx24(sample & 0x00ffffff); goto PUT_END;
-put_0123_0923: as_u32(dst) = sx24((sample & 0x00ffffff) ^ 0x800000); goto PUT_END;
-put_0123_3210: as_s32(dst) = sx24s(bswap_32(sample) & 0xffffff00); goto PUT_END;
-put_0123_3290: as_u32(dst) = sx24s((bswap_32(sample) & 0xffffff00) ^ 0x8000); goto PUT_END;
-put_1234_1234: as_s32(dst) = sample; goto PUT_END;
-put_1234_9234: as_u32(dst) = sample ^ 0x80000000; goto PUT_END;
-put_1234_4321: as_s32(dst) = bswap_32(sample); goto PUT_END;
-put_1234_4329: as_u32(dst) = bswap_32(sample) ^ 0x80; goto PUT_END;
-#endif
-
 #ifdef PUT32F_LABELS
 /* type (0 = float, 1 = float64), endswap */
 static void *const put32float_labels[2 * 2] = {
-- 
2.0.1

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[shengjiu.wang]

Hi Iwai

   I have tested your patch,  it is ok for me. Thanks.

Best regards
Wang shengjiu

-----Original Message-----
From: Takashi Iwai [mailto:tiwai@suse.de] 
Sent: Tuesday, July 22, 2014 8:24 PM
To: Wang Shengjiu-B02247
Cc: perex@perex.cz; alsa-devel@alsa-project.org
Subject: Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

At Tue, 22 Jul 2014 11:38:05 +0000,
shengjiu.wang@freescale.com wrote:
> 
> Hi Iwai
> 
>      Get32_labels has left shift the data to 32bit, but gets_labels don't shift bits. So the index for norm_labels need to be updated too.

Oh yes, this was forgotten.

>      I am not sure which change is better, fix the params->src_size to 4, or remove the norm table ( but there is norm_xx_att, norm_xx_noatt choices)? 

The norm table can be simply removed.  The att and noatt are needed for integer case.  The revised patch is below.


thanks,

Takashi

---
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH v2] pcm: route: Use get32 for multi-source route calculation

The PCM route plugin can assign the destination value from average of multiple sources with attenuation.  This requires the read of each channel value, sums and writes the resultant value in the requested format.

Currently, get_labels is used for reading source values while put32_labels is used for writing the dest value.  This is, however, a buggy implementation; get_labels gives the value as is only with endianness and signedness conversions, but put32_labels assumes that the value is normalized to 32bit int and it shifts down to the dest format.  In addition, the current code lacks get_labels entries for the 24bit formats, as Shengjiu Wang spotted out.

For fixing these bugs, this patch replaces the read with get32_labels and use always 64bit int for sum.  This simplifies the code a lot and drops many lines.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 src/pcm/pcm_route.c  | 128 +++++++++------------------------------------------
 src/pcm/plugin_ops.h |  81 --------------------------------
 2 files changed, 23 insertions(+), 186 deletions(-)

diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index 72c198cb25d2..5dac7ebcb7df 100644
--- a/src/pcm/pcm_route.c
+++ b/src/pcm/pcm_route.c
@@ -60,7 +60,7 @@ typedef struct {
 typedef struct snd_pcm_route_ttable_dst snd_pcm_route_ttable_dst_t;
 
 typedef struct {
-	enum {UINT32=0, UINT64=1, FLOAT=2} sum_idx;
+	enum {UINT64, FLOAT} sum_idx;
 	unsigned int get_idx;
 	unsigned int put_idx;
 	unsigned int conv_idx;
@@ -233,55 +233,34 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 					const snd_pcm_route_ttable_dst_t* ttable,
 					const snd_pcm_route_params_t *params)  { -#define GETS_LABELS
+#define GET32_LABELS
 #define PUT32_LABELS
 #include "plugin_ops.h"
-#undef GETS_LABELS
+#undef GET32_LABELS
 #undef PUT32_LABELS
-	static void *const zero_labels[3] = {
-		&&zero_int32, &&zero_int64,
+	static void *const zero_labels[2] = {
+		&&zero_int64,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&zero_float
 #endif
 	};
 	/* sum_type att */
-	static void *const add_labels[3 * 2] = {
-		&&add_int32_noatt, &&add_int32_att,
+	static void *const add_labels[2 * 2] = {
 		&&add_int64_noatt, &&add_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
 		&&add_float_noatt, &&add_float_att
 #endif
 	};
-	/* sum_type att shift */
-	static void *const norm_labels[3 * 2 * 4] = {
-		0,
-		&&norm_int32_8_noatt,
-		&&norm_int32_16_noatt,
-		&&norm_int32_24_noatt,
-		0,
-		&&norm_int32_8_att,
-		&&norm_int32_16_att,
-		&&norm_int32_24_att,
-		&&norm_int64_0_noatt,
-		&&norm_int64_8_noatt,
-		&&norm_int64_16_noatt,
-		&&norm_int64_24_noatt,
-		&&norm_int64_0_att,
-		&&norm_int64_8_att,
-		&&norm_int64_16_att,
-		&&norm_int64_24_att,
+	/* sum_type att */
+	static void *const norm_labels[2 * 2] = {
+		&&norm_int64_noatt,
+		&&norm_int64_att,
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
-		&&norm_float_0,
-		&&norm_float_8,
-		&&norm_float_16,
-		&&norm_float_24,
-		&&norm_float_0,
-		&&norm_float_8,
-		&&norm_float_16,
-		&&norm_float_24,
+		&&norm_float,
+		&&norm_float,
 #endif
 	};
-	void *zero, *get, *add, *norm, *put32;
+	void *zero, *get32, *add, *norm, *put32;
 	int nsrcs = ttable->nsrcs;
 	char *dst;
 	int dst_step;
@@ -323,9 +302,9 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 	}
 
 	zero = zero_labels[params->sum_idx];
-	get = gets_labels[params->get_idx];
+	get32 = get32_labels[params->get_idx];
 	add = add_labels[params->sum_idx * 2 + ttable->att];
-	norm = norm_labels[params->sum_idx * 8 + ttable->att * 4 + 4 - params->src_size];
+	norm = norm_labels[params->sum_idx * 2 + ttable->att];
 	put32 = put32_labels[params->put_idx];
 	dst = snd_pcm_channel_area_addr(dst_area, dst_offset);
 	dst_step = snd_pcm_channel_area_step(dst_area);
@@ -336,9 +315,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 
 		/* Zero sum */
 		goto *zero;
-	zero_int32:
-		sum.as_sint32 = 0;
-		goto zero_end;
 	zero_int64: 
 		sum.as_sint64 = 0;
 		goto zero_end;
@@ -352,21 +328,14 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 			const char *src = srcs[srcidx];
 			
 			/* Get sample */
-			goto *get;
-#define GETS_END after_get
+			goto *get32;
+#define GET32_END after_get
 #include "plugin_ops.h"
-#undef GETS_END
+#undef GET32_END
 		after_get:
 
 			/* Sum */
 			goto *add;
-		add_int32_att:
-			sum.as_sint32 += sample * ttp->as_int;
-			goto after_sum;
-		add_int32_noatt:
-			if (ttp->as_int)
-				sum.as_sint32 += sample;
-			goto after_sum;
 		add_int64_att:
 			sum.as_sint64 += (int64_t) sample * ttp->as_int;
 			goto after_sum;
@@ -390,48 +359,10 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		
 		/* Normalization */
 		goto *norm;
-	norm_int32_8_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_8_att:
-		sum.as_sint64 <<= 8;
-	norm_int64_0_att:
+	norm_int64_att:
 		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_16_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_16_att:
-		sum.as_sint64 <<= 16;
-		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_24_att:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_24_att:
-		sum.as_sint64 <<= 24;
-		div(sum.as_sint64);
-		goto norm_int;
-
-	norm_int32_8_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_8_noatt:
-		sum.as_sint64 <<= 8;
-		goto norm_int;
-
-	norm_int32_16_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_16_noatt:
-		sum.as_sint64 <<= 16;
-		goto norm_int;
-
-	norm_int32_24_noatt:
-		sum.as_sint64 = sum.as_sint32;
-	norm_int64_24_noatt:
-		sum.as_sint64 <<= 24;
-		goto norm_int;
-
-	norm_int64_0_noatt:
-	norm_int:
+		/* fallthru */
+	norm_int64_noatt:
 		if (sum.as_sint64 > (int64_t)0x7fffffff)
 			sample = 0x7fffffff;	/* maximum positive value */
 		else if (sum.as_sint64 < -(int64_t)0x80000000) @@ -441,16 +372,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
 		goto after_norm;
 
 #if SND_PCM_PLUGIN_ROUTE_FLOAT
-	norm_float_8:
-		sum.as_float *= 1 << 8;
-		goto norm_float;
-	norm_float_16:
-		sum.as_float *= 1 << 16;
-		goto norm_float;
-	norm_float_24:
-		sum.as_float *= 1 << 24;
-		goto norm_float;
-	norm_float_0:
 	norm_float:
 		sum.as_float = rint(sum.as_float);
 		if (sum.as_float > (int64_t)0x7fffffff) @@ -648,7 +569,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 	route->params.use_getput =
 		(snd_pcm_format_physical_width(src_format) + 7) / 3 == 3 ||
 		(snd_pcm_format_physical_width(dst_format) + 7) / 3 == 3;
-	route->params.get_idx = snd_pcm_linear_get_index(src_format, SND_PCM_FORMAT_S16);
+	route->params.get_idx = snd_pcm_linear_get32_index(src_format, 
+SND_PCM_FORMAT_S32);
 	route->params.put_idx = snd_pcm_linear_put32_index(SND_PCM_FORMAT_S32, dst_format);
 	route->params.conv_idx = snd_pcm_linear_convert_index(src_format, dst_format);
 	route->params.src_size = snd_pcm_format_width(src_format) / 8; @@ -656,10 +577,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)  #if SND_PCM_PLUGIN_ROUTE_FLOAT
 	route->params.sum_idx = FLOAT;
 #else
-	if (snd_pcm_format_width(src_format) == 32)
-		route->params.sum_idx = UINT64;
-	else
-		route->params.sum_idx = UINT32;
+	route->params.sum_idx = UINT64;
 #endif
 	return 0;
 }
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 21535c9ca8d6..eb8c2c4f4dac 100644
--- a/src/pcm/plugin_ops.h
+++ b/src/pcm/plugin_ops.h
@@ -668,87 +668,6 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;  }  #endif
 
-#ifdef GETS_LABELS
-/* width endswap sign_toggle */
-static void *const gets_labels[4 * 2 * 2] = {
-	&&gets_1_1,		/*  8h ->  8h */
-	&&gets_1_9,		/*  8h ^>  8h */
-	&&gets_1_1,		/*  8s ->  8h */
-	&&gets_1_9,		/*  8s ^>  8h */
-	&&gets_12_12,		/* 16h -> 16h */
-	&&gets_12_92,		/* 16h ^> 16h */
-	&&gets_12_21,		/* 16s -> 16h */
-	&&gets_12_A1,		/* 16s ^> 16h */
-	&&gets_0123_0123,	/* 24h -> 24h */
-	&&gets_0123_0923,	/* 24h ^> 24h */
-	&&gets_1230_0321,	/* 24s -> 24h */
-	&&gets_1230_0B21,	/* 24s ^> 24h */
-	&&gets_1234_1234,	/* 32h -> 32h */
-	&&gets_1234_9234,	/* 32h ^> 32h */
-	&&gets_1234_4321,	/* 32s -> 32h */
-	&&gets_1234_C321,	/* 32s ^> 32h */
-};
-#endif
-
-#ifdef GETS_END
-while (0) {
-gets_1_1: sample = as_s8c(src); goto GETS_END;
-gets_1_9: sample = (int8_t)(as_s8c(src) ^ 0x80); goto GETS_END;
-gets_12_12: sample = as_s16c(src); goto GETS_END;
-gets_12_92: sample = (int16_t)(as_s16c(src) ^ 0x8000); goto GETS_END;
-gets_12_21: sample = (int16_t)bswap_16(as_s16c(src)); goto GETS_END;
-gets_12_A1: sample = (int16_t)bswap_16(as_s16c(src) ^ 0x80); goto GETS_END;
-gets_0123_0123: sample = sx24((int32_t)(as_s32c(src) << 8) >> 8); goto GETS_END;
-gets_0123_0923: sample = sx24((int32_t)((as_s32c(src) ^ 0x800000) << 8) >> 8); goto GETS_END;
-gets_1230_0321: sample = sx24((int32_t)(bswap_32(as_s32c(src)) << 8) >> 8); goto GETS_END;
-gets_1230_0B21: sample = sx24((int32_t)(bswap_32(as_s32c(src) ^ 0x8000) << 8) >> 8); goto GETS_END;
-gets_1234_1234: sample = as_s32c(src); goto GETS_END;
-gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
-gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
-gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END; -} -#endif
-
-#ifdef PUT_LABELS
-/* width endswap sign_toggle */
-static void *const put_labels[4 * 2 * 2] = {
-	&&put_1_1,		/*  8h ->  8h */
-	&&put_1_9,		/*  8h ^>  8h */
-	&&put_1_1,		/*  8h ->  8s */
-	&&put_1_9,		/*  8h ^>  8s */
-	&&put_12_12,		/* 16h -> 16h */
-	&&put_12_92,		/* 16h ^> 16h */
-	&&put_12_21,		/* 16h -> 16s */
-	&&put_12_29,		/* 16h ^> 16s */
-	&&put_0123_0123,	/* 24h -> 24h */
-	&&put_0123_0923,	/* 24h ^> 24h */
-	&&put_0123_3210,	/* 24h -> 24s */
-	&&put_0123_3290,	/* 24h ^> 24s */
-	&&put_1234_1234,	/* 32h -> 32h */
-	&&put_1234_9234,	/* 32h ^> 32h */
-	&&put_1234_4321,	/* 32h -> 32s */
-	&&put_1234_4329,	/* 32h ^> 32s */
-};
-#endif
-
-#ifdef PUT_END
-put_1_1: as_s8(dst) = sample; goto PUT_END;
-put_1_9: as_u8(dst) = sample ^ 0x80; goto PUT_END;
-put_12_12: as_s16(dst) = sample; goto PUT_END;
-put_12_92: as_u16(dst) = sample ^ 0x8000; goto PUT_END;
-put_12_21: as_s16(dst) = bswap_16(sample); goto PUT_END;
-put_12_29: as_u16(dst) = bswap_16(sample) ^ 0x80; goto PUT_END;
-/* this always writes the unused byte in 24-bit formats as 0x00 */
-put_0123_0123: as_s32(dst) = sx24(sample & 0x00ffffff); goto PUT_END;
-put_0123_0923: as_u32(dst) = sx24((sample & 0x00ffffff) ^ 0x800000); goto PUT_END;
-put_0123_3210: as_s32(dst) = sx24s(bswap_32(sample) & 0xffffff00); goto PUT_END;
-put_0123_3290: as_u32(dst) = sx24s((bswap_32(sample) & 0xffffff00) ^ 0x8000); goto PUT_END;
-put_1234_1234: as_s32(dst) = sample; goto PUT_END;
-put_1234_9234: as_u32(dst) = sample ^ 0x80000000; goto PUT_END;
-put_1234_4321: as_s32(dst) = bswap_32(sample); goto PUT_END;
-put_1234_4329: as_u32(dst) = bswap_32(sample) ^ 0x80; goto PUT_END; -#endif
-
 #ifdef PUT32F_LABELS
 /* type (0 = float, 1 = float64), endswap */  static void *const put32float_labels[2 * 2] = {
--
2.0.1

Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label

[Takashi Iwai]

At Wed, 23 Jul 2014 02:08:33 +0000,
shengjiu.wang@freescale.com wrote:
> 
> Hi Iwai
> 
>    I have tested your patch,  it is ok for me. Thanks.

Thanks, I merged and pushed to git tree now.


Takashi


> Best regards
> Wang shengjiu
> 
> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de] 
> Sent: Tuesday, July 22, 2014 8:24 PM
> To: Wang Shengjiu-B02247
> Cc: perex@perex.cz; alsa-devel@alsa-project.org
> Subject: Re: [PATCH V2] pcm: plugin_ops.h: Add S24_3LE/S20_3LE/S18_3LE support in gets_label
> 
> At Tue, 22 Jul 2014 11:38:05 +0000,
> shengjiu.wang@freescale.com wrote:
> > 
> > Hi Iwai
> > 
> >      Get32_labels has left shift the data to 32bit, but gets_labels don't shift bits. So the index for norm_labels need to be updated too.
> 
> Oh yes, this was forgotten.
> 
> >      I am not sure which change is better, fix the params->src_size to 4, or remove the norm table ( but there is norm_xx_att, norm_xx_noatt choices)? 
> 
> The norm table can be simply removed.  The att and noatt are needed for integer case.  The revised patch is below.
> 
> 
> thanks,
> 
> Takashi
> 
> ---
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH v2] pcm: route: Use get32 for multi-source route calculation
> 
> The PCM route plugin can assign the destination value from average of multiple sources with attenuation.  This requires the read of each channel value, sums and writes the resultant value in the requested format.
> 
> Currently, get_labels is used for reading source values while put32_labels is used for writing the dest value.  This is, however, a buggy implementation; get_labels gives the value as is only with endianness and signedness conversions, but put32_labels assumes that the value is normalized to 32bit int and it shifts down to the dest format.  In addition, the current code lacks get_labels entries for the 24bit formats, as Shengjiu Wang spotted out.
> 
> For fixing these bugs, this patch replaces the read with get32_labels and use always 64bit int for sum.  This simplifies the code a lot and drops many lines.
> 
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  src/pcm/pcm_route.c  | 128 +++++++++------------------------------------------
>  src/pcm/plugin_ops.h |  81 --------------------------------
>  2 files changed, 23 insertions(+), 186 deletions(-)
> 
> diff --git a/src/pcm/pcm_route.c b/src/pcm/pcm_route.c index 72c198cb25d2..5dac7ebcb7df 100644
> --- a/src/pcm/pcm_route.c
> +++ b/src/pcm/pcm_route.c
> @@ -60,7 +60,7 @@ typedef struct {
>  typedef struct snd_pcm_route_ttable_dst snd_pcm_route_ttable_dst_t;
>  
>  typedef struct {
> -	enum {UINT32=0, UINT64=1, FLOAT=2} sum_idx;
> +	enum {UINT64, FLOAT} sum_idx;
>  	unsigned int get_idx;
>  	unsigned int put_idx;
>  	unsigned int conv_idx;
> @@ -233,55 +233,34 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  					const snd_pcm_route_ttable_dst_t* ttable,
>  					const snd_pcm_route_params_t *params)  { -#define GETS_LABELS
> +#define GET32_LABELS
>  #define PUT32_LABELS
>  #include "plugin_ops.h"
> -#undef GETS_LABELS
> +#undef GET32_LABELS
>  #undef PUT32_LABELS
> -	static void *const zero_labels[3] = {
> -		&&zero_int32, &&zero_int64,
> +	static void *const zero_labels[2] = {
> +		&&zero_int64,
>  #if SND_PCM_PLUGIN_ROUTE_FLOAT
>  		&&zero_float
>  #endif
>  	};
>  	/* sum_type att */
> -	static void *const add_labels[3 * 2] = {
> -		&&add_int32_noatt, &&add_int32_att,
> +	static void *const add_labels[2 * 2] = {
>  		&&add_int64_noatt, &&add_int64_att,
>  #if SND_PCM_PLUGIN_ROUTE_FLOAT
>  		&&add_float_noatt, &&add_float_att
>  #endif
>  	};
> -	/* sum_type att shift */
> -	static void *const norm_labels[3 * 2 * 4] = {
> -		0,
> -		&&norm_int32_8_noatt,
> -		&&norm_int32_16_noatt,
> -		&&norm_int32_24_noatt,
> -		0,
> -		&&norm_int32_8_att,
> -		&&norm_int32_16_att,
> -		&&norm_int32_24_att,
> -		&&norm_int64_0_noatt,
> -		&&norm_int64_8_noatt,
> -		&&norm_int64_16_noatt,
> -		&&norm_int64_24_noatt,
> -		&&norm_int64_0_att,
> -		&&norm_int64_8_att,
> -		&&norm_int64_16_att,
> -		&&norm_int64_24_att,
> +	/* sum_type att */
> +	static void *const norm_labels[2 * 2] = {
> +		&&norm_int64_noatt,
> +		&&norm_int64_att,
>  #if SND_PCM_PLUGIN_ROUTE_FLOAT
> -		&&norm_float_0,
> -		&&norm_float_8,
> -		&&norm_float_16,
> -		&&norm_float_24,
> -		&&norm_float_0,
> -		&&norm_float_8,
> -		&&norm_float_16,
> -		&&norm_float_24,
> +		&&norm_float,
> +		&&norm_float,
>  #endif
>  	};
> -	void *zero, *get, *add, *norm, *put32;
> +	void *zero, *get32, *add, *norm, *put32;
>  	int nsrcs = ttable->nsrcs;
>  	char *dst;
>  	int dst_step;
> @@ -323,9 +302,9 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  	}
>  
>  	zero = zero_labels[params->sum_idx];
> -	get = gets_labels[params->get_idx];
> +	get32 = get32_labels[params->get_idx];
>  	add = add_labels[params->sum_idx * 2 + ttable->att];
> -	norm = norm_labels[params->sum_idx * 8 + ttable->att * 4 + 4 - params->src_size];
> +	norm = norm_labels[params->sum_idx * 2 + ttable->att];
>  	put32 = put32_labels[params->put_idx];
>  	dst = snd_pcm_channel_area_addr(dst_area, dst_offset);
>  	dst_step = snd_pcm_channel_area_step(dst_area);
> @@ -336,9 +315,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  
>  		/* Zero sum */
>  		goto *zero;
> -	zero_int32:
> -		sum.as_sint32 = 0;
> -		goto zero_end;
>  	zero_int64: 
>  		sum.as_sint64 = 0;
>  		goto zero_end;
> @@ -352,21 +328,14 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  			const char *src = srcs[srcidx];
>  			
>  			/* Get sample */
> -			goto *get;
> -#define GETS_END after_get
> +			goto *get32;
> +#define GET32_END after_get
>  #include "plugin_ops.h"
> -#undef GETS_END
> +#undef GET32_END
>  		after_get:
>  
>  			/* Sum */
>  			goto *add;
> -		add_int32_att:
> -			sum.as_sint32 += sample * ttp->as_int;
> -			goto after_sum;
> -		add_int32_noatt:
> -			if (ttp->as_int)
> -				sum.as_sint32 += sample;
> -			goto after_sum;
>  		add_int64_att:
>  			sum.as_sint64 += (int64_t) sample * ttp->as_int;
>  			goto after_sum;
> @@ -390,48 +359,10 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  		
>  		/* Normalization */
>  		goto *norm;
> -	norm_int32_8_att:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_8_att:
> -		sum.as_sint64 <<= 8;
> -	norm_int64_0_att:
> +	norm_int64_att:
>  		div(sum.as_sint64);
> -		goto norm_int;
> -
> -	norm_int32_16_att:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_16_att:
> -		sum.as_sint64 <<= 16;
> -		div(sum.as_sint64);
> -		goto norm_int;
> -
> -	norm_int32_24_att:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_24_att:
> -		sum.as_sint64 <<= 24;
> -		div(sum.as_sint64);
> -		goto norm_int;
> -
> -	norm_int32_8_noatt:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_8_noatt:
> -		sum.as_sint64 <<= 8;
> -		goto norm_int;
> -
> -	norm_int32_16_noatt:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_16_noatt:
> -		sum.as_sint64 <<= 16;
> -		goto norm_int;
> -
> -	norm_int32_24_noatt:
> -		sum.as_sint64 = sum.as_sint32;
> -	norm_int64_24_noatt:
> -		sum.as_sint64 <<= 24;
> -		goto norm_int;
> -
> -	norm_int64_0_noatt:
> -	norm_int:
> +		/* fallthru */
> +	norm_int64_noatt:
>  		if (sum.as_sint64 > (int64_t)0x7fffffff)
>  			sample = 0x7fffffff;	/* maximum positive value */
>  		else if (sum.as_sint64 < -(int64_t)0x80000000) @@ -441,16 +372,6 @@ static void snd_pcm_route_convert1_many(const snd_pcm_channel_area_t *dst_area,
>  		goto after_norm;
>  
>  #if SND_PCM_PLUGIN_ROUTE_FLOAT
> -	norm_float_8:
> -		sum.as_float *= 1 << 8;
> -		goto norm_float;
> -	norm_float_16:
> -		sum.as_float *= 1 << 16;
> -		goto norm_float;
> -	norm_float_24:
> -		sum.as_float *= 1 << 24;
> -		goto norm_float;
> -	norm_float_0:
>  	norm_float:
>  		sum.as_float = rint(sum.as_float);
>  		if (sum.as_float > (int64_t)0x7fffffff) @@ -648,7 +569,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
>  	route->params.use_getput =
>  		(snd_pcm_format_physical_width(src_format) + 7) / 3 == 3 ||
>  		(snd_pcm_format_physical_width(dst_format) + 7) / 3 == 3;
> -	route->params.get_idx = snd_pcm_linear_get_index(src_format, SND_PCM_FORMAT_S16);
> +	route->params.get_idx = snd_pcm_linear_get32_index(src_format, 
> +SND_PCM_FORMAT_S32);
>  	route->params.put_idx = snd_pcm_linear_put32_index(SND_PCM_FORMAT_S32, dst_format);
>  	route->params.conv_idx = snd_pcm_linear_convert_index(src_format, dst_format);
>  	route->params.src_size = snd_pcm_format_width(src_format) / 8; @@ -656,10 +577,7 @@ static int snd_pcm_route_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)  #if SND_PCM_PLUGIN_ROUTE_FLOAT
>  	route->params.sum_idx = FLOAT;
>  #else
> -	if (snd_pcm_format_width(src_format) == 32)
> -		route->params.sum_idx = UINT64;
> -	else
> -		route->params.sum_idx = UINT32;
> +	route->params.sum_idx = UINT64;
>  #endif
>  	return 0;
>  }
> diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 21535c9ca8d6..eb8c2c4f4dac 100644
> --- a/src/pcm/plugin_ops.h
> +++ b/src/pcm/plugin_ops.h
> @@ -668,87 +668,6 @@ getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END;  }  #endif
>  
> -#ifdef GETS_LABELS
> -/* width endswap sign_toggle */
> -static void *const gets_labels[4 * 2 * 2] = {
> -	&&gets_1_1,		/*  8h ->  8h */
> -	&&gets_1_9,		/*  8h ^>  8h */
> -	&&gets_1_1,		/*  8s ->  8h */
> -	&&gets_1_9,		/*  8s ^>  8h */
> -	&&gets_12_12,		/* 16h -> 16h */
> -	&&gets_12_92,		/* 16h ^> 16h */
> -	&&gets_12_21,		/* 16s -> 16h */
> -	&&gets_12_A1,		/* 16s ^> 16h */
> -	&&gets_0123_0123,	/* 24h -> 24h */
> -	&&gets_0123_0923,	/* 24h ^> 24h */
> -	&&gets_1230_0321,	/* 24s -> 24h */
> -	&&gets_1230_0B21,	/* 24s ^> 24h */
> -	&&gets_1234_1234,	/* 32h -> 32h */
> -	&&gets_1234_9234,	/* 32h ^> 32h */
> -	&&gets_1234_4321,	/* 32s -> 32h */
> -	&&gets_1234_C321,	/* 32s ^> 32h */
> -};
> -#endif
> -
> -#ifdef GETS_END
> -while (0) {
> -gets_1_1: sample = as_s8c(src); goto GETS_END;
> -gets_1_9: sample = (int8_t)(as_s8c(src) ^ 0x80); goto GETS_END;
> -gets_12_12: sample = as_s16c(src); goto GETS_END;
> -gets_12_92: sample = (int16_t)(as_s16c(src) ^ 0x8000); goto GETS_END;
> -gets_12_21: sample = (int16_t)bswap_16(as_s16c(src)); goto GETS_END;
> -gets_12_A1: sample = (int16_t)bswap_16(as_s16c(src) ^ 0x80); goto GETS_END;
> -gets_0123_0123: sample = sx24((int32_t)(as_s32c(src) << 8) >> 8); goto GETS_END;
> -gets_0123_0923: sample = sx24((int32_t)((as_s32c(src) ^ 0x800000) << 8) >> 8); goto GETS_END;
> -gets_1230_0321: sample = sx24((int32_t)(bswap_32(as_s32c(src)) << 8) >> 8); goto GETS_END;
> -gets_1230_0B21: sample = sx24((int32_t)(bswap_32(as_s32c(src) ^ 0x8000) << 8) >> 8); goto GETS_END;
> -gets_1234_1234: sample = as_s32c(src); goto GETS_END;
> -gets_1234_9234: sample = (int32_t)(as_s32c(src) ^ 0x80000000); goto GETS_END;
> -gets_1234_4321: sample = (int32_t)bswap_32(as_s32c(src)); goto GETS_END;
> -gets_1234_C321: sample = (int32_t)bswap_32(as_s32c(src) ^ 0x80); goto GETS_END; -} -#endif
> -
> -#ifdef PUT_LABELS
> -/* width endswap sign_toggle */
> -static void *const put_labels[4 * 2 * 2] = {
> -	&&put_1_1,		/*  8h ->  8h */
> -	&&put_1_9,		/*  8h ^>  8h */
> -	&&put_1_1,		/*  8h ->  8s */
> -	&&put_1_9,		/*  8h ^>  8s */
> -	&&put_12_12,		/* 16h -> 16h */
> -	&&put_12_92,		/* 16h ^> 16h */
> -	&&put_12_21,		/* 16h -> 16s */
> -	&&put_12_29,		/* 16h ^> 16s */
> -	&&put_0123_0123,	/* 24h -> 24h */
> -	&&put_0123_0923,	/* 24h ^> 24h */
> -	&&put_0123_3210,	/* 24h -> 24s */
> -	&&put_0123_3290,	/* 24h ^> 24s */
> -	&&put_1234_1234,	/* 32h -> 32h */
> -	&&put_1234_9234,	/* 32h ^> 32h */
> -	&&put_1234_4321,	/* 32h -> 32s */
> -	&&put_1234_4329,	/* 32h ^> 32s */
> -};
> -#endif
> -
> -#ifdef PUT_END
> -put_1_1: as_s8(dst) = sample; goto PUT_END;
> -put_1_9: as_u8(dst) = sample ^ 0x80; goto PUT_END;
> -put_12_12: as_s16(dst) = sample; goto PUT_END;
> -put_12_92: as_u16(dst) = sample ^ 0x8000; goto PUT_END;
> -put_12_21: as_s16(dst) = bswap_16(sample); goto PUT_END;
> -put_12_29: as_u16(dst) = bswap_16(sample) ^ 0x80; goto PUT_END;
> -/* this always writes the unused byte in 24-bit formats as 0x00 */
> -put_0123_0123: as_s32(dst) = sx24(sample & 0x00ffffff); goto PUT_END;
> -put_0123_0923: as_u32(dst) = sx24((sample & 0x00ffffff) ^ 0x800000); goto PUT_END;
> -put_0123_3210: as_s32(dst) = sx24s(bswap_32(sample) & 0xffffff00); goto PUT_END;
> -put_0123_3290: as_u32(dst) = sx24s((bswap_32(sample) & 0xffffff00) ^ 0x8000); goto PUT_END;
> -put_1234_1234: as_s32(dst) = sample; goto PUT_END;
> -put_1234_9234: as_u32(dst) = sample ^ 0x80000000; goto PUT_END;
> -put_1234_4321: as_s32(dst) = bswap_32(sample); goto PUT_END;
> -put_1234_4329: as_u32(dst) = bswap_32(sample) ^ 0x80; goto PUT_END; -#endif
> -
>  #ifdef PUT32F_LABELS
>  /* type (0 = float, 1 = float64), endswap */  static void *const put32float_labels[2 * 2] = {
> --
> 2.0.1
>