[alsa-devel] [alsa-lib][PATCH 0/4] pcm: minor code cleanup

Hi,
When reviewing Maciej's patches[1], I found that alsa-lib includes some unused codes in linear interpolation functionality. Furthermore, some code comments includes 'copy & paste' mistakes.
This patchset improves them.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129370.ht...
Takashi Sakamoto (4): pcm: remove unused macros of COPY_LABELS/COPY_END pcm: remove unused macros of GETU_LABELS/GETU_END pcm: remove unused macros of NORMS_LABELS/NORMS_END pcm: fix wrong comments for some cases of linear interpolation of PCM samples
src/pcm/plugin_ops.h | 341 ++------------------------------------------------- 1 file changed, 8 insertions(+), 333 deletions(-)

A commit 7b054f4dce56 obsoleted usage of a pair of COPY_LABELS/COPY_END, however it did not remove some codes in 'src/pcm/plugin_ops.h'.
This commit removes them in a point to reduce the amount of code to maintain. --- src/pcm/plugin_ops.h | 20 -------------------- 1 file changed, 20 deletions(-)
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index a784d9c2..a0404909 100644 --- a/src/pcm/plugin_ops.h +++ b/src/pcm/plugin_ops.h @@ -92,26 +92,6 @@ static inline uint32_t sx24s(uint32_t x) #define _put_triple_s(ptr,val) _put_triple_le(ptr,val) #endif
-#ifdef COPY_LABELS -static void *copy_labels[5] = { - &©_8, - &©_16, - &©_24 - &©_32, - &©_64 -}; -#endif - -#ifdef COPY_END -while(0) { -copy_8: as_s8(dst) = as_s8c(src); goto COPY_END; -copy_16: as_s16(dst) = as_s16c(src); goto COPY_END; -copy_24: memcpy(dst,src,3); goto COPY_END; -copy_32: as_s32(dst) = as_s32c(src); goto COPY_END; -copy_64: as_s64(dst) = as_s64c(src); goto COPY_END; -} -#endif - #ifdef CONV_LABELS /* src_wid src_endswap sign_toggle dst_wid dst_endswap */ static void *const conv_labels[4 * 2 * 2 * 4 * 2] = {

A commit 07c07da44f27 ("Fixed signess for route conversion") obsoletes usage of a pair of GETU_LABEL/GETU_END, but it did not remove some actual codes in 'src/pcm_plugin_ops.h'.
This commit removes them in a point to reduce the amount of code to maintain. --- src/pcm/plugin_ops.h | 41 ----------------------------------------- 1 file changed, 41 deletions(-)
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index a0404909..1866d6d9 100644 --- a/src/pcm/plugin_ops.h +++ b/src/pcm/plugin_ops.h @@ -649,47 +649,6 @@ put32_1234_329_18: _put_triple_s(dst, (sample ^ 0x80000000) >> 14); goto PUT32_E #undef PUT32_END #endif
-#ifdef GETU_LABELS -/* width endswap sign_toggle */ -static void *const getu_labels[4 * 2 * 2] = { - &&getu_1_1, /* 8h -> 8h */ - &&getu_1_9, /* 8h ^> 8h */ - &&getu_1_1, /* 8s -> 8h */ - &&getu_1_9, /* 8s ^> 8h */ - &&getu_12_12, /* 16h -> 16h */ - &&getu_12_92, /* 16h ^> 16h */ - &&getu_12_21, /* 16s -> 16h */ - &&getu_12_A1, /* 16s ^> 16h */ - &&getu_0123_0123, /* 24h -> 24h */ - &&getu_0123_0923, /* 24h ^> 24h */ - &&getu_1230_0321, /* 24s -> 24h */ - &&getu_1230_0B21, /* 24s ^> 24h */ - &&getu_1234_1234, /* 32h -> 32h */ - &&getu_1234_9234, /* 32h ^> 32h */ - &&getu_1234_4321, /* 32s -> 32h */ - &&getu_1234_C321, /* 32s ^> 32h */ -}; -#endif - -#ifdef GETU_END -while (0) { -getu_1_1: sample = as_u8c(src); goto GETU_END; -getu_1_9: sample = as_u8c(src) ^ 0x80; goto GETU_END; -getu_12_12: sample = as_u16c(src); goto GETU_END; -getu_12_92: sample = as_u16c(src) ^ 0x8000; goto GETU_END; -getu_12_21: sample = bswap_16(as_u16c(src)); goto GETU_END; -getu_12_A1: sample = bswap_16(as_u16c(src) ^ 0x80); goto GETU_END; -getu_0123_0123: sample = sx24(as_u32c(src)); goto GETU_END; -getu_0123_0923: sample = sx24(as_u32c(src) ^ 0x800000); goto GETU_END; -getu_1230_0321: sample = sx24(bswap_32(as_u32c(src))); goto GETU_END; -getu_1230_0B21: sample = sx24(bswap_32(as_u32c(src) ^ 0x8000)); goto GETU_END; -getu_1234_1234: sample = as_u32c(src); goto GETU_END; -getu_1234_9234: sample = as_u32c(src) ^ 0x80000000; goto GETU_END; -getu_1234_4321: sample = bswap_32(as_u32c(src)); goto GETU_END; -getu_1234_C321: sample = bswap_32(as_u32c(src) ^ 0x80); goto GETU_END; -} -#endif - #ifdef PUT32F_LABELS /* type (0 = float, 1 = float64), endswap */ static void *const put32float_labels[2 * 2] = {

