Hey Peter, On Sun, 2012-03-11 at 19:12 +0100, Peter Meerwald wrote: > Hello Arun, > > thank you for testing; you are covering areas I have not thought of :) > > > > v2: > > > * add ARM NEON stereo-to-mono remapping code > > > * static __attribute__ ((noinline)) is necessary to prevent inlining and > > > work around gcc 4.6 ICE, see https://bugs.launchpad.net/bugs/936863 > > > * call test code, the reference implementation is obtained using > > > pa_get_init_remap_func() > > > * remove check for NEON flags > > > v1: > > > * ARM NEON mono-to-stereo remapping code > > > A couple of issues here. Firstly, if I turn up the test loop count to > > 100000, I can fairly reliably see a bunch of failures like the one > > below. > > I have not been able to reproduce the issue so far; my setup is beagle-xm > with softfp, the particular compiler might also be different FWIW, you can get a Gentoo rootfs (stage3) snapshot at -- http://gentoo.osuosl.org/releases/arm/autobuilds/ > you get different results for remap_stereo_to_mono, s16 > > are you testing the patches one-by-one? i.e. you have not yet applied > '[PATCH 5/6] core: add stereo to mono special case remapping'? I only pulled patches 1 through 4 for the first round of testing. > I think there is an issue in run_test_s16_stereo_to_mono() when setting up > the remap structure: > remap.format = &sf; > iss.format = PA_SAMPLE_S16NE; > iss.channels = 2; > oss.format = PA_SAMPLE_S16NE; > oss.channels = 1; > remap.i_ss = &iss; > remap.o_ss = &oss; > remap.map_table_f[0][0] = 1.0; > remap.map_table_f[0][1] = 1.0; > remap_init_func(&remap); > > what is missing is the map table for int values: > remap.map_table_i[0][0] = 0x10000; > remap.map_table_i[0][1] = 0x10000; > > in run_test_s16_mono_to_stereo() there is a similar issue, but here > init_remap_c() in remap.c will use the remap_mono_to_stereo_c instead of > the generic mapper; remap_mono_to_stereo_c does not need the map_table > > in '[PATCH 5/6] core: add stereo to mono special case remapping' a special > case is added to init_remap_c() to cover the stereo-to-mono case which > might mask the issue > > ... but I am just speculating here :( > > could you try above initialization? Will do this and get back to you. > > Next, I see the reference implementation doing better in the > > mono-to-stereo float remapping. > > the C code seems a lot more efficient on the panda, it is well known that > floats are handled better > > NEON remap_mono_to_stereo(float) is ~ 4x slower on panda, and only > slightly faster on beagle > > remap_mono_to_stereo(s16) is ~ 1.5x slower on panda, and somewhat faster > on beagle > > I'll try to build a hardfp system and see if the runtime can be improved > (or at least avoid regressions) I'll also try to get another rootfs up for my IGEPv2 board which is OMAP3-based. Given that this is a complicated change, I'd like to push merging this to after 2.0. Unfortunately I've not had enough free time to look at your patches for a while, and I'm sorry about the delay. Hopefully we can iron out the kinks and get this merged early in the 3.0 cycle. Regards, Arun
