On Mon, 2017-08-21 at 10:25 +0300, Arkadiusz Hiler wrote: > 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@xxxxxxxxxxxxxx > > > m> > > > --- > > > 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. Note that this series is not tied to the chamelium at all: it requires an HDMI-VGA adapter with audio out instead of a chamelium. For this reason, it wouldn't make sense to tie it to --enable-chamelium. The problem remains the same though: the audio part (patch 1/3) requires the GSL library and the alsa part requires the alsa lib and we need to decide how to handle the dependencies. I think what would make the most sense is to have an --enable-audio parameter (referring to the newly-introduced audio test, not the audio library) that requires the alsa and gsl libraries. Then, when audio support is added to the chamelium code, the alsa library should become a dependency for --enable-chamelium as well. For the latter, it would be consistent with the idea that --enable- chamelium pulls-in all possible chamelium tests and thus requires all their dependencies, instead of only enabling the tests for which dependencies are installed. Does that seem agreeable? -- Paul Kocialkowski <paul.kocialkowski@xxxxxxxxxxxxxxx> Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo, Finland _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