A commit fcd164e6229c ("Permit to PCM plug configuration to specify unchanged parameters. Added support for RT signals to async interface. Added ops for PCM mix.") added a pair of NORMS_LABELS/NORMS_END, however they have been no longer used.
This commit removes them in a point to reduce the amount of code to maintain. --- src/pcm/plugin_ops.h | 264 --------------------------------------------------- 1 file changed, 264 deletions(-)
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 1866d6d9..63e3c827 100644 --- a/src/pcm/plugin_ops.h +++ b/src/pcm/plugin_ops.h @@ -713,270 +713,6 @@ get32f_4321D_1234: tmp_double.l = bswap_64(as_u64c(src)); goto GET32F_END; #endif
-#ifdef NORMS_LABELS -static inline void _norms(const void *src, void *dst, - int src_wid, - int dst_sign, int dst_wid, int dst_end) -{ - int32_t s; - switch (src_wid) { - case 8: - s = *(int32_t*)src; - if (s >= 0x7f) - goto _max; - else if (s <= -0x80) - goto _min; - break; - case 16: - s = *(int32_t*)src; - if (s >= 0x7fff) - goto _max; - else if (s <= -0x8000) - goto _min; - break; - case 24: - s = *(int32_t*)src; - if (s >= 0x7fffff) - goto _max; - else if (s <= -0x800000) - goto _min; - break; - case 32: - { - int64_t s64; - s64 = *(int64_t*)src; - if (s64 >= 0x7fffffff) - goto _max; - else if (s64 <= -0x80000000) - goto _min; - s = s64; - break; - } - default: - assert(0); - return; - } - if (src_wid < dst_wid) { - unsigned int bits = dst_wid - src_wid; - s *= 1 << bits; - } else if (src_wid > dst_wid) { - unsigned int bits = src_wid - dst_wid; - s = (s + (1 << (bits - 1))) / (1 << bits); - } - if (!dst_sign) - s += (1U << (dst_wid - 1)); - switch (dst_wid) { - case 8: - *(uint8_t*)dst = s; - break; - case 16: - if (dst_end) - s = bswap_16(s); - *(uint16_t*)dst = s; - break; - case 24: - case 32: - if (dst_end) - s = bswap_32(s); - *(uint32_t*)dst = s; - break; - } - return; - - _min: - switch (dst_wid) { - case 8: - if (dst_sign) - *(uint8_t*)dst = 0x80; - else - *(uint8_t*)dst = 0; - break; - case 16: - if (dst_sign) - *(uint16_t*)dst = dst_end ? 0x0080 : 0x8000; - else - *(uint16_t*)dst = 0; - break; - case 24: - if (dst_sign) - *(uint32_t*)dst = dst_end ? 0x00008000 : 0x00800000; - else - *(uint32_t*)dst = 0; - break; - case 32: - if (dst_sign) - *(uint32_t*)dst = dst_end ? 0x00000080 : 0x80000000; - else - *(uint32_t*)dst = 0; - break; - default: - assert(0); - break; - } - return; - - _max: - switch (dst_wid) { - case 8: - if (dst_sign) - *(uint8_t*)dst = 0x7f; - else - *(uint8_t*)dst = 0xff; - break; - case 16: - if (dst_sign) - *(uint16_t*)dst = dst_end ? 0xff7f : 0x7fff; - else - *(uint16_t*)dst = 0; - break; - case 24: - if (dst_sign) - *(uint32_t*)dst = dst_end ? 0xffff7f00 : 0x007fffff; - else - *(uint32_t*)dst = 0; - break; - case 32: - if (dst_sign) - *(uint32_t*)dst = dst_end ? 0xffffff7f : 0x7fffffff; - else - *(uint32_t*)dst = 0; - break; - default: - assert(0); - break; - } - return; -} - -/* src_wid dst_sign dst_wid dst_end */ -static void *const norms_labels[4 * 2 * 4 * 2] = { - &&norms_8_u8, /* s8 -> u8 */ - &&norms_8_u8, /* s8 -> u8 */ - &&norms_8_u16h, /* s8 -> u16h */ - &&norms_8_u16s, /* s8 -> u16s */ - &&norms_8_u24h, /* s8 -> u24h */ - &&norms_8_u24s, /* s8 -> u24s */ - &&norms_8_u32h, /* s8 -> u32h */ - &&norms_8_u32s, /* s8 -> u32s */ - &&norms_8_s8, /* s8 -> s8 */ - &&norms_8_s8, /* s8 -> s8 */ - &&norms_8_s16h, /* s8 -> s16h */ - &&norms_8_s16s, /* s8 -> s16s */ - &&norms_8_s24h, /* s8 -> s24h */ - &&norms_8_s24s, /* s8 -> s24s */ - &&norms_8_s32h, /* s8 -> s32h */ - &&norms_8_s32s, /* s8 -> s32s */ - &&norms_16_u8, /* s16 -> u8 */ - &&norms_16_u8, /* s16 -> u8 */ - &&norms_16_u16h, /* s16 -> u16h */ - &&norms_16_u16s, /* s16 -> u16s */ - &&norms_16_u24h, /* s16 -> u24h */ - &&norms_16_u24s, /* s16 -> u24s */ - &&norms_16_u32h, /* s16 -> u32h */ - &&norms_16_u32s, /* s16 -> u32s */ - &&norms_16_s8, /* s16 -> s8 h*/ - &&norms_16_s8, /* s16 -> s8 */ - &&norms_16_s16h, /* s16 -> s16h */ - &&norms_16_s16s, /* s16 -> s16s */ - &&norms_16_s24h, /* s16 -> s24h */ - &&norms_16_s24s, /* s16 -> s24s */ - &&norms_16_s32h, /* s16 -> s32h */ - &&norms_16_s32s, /* s16 -> s32s */ - &&norms_24_u8, /* s24 -> u8 */ - &&norms_24_u8, /* s24 -> u8 */ - &&norms_24_u16h, /* s24 -> u16h */ - &&norms_24_u16s, /* s24 -> u16s */ - &&norms_24_u24h, /* s24 -> u24h */ - &&norms_24_u24s, /* s24 -> u24s */ - &&norms_24_u32h, /* s24 -> u32h */ - &&norms_24_u32s, /* s24 -> u32s */ - &&norms_24_s8, /* s24 -> s8 */ - &&norms_24_s8, /* s24 -> s8 */ - &&norms_24_s16h, /* s24 -> s16h */ - &&norms_24_s16s, /* s24 -> s16s */ - &&norms_24_s24h, /* s24 -> s24h */ - &&norms_24_s24s, /* s24 -> s24s */ - &&norms_24_s32h, /* s24 -> s32h */ - &&norms_24_s32s, /* s24 -> s32s */ - &&norms_32_u8, /* s32 -> u8 */ - &&norms_32_u8, /* s32 -> u8 */ - &&norms_32_u16h, /* s32 -> u16h */ - &&norms_32_u16s, /* s32 -> u16s */ - &&norms_32_u24h, /* s32 -> u24h */ - &&norms_32_u24s, /* s32 -> u24s */ - &&norms_32_u32h, /* s32 -> u32h */ - &&norms_32_u32s, /* s32 -> u32s */ - &&norms_32_s8, /* s32 -> s8 */ - &&norms_32_s8, /* s32 -> s8 */ - &&norms_32_s16h, /* s32 -> s16h */ - &&norms_32_s16s, /* s32 -> s16s */ - &&norms_32_s24h, /* s32 -> s24h */ - &&norms_32_s24s, /* s32 -> s24s */ - &&norms_32_s32h, /* s32 -> s32h */ - &&norms_32_s32s, /* s32 -> s32s */ -}; -#endif - -#ifdef NORMS_END -norms_8_u8: _norms(src, dst, 8, 0, 8, 0); goto NORMS_END; -norms_8_u16h: _norms(src, dst, 8, 0, 16, 0); goto NORMS_END; -norms_8_u16s: _norms(src, dst, 8, 0, 16, 1); goto NORMS_END; -norms_8_u24h: _norms(src, dst, 8, 0, 24, 0); goto NORMS_END; -norms_8_u24s: _norms(src, dst, 8, 0, 24, 1); goto NORMS_END; -norms_8_u32h: _norms(src, dst, 8, 0, 32, 0); goto NORMS_END; -norms_8_u32s: _norms(src, dst, 8, 0, 32, 1); goto NORMS_END; -norms_8_s8: _norms(src, dst, 8, 1, 8, 0); goto NORMS_END; -norms_8_s16h: _norms(src, dst, 8, 1, 16, 0); goto NORMS_END; -norms_8_s16s: _norms(src, dst, 8, 1, 16, 1); goto NORMS_END; -norms_8_s24h: _norms(src, dst, 8, 1, 24, 0); goto NORMS_END; -norms_8_s24s: _norms(src, dst, 8, 1, 24, 1); goto NORMS_END; -norms_8_s32h: _norms(src, dst, 8, 1, 32, 0); goto NORMS_END; -norms_8_s32s: _norms(src, dst, 8, 1, 32, 1); goto NORMS_END; -norms_16_u8: _norms(src, dst, 16, 0, 8, 0); goto NORMS_END; -norms_16_u16h: _norms(src, dst, 16, 0, 16, 0); goto NORMS_END; -norms_16_u16s: _norms(src, dst, 16, 0, 16, 1); goto NORMS_END; -norms_16_u24h: _norms(src, dst, 16, 0, 24, 0); goto NORMS_END; -norms_16_u24s: _norms(src, dst, 16, 0, 24, 1); goto NORMS_END; -norms_16_u32h: _norms(src, dst, 16, 0, 32, 0); goto NORMS_END; -norms_16_u32s: _norms(src, dst, 16, 0, 32, 1); goto NORMS_END; -norms_16_s8: _norms(src, dst, 16, 1, 8, 0); goto NORMS_END; -norms_16_s16h: _norms(src, dst, 16, 1, 16, 0); goto NORMS_END; -norms_16_s16s: _norms(src, dst, 16, 1, 16, 1); goto NORMS_END; -norms_16_s24h: _norms(src, dst, 16, 1, 24, 0); goto NORMS_END; -norms_16_s24s: _norms(src, dst, 16, 1, 24, 1); goto NORMS_END; -norms_16_s32h: _norms(src, dst, 16, 1, 32, 0); goto NORMS_END; -norms_16_s32s: _norms(src, dst, 16, 1, 32, 1); goto NORMS_END; -norms_24_u8: _norms(src, dst, 24, 0, 8, 0); goto NORMS_END; -norms_24_u16h: _norms(src, dst, 24, 0, 16, 0); goto NORMS_END; -norms_24_u16s: _norms(src, dst, 24, 0, 16, 1); goto NORMS_END; -norms_24_u24h: _norms(src, dst, 24, 0, 24, 0); goto NORMS_END; -norms_24_u24s: _norms(src, dst, 24, 0, 24, 1); goto NORMS_END; -norms_24_u32h: _norms(src, dst, 24, 0, 32, 0); goto NORMS_END; -norms_24_u32s: _norms(src, dst, 24, 0, 32, 1); goto NORMS_END; -norms_24_s8: _norms(src, dst, 24, 1, 8, 0); goto NORMS_END; -norms_24_s16h: _norms(src, dst, 24, 1, 16, 0); goto NORMS_END; -norms_24_s16s: _norms(src, dst, 24, 1, 16, 1); goto NORMS_END; -norms_24_s24h: _norms(src, dst, 24, 1, 24, 0); goto NORMS_END; -norms_24_s24s: _norms(src, dst, 24, 1, 24, 1); goto NORMS_END; -norms_24_s32h: _norms(src, dst, 24, 1, 32, 0); goto NORMS_END; -norms_24_s32s: _norms(src, dst, 24, 1, 32, 1); goto NORMS_END; -norms_32_u8: _norms(src, dst, 32, 0, 8, 0); goto NORMS_END; -norms_32_u16h: _norms(src, dst, 32, 0, 16, 0); goto NORMS_END; -norms_32_u16s: _norms(src, dst, 32, 0, 16, 1); goto NORMS_END; -norms_32_u24h: _norms(src, dst, 32, 0, 24, 0); goto NORMS_END; -norms_32_u24s: _norms(src, dst, 32, 0, 24, 1); goto NORMS_END; -norms_32_u32h: _norms(src, dst, 32, 0, 32, 0); goto NORMS_END; -norms_32_u32s: _norms(src, dst, 32, 0, 32, 1); goto NORMS_END; -norms_32_s8: _norms(src, dst, 32, 1, 8, 0); goto NORMS_END; -norms_32_s16h: _norms(src, dst, 32, 1, 16, 0); goto NORMS_END; -norms_32_s16s: _norms(src, dst, 32, 1, 16, 1); goto NORMS_END; -norms_32_s24h: _norms(src, dst, 32, 1, 24, 0); goto NORMS_END; -norms_32_s24s: _norms(src, dst, 32, 1, 24, 1); goto NORMS_END; -norms_32_s32h: _norms(src, dst, 32, 1, 32, 0); goto NORMS_END; -norms_32_s32s: _norms(src, dst, 32, 1, 32, 1); goto NORMS_END; -#endif - - #undef as_u8 #undef as_u16 #undef as_u32

A commit 16b3bf447c28 ('Enhanced bitmasks in PCM - added support for more formats by Takashi and me') adds support for some cases of linear interpolation of PCM samples, however some of added comments are not proper. This commit fixes them. --- src/pcm/plugin_ops.h | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/pcm/plugin_ops.h b/src/pcm/plugin_ops.h index 63e3c827..6392073c 100644 --- a/src/pcm/plugin_ops.h +++ b/src/pcm/plugin_ops.h @@ -593,14 +593,14 @@ static void *const put32_labels[5 * 2 * 2 + 4 * 3] = { &&put32_1234_923, /* 32h ^> 24h */ &&put32_1234_321, /* 32h -> 24s */ &&put32_1234_329, /* 32h ^> 24s */ - &&put32_1234_123_20, /* 32h -> 24h */ - &&put32_1234_923_20, /* 32h ^> 24h */ - &&put32_1234_321_20, /* 32h -> 24s */ - &&put32_1234_329_20, /* 32h ^> 24s */ - &&put32_1234_123_18, /* 32h -> 24h */ - &&put32_1234_923_18, /* 32h ^> 24h */ - &&put32_1234_321_18, /* 32h -> 24s */ - &&put32_1234_329_18, /* 32h ^> 24s */ + &&put32_1234_123_20, /* 32h -> 20h */ + &&put32_1234_923_20, /* 32h ^> 20h */ + &&put32_1234_321_20, /* 32h -> 20s */ + &&put32_1234_329_20, /* 32h ^> 20s */ + &&put32_1234_123_18, /* 32h -> 18h */ + &&put32_1234_923_18, /* 32h ^> 18h */ + &&put32_1234_321_18, /* 32h -> 18s */ + &&put32_1234_329_18, /* 32h ^> 18s */ }; #endif

