[pulseaudio-discuss] [PATCH 05/11] remap: Split remapping functions into s16 and float implementation

[Alexander E. Patrakov]

24.04.2014 22:09, Peter Meerwald wrote:
> From: Peter Meerwald <p.meerwald at bct-electronic.com>
>
> The sample format is known when the remap structure is initialized,
> no runtime decision needed.

> -static void remap_mono_to_stereo_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {
> +static void remap_mono_to_stereo_s16ne_c(pa_remap_t *m, int16_t *dst, const int16_t *src, unsigned n) {
...
> +static void remap_mono_to_stereo_float32ne_c(pa_remap_t *m, float *dst, const float *src, unsigned n) {

I see that here you have changed the prototype here and made the 
resulting two functions more type-safe...

> -static void remap_channels_matrix_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {
> +static void remap_channels_matrix_s16ne_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {
...
> +static void remap_channels_matrix_float32ne_c(pa_remap_t *m, void *dst, const void *src, unsigned n) {

...but not here. Is there any particular reason for this?

On the same topic, in patch 09/11, you use the same pa_do_remap_func_t 
type for func_s16 and func_float in the pa_set_remap_func prototype. 
This leads to typecasts in callers such as init_remap_c(). Maybe it 
would be better to introduce different types for function pointers for 
the s16 and float cases?

And sorry, I skipped the MMX and SSE-related parts of the patch while 
reviewing it, due to not having the necessary knowledge for the review.

Patches 01-04, 06, 08 are just equivalent refactoring or adding a good 
function and thus look good to me. 07 also looks good, but IMHO 
misplaced, putting it just before 10 (the first user of state) would be 
IMHO more logical.

I have also looked at patches 10 and 11, but I would like to take a 
second look tomorrow. I have not done any runtime testing yet.

-- 
Alexander E. Patrakov