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

[Peter Meerwald]

Hello Alexander,

thank you for taking time to look at the patches!

> > +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...
 
> ...but not here. Is there any particular reason for this?

no, just my laziness; will be fixed
 
> 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?

at some point a type cast has to happen; 
probably the function implementation should have non-void pointer types, 
and the function pointer expose void pointers?
let's stick to that 
 
> 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.

there is nothing MMX or SSE related, just reorganizing code

> 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.

makes sense, will reorder

> 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.

benchmark results are in the commit messages for ARM and x86-64, I have 
not tried 32bit

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)