On Wed, 20 Dec 2017 18:16:59 +0100, Takashi Sakamoto wrote:
Hi,
When reviewing Maciej's patches[1], I found that alsa-lib includes some unused codes in linear interpolation functionality. Furthermore, some code comments includes 'copy & paste' mistakes.
This patchset improves them.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129370.ht...
Takashi Sakamoto (4): pcm: remove unused macros of COPY_LABELS/COPY_END pcm: remove unused macros of GETU_LABELS/GETU_END pcm: remove unused macros of NORMS_LABELS/NORMS_END pcm: fix wrong comments for some cases of linear interpolation of PCM samples
All patches seem missing your sign-off. It's not mandatory for alsa-lib, but preferable. Would you like me adding them manually?
thanks,
Takashi

Hi,
On Dec 21 2017 23:46, Takashi Iwai wrote:
On Wed, 20 Dec 2017 18:16:59 +0100, Takashi Sakamoto wrote:
Hi,
When reviewing Maciej's patches[1], I found that alsa-lib includes some unused codes in linear interpolation functionality. Furthermore, some code comments includes 'copy & paste' mistakes.
This patchset improves them.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129370.ht...
Takashi Sakamoto (4): pcm: remove unused macros of COPY_LABELS/COPY_END pcm: remove unused macros of GETU_LABELS/GETU_END pcm: remove unused macros of NORMS_LABELS/NORMS_END pcm: fix wrong comments for some cases of linear interpolation of PCM samples
All patches seem missing your sign-off. It's not mandatory for alsa-lib, but preferable. Would you like me adding them manually?
Oops... Yes, please. (I should have a script to check it before posting...)
Thanks
Takashi Sakamoto

On Dec 22 2017 08:25, Takashi Sakamoto wrote:
Hi,
On Dec 21 2017 23:46, Takashi Iwai wrote:
On Wed, 20 Dec 2017 18:16:59 +0100, Takashi Sakamoto wrote:
Hi,
When reviewing Maciej's patches[1], I found that alsa-lib includes some unused codes in linear interpolation functionality. Furthermore, some code comments includes 'copy & paste' mistakes.
This patchset improves them.
[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2017-December/129370.ht...
Takashi Sakamoto (4): pcm: remove unused macros of COPY_LABELS/COPY_END pcm: remove unused macros of GETU_LABELS/GETU_END pcm: remove unused macros of NORMS_LABELS/NORMS_END pcm: fix wrong comments for some cases of linear interpolation of PCM samples
All patches seem missing your sign-off. It's not mandatory for alsa-lib, but preferable. Would you like me adding them manually?
Oops... Yes, please. (I should have a script to check it before posting...)
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
Regards
Takashi Sakamoto
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto