Hello David,
> > The remapper and channel mixing code have (faster) specialized and (slower)
> > generic code certain code path. The flag force_generic_code can be set to
> > force the generic code path which is useful for testing. Code duplication
> > (such as in mix-special-test) can be avoided, cleanup patches follow.
> >
> > Signed-off-by: Peter Meerwald <pmeerw at pmeerw.net>
> > ---
> > src/Makefile.am | 2 +-
> > src/daemon/main.c | 4 +++-
> > src/pulsecore/cpu.c | 28 ++++++++++++++++++++++++++++
> > src/pulsecore/cpu.h | 5 +++++
> > src/pulsecore/mix.c | 8 ++++++++
> > src/pulsecore/remap.c | 13 +++++++++++++
> > 6 files changed, 58 insertions(+), 2 deletions(-)
> > create mode 100644 src/pulsecore/cpu.c
> >
> > diff --git a/src/Makefile.am b/src/Makefile.am
> > index 51ef690..634e26b 100644
> > --- a/src/Makefile.am
> > +++ b/src/Makefile.am
> > @@ -897,7 +897,7 @@ libpulsecore_ at PA_MAJORMINOR@_la_SOURCES = \
> > pulsecore/rtpoll.c pulsecore/rtpoll.h \
> > pulsecore/stream-util.c pulsecore/stream-util.h \
> > pulsecore/mix.c pulsecore/mix.h \
> > - pulsecore/cpu.h \
> > + pulsecore/cpu.c pulsecore/cpu.h \
> > pulsecore/cpu-arm.c pulsecore/cpu-arm.h \
> > pulsecore/cpu-x86.c pulsecore/cpu-x86.h \
> > pulsecore/cpu-orc.c pulsecore/cpu-orc.h \
> > diff --git a/src/daemon/main.c b/src/daemon/main.c
> > index 02a8ea6..74527be 100644
> > --- a/src/daemon/main.c
> > +++ b/src/daemon/main.c
> > @@ -1050,13 +1050,15 @@ int main(int argc, char *argv[]) {
> > #endif
> >
> > c->cpu_info.cpu_type = PA_CPU_UNDEFINED;
> > + c->cpu_info.force_generic_code = false; /* use generic, slow code */
>
> Are you sure this comment is correct? It looks like the opposite.
the comment is not very clear at least; it describes the purpose of the
flag, not the assignment
replacing it with
/* don't force generic code, used for testing only */
in v2
> Also, what is the difference between setting force_generic_code and setting
> PULSE_NO_SIMD?
mixing and remapping as generic and special-case code; the flag can be
set to force use of the generic code
this is useful for the test code so that special-case code can be compared
with the (presumably correct) generic code -- it saves quite a lot of code
in the test suits (the alternative would be to expose the internal
mixing/remapping function); special-case code path are beneficial on all
CPUs
PULSE_NO_SIMD suppresses CPU-specific code path
thanks, p.
> > if (!getenv("PULSE_NO_SIMD")) {
> > if (pa_cpu_init_x86(&(c->cpu_info.flags.x86)))
> > c->cpu_info.cpu_type = PA_CPU_X86;
> > - if (pa_cpu_init_arm(&(c->cpu_info.flags.arm)))
> > + else if (pa_cpu_init_arm(&(c->cpu_info.flags.arm)))
> > c->cpu_info.cpu_type = PA_CPU_ARM;
> > pa_cpu_init_orc(c->cpu_info);
> > }
> > + pa_cpu_init(c->cpu_info);
>
> Now that we have a cpu.c, I think we should move all pa_cpu_init_* functions
> there and encapsulate them so that only pa_cpu_init is visible from the
> outside. I e, move the entire block qouted above to cpu.c.
good idea, doing so in v2
> >
> > pa_assert_se(pa_signal_init(pa_mainloop_get_api(mainloop)) == 0);
> > pa_signal_new(SIGINT, signal_callback, c);
> > diff --git a/src/pulsecore/cpu.c b/src/pulsecore/cpu.c
> > new file mode 100644
> > index 0000000..659bfdf
> > --- /dev/null
> > +++ b/src/pulsecore/cpu.c
> > @@ -0,0 +1,28 @@
> > +/***
> > + This file is part of PulseAudio.
> > +
> > + Copyright 2014 Peter Meerwald <pmeerw at pmeerw.net>
> > +
> > + PulseAudio is free software; you can redistribute it and/or modify
> > + it under the terms of the GNU Lesser General Public License as published
> > + by the Free Software Foundation; either version 2.1 of the License,
> > + or (at your option) any later version.
> > +
> > + PulseAudio is distributed in the hope that it will be useful, but
> > + WITHOUT ANY WARRANTY; without even the implied warranty of
> > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + General Public License for more details.
> > +***/
> > +
> > +#ifdef HAVE_CONFIG_H
> > +#include <config.h>
> > +#endif
> > +
> > +#include "cpu.h"
> > +
> > +bool pa_cpu_init(pa_cpu_info cpu_info) {
> > + pa_remap_func_init(cpu_info);
> > + pa_mix_func_init(cpu_info);
> > +
> > + return false;
> > +}
> > diff --git a/src/pulsecore/cpu.h b/src/pulsecore/cpu.h
> > index 7fe6f0b..9c931f7 100644
> > --- a/src/pulsecore/cpu.h
> > +++ b/src/pulsecore/cpu.h
> > @@ -40,6 +40,11 @@ struct pa_cpu_info {
> > pa_cpu_x86_flag_t x86;
> > pa_cpu_arm_flag_t arm;
> > } flags;
> > + bool force_generic_code;
> > };
> >
> > +bool pa_cpu_init(pa_cpu_info cpu_info);
> > +void pa_remap_func_init(pa_cpu_info cpu_info);
> > +void pa_mix_func_init(pa_cpu_info cpu_info);
> > +
> > #endif /* foocpuhfoo */
> > diff --git a/src/pulsecore/mix.c b/src/pulsecore/mix.c
> > index 4b789a6..f614138 100644
> > --- a/src/pulsecore/mix.c
> > +++ b/src/pulsecore/mix.c
> > @@ -32,6 +32,7 @@
> > #include <pulsecore/g711.h>
> > #include <pulsecore/endianmacros.h>
> >
> > +#include "cpu.h"
> > #include "mix.h"
> >
> > #define VOLUME_PADDING 32
> > @@ -609,6 +610,13 @@ static pa_do_mix_func_t do_mix_table[] = {
> > [PA_SAMPLE_S24_32RE] = (pa_do_mix_func_t) pa_mix_s24_32re_c
> > };
> >
> > +void pa_mix_func_init(pa_cpu_info cpu_info) {
> > + if (cpu_info.force_generic_code)
> > + do_mix_table[PA_SAMPLE_S16NE] = (pa_do_mix_func_t)
> > pa_mix_generic_s16ne;
> > + else
> > + do_mix_table[PA_SAMPLE_S16NE] = (pa_do_mix_func_t) pa_mix_s16ne_c;
> > +}
> > +
> > size_t pa_mix(
> > pa_mix_info streams[],
> > unsigned nstreams,
> > diff --git a/src/pulsecore/remap.c b/src/pulsecore/remap.c
> > index adff2a5..a62ec91 100644
> > --- a/src/pulsecore/remap.c
> > +++ b/src/pulsecore/remap.c
> > @@ -32,6 +32,7 @@
> > #include <pulsecore/log.h>
> > #include <pulsecore/macro.h>
> >
> > +#include "cpu.h"
> > #include "remap.h"
> >
> > static void remap_mono_to_stereo_s16ne_c(pa_remap_t *m, int16_t *dst,
> > const int16_t *src, unsigned n) {
> > @@ -342,6 +343,8 @@ void pa_set_remap_func(pa_remap_t *m, pa_do_remap_func_t
> > func_s16,
> > pa_assert_not_reached();
> > }
> >
> > +static bool force_generic_code = false;
> > +
> > /* set the function that will execute the remapping based on the matrices
> > */
> > static void init_remap_c(pa_remap_t *m) {
> > unsigned n_oc, n_ic;
> > @@ -351,6 +354,12 @@ static void init_remap_c(pa_remap_t *m) {
> > n_ic = m->i_ss.channels;
> >
> > /* find some common channel remappings, fall back to full matrix
> > operation. */
> > + if (force_generic_code) {
> > + pa_log_info("Forced to use generic matrix remapping");
> > + pa_set_remap_func(m, remap_channels_matrix_s16ne_c,
> > remap_channels_matrix_float32ne_c);
> > + return;
> > + }
> > +
> > if (n_ic == 1 && n_oc == 2 &&
> > m->map_table_i[0][0] == 0x10000 && m->map_table_i[1][0] ==
> > 0x10000) {
> >
> > @@ -423,3 +432,7 @@ pa_init_remap_func_t pa_get_init_remap_func(void) {
> > void pa_set_init_remap_func(pa_init_remap_func_t func) {
> > init_remap_func = func;
> > }
> > +
> > +void pa_remap_func_init(pa_cpu_info cpu_info) {
> > + force_generic_code = cpu_info.force_generic_code;
> > +}
> >
>
>
--
Peter Meerwald
+43-664-2444418 (mobile)
