[Intel-gfx] [PATCH i-g-t 2/3] lib: Add ALSA library with dedicated helpers

[Arkadiusz Hiler]

On Fri, Aug 18, 2017 at 12:46:38PM +0300, Arkadiusz Hiler wrote:
> On Thu, Aug 17, 2017 at 07:05:56PM +0300, Paul Kocialkowski wrote:
> > This introduces an ALSA library, with dedicated helpers for handling
> > playback and capture. It handles ALSA device identification and
> > configuration as well as a run loop with callback mechanisms for feeding
> > output data and handling input data.
> > 
> > This library paves the way for testing audio going through display
> > connectors, such as HDMI.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski at linux.intel.com>
> > ---
> >  configure.ac                                       |   3 +
> >  .../intel-gpu-tools/intel-gpu-tools-docs.xml       |   1 +
> >  lib/Makefile.am                                    |   7 +
> >  lib/igt.h                                          |   1 +
> >  lib/igt_alsa.c                                     | 624 +++++++++++++++++++++
> >  lib/igt_alsa.h                                     |  60 ++
> >  6 files changed, 696 insertions(+)
> >  create mode 100644 lib/igt_alsa.c
> >  create mode 100644 lib/igt_alsa.h
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 50aa86b5..e66273a4 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -219,6 +219,9 @@ if test "x$enable_chamelium" = xyes; then
> >  	AC_DEFINE(HAVE_CHAMELIUM, 1, [Enable Chamelium support])
> >  fi
> >  
> > +PKG_CHECK_MODULES(ALSA, [alsa], [alsa=yes], [alsa=no])
> > +AM_CONDITIONAL(HAVE_ALSA, [test "x$alsa" = xyes])
> 
> please mention the new dependency in the README

One more thing that came to my mind - how do we want to have this new
dependency handled configure-time?

I see three options:

1. Having it as optional component, so after we --enable-chamelium we can
end up with the alsa part either being there or not.

2. Having to explicitly enable it, so that only with --enable-alsa we
will have this new, shiny feature (and a nice ./configure-time fail).

3. Having to explicitly disable it, i.e. --enable-chamelium enables ALSA
check, if it's not there configure fails and we can explicitly disable
it, resulting in --enable-chamelium --disable-alsa switch combo.

As a rule of thumb, anything similar to option #1 should be avoided to
not suprise people with missing features depending on machine they
compile on - unless we collectively decide that having optional
component is okay (e.g. in case we have a fallback so it doesn't matter
that much).

IMO in this case we should go with option #2.

-- 
Cheers,
Arek