On Mon, Aug 21, 2017 at 12:39:58PM +0300, Paul Kocialkowski wrote: > 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. Ow, right. I've missed that. Thanks for clarification! > 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? LGTM, carry on! :-) _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
