Re: [Intel-gfx] [PATCH i-g-t 3/3] tests: Introduce audio tests, starting with HDMI signal integrity

[Lyude Paul]

On Mon, 2017-08-21 at 18:14 +0300, Paul Kocialkowski wrote:
> Hi,
> 
> On Fri, 2017-08-18 at 12:15 -0400, Lyude Paul wrote:
> > Nice job! Only one small formatting change
> 
> Thanks a lot for the timely review! I only have a week and a half
> left
> at my internship, so it is much appreciated :)
> 
> > On Thu, 2017-08-17 at 19:05 +0300, Paul Kocialkowski wrote:
> > > This introduces a new test for audio going through display
> > > connectors.
> > > It currently contains a single subtest for HDMI signal integrity,
> > > but
> > > other test cases will be added later on.
> > > 
> > > The test setup consists in using an HDMI-VGA bridge that
> > > separates
> > > the
> > > audio out (via a 3.5 mm jack) and feeding this back to the DUT's
> > > line-in
> > > where it can be recorded by ALSA with controls correctly
> > > configured.
> > > 
> > > The audio test makes use of the audio and ALSA igt libraries
> > > helpers.
> > > 
> > > Signed-off-by: Paul Kocialkowski  > > om>
> > > ---
> > >  tests/Makefile.am |  12 
> > >  tests/audio.c | 170
> > > ++
> > >  2 files changed, 182 insertions(+)
> > >  create mode 100644 tests/audio.c
> > > 
> > > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > > index f9d11e6c..471f3818 100644
> > > --- a/tests/Makefile.am
> > > +++ b/tests/Makefile.am
> > > @@ -20,6 +20,15 @@ TESTS_progs += \
> > >   $(NULL)
> > >  endif
> > >  
> > > +if HAVE_ALSA
> > > +if HAVE_GSL
> > > +TESTS_progs += \
> > > + audio \
> > > + $(NULL)
> > > +endif
> > > +endif
> > > +
> > > +
> > >  if BUILD_TESTS
> > >  test-list.txt: Makefile.sources
> > >   @echo TESTLIST > $@
> > > @@ -134,6 +143,9 @@ vc4_wait_seqno_LDADD = $(LDADD)
> > > $(DRM_VC4_LIBS)
> > >  chamelium_CFLAGS = $(AM_CFLAGS) $(XMLRPC_CFLAGS)
> > > $(LIBUDEV_CFLAGS)
> > >  chamelium_LDADD = $(LDADD) $(XMLRPC_LIBS) $(LIBUDEV_LIBS)
> > >  
> > > +audio_CFLAGS = $(AM_CFLAGS) $(ALSA_CFLAGS)
> > > +audio_LDADD = $(LDADD) $(ALSA_LIBS)
> > > +
> > >  amdgpu_amd_basic_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
> > >  amdgpu_amd_basic_LDADD = $(LDADD) $(DRM_AMDGPU_LIBS)
> > >  amdgpu_amd_cs_nop_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
> > > diff --git a/tests/audio.c b/tests/audio.c
> > > new file mode 100644
> > > index ..7099dd99
> > > --- /dev/null
> > > +++ b/tests/audio.c
> > > @@ -0,0 +1,170 @@
> > > +/*
> > > + * Copyright © 2017 Intel Corporation
> > > + *
> > > + * Permission is hereby granted, free of charge, to any person
> > > obtaining a
> > > + * copy of this software and associated documentation files (the
> > > "Software"),
> > > + * to deal in the Software without restriction, including
> > > without
> > > limitation
> > > + * the rights to use, copy, modify, merge, publish, distribute,
> > > sublicense,
> > > + * and/or sell copies of the Software, and to permit persons to
> > > whom
> > > the
> > > + * Software is furnished to do so, subject to the following
> > > conditions:
> > > + *
> > > + * The above copyright notice and this permission notice
> > > (including
> > > the next
> > > + * paragraph) shall be included in all copies or substantial
> > > portions of the
> > > + * Software.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> > > KIND,
> > > EXPRESS OR
> > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > > MERCHANTABILITY,
> > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > > EVENT SHALL
> > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > > DAMAGES
> > > OR OTHER
> > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> > > OTHERWISE,
> > > ARISING
> > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > > OTHER DEALINGS
> > > + * IN THE SOFTWARE.
> > > + *
> > > + * Authors:
> > > + *  Paul Kocialkowski 
> > > + */
> > > +
> > > +#include "config.h"
> > > +#include "igt.h"
> > > +
> > > +#define PLAYBACK_CHANNELS2
> > > +#define PLAYBACK_FRAMES  1024
> > > +
> > > +#define CAPTURE_SAMPLE_RATE  48000
> > > +#define CAPTURE_CHANNELS 2
> > > +#define CAPTURE_DEVICE_NAME  "default"
> > > +#define CAPTURE_FRAMES   2048
> > 
> > Might want to fix the indenting here
> 
> cat, git and my editor show them aligned, so I think this is due to
> patch formating+email.
Figured as much. Since that only leaves the printf attribute (it's not
used anywhere else, but I still think it's a good idea to have for the
day this code gets changed again) I'll just make that small change
myself and push the series. Thanks for the great work you've been
doing!
> 
> > > +
> > > +#define RUN_TIMEOUT  2000
> > > +
> > > +struct test_data {
> > > + struct alsa *alsa;
> > > + struct audio_signal *signal;
> > > +
> > > + int streak;
> > > +};
> > > +
> > > +static int sampling_rates[] = {
> > > + 32000,
> > > + 44100,
> > > + 48000,
> > > + 88200,
> > 

Re: [Intel-gfx] [PATCH i-g-t 3/3] tests: Introduce audio tests, starting with HDMI signal integrity

[Paul Kocialkowski]

Hi,

On Fri, 2017-08-18 at 12:15 -0400, Lyude Paul wrote:
> Nice job! Only one small formatting change

Thanks a lot for the timely review! I only have a week and a half left
at my internship, so it is much appreciated :)

> On Thu, 2017-08-17 at 19:05 +0300, Paul Kocialkowski wrote:
> > This introduces a new test for audio going through display
> > connectors.
> > It currently contains a single subtest for HDMI signal integrity,
> > but
> > other test cases will be added later on.
> > 
> > The test setup consists in using an HDMI-VGA bridge that separates
> > the
> > audio out (via a 3.5 mm jack) and feeding this back to the DUT's
> > line-in
> > where it can be recorded by ALSA with controls correctly configured.
> > 
> > The audio test makes use of the audio and ALSA igt libraries
> > helpers.
> > 
> > Signed-off-by: Paul Kocialkowski 
> > ---
> >  tests/Makefile.am |  12 
> >  tests/audio.c | 170
> > ++
> >  2 files changed, 182 insertions(+)
> >  create mode 100644 tests/audio.c
> > 
> > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > index f9d11e6c..471f3818 100644
> > --- a/tests/Makefile.am
> > +++ b/tests/Makefile.am
> > @@ -20,6 +20,15 @@ TESTS_progs += \
> > $(NULL)
> >  endif
> >  
> > +if HAVE_ALSA
> > +if HAVE_GSL
> > +TESTS_progs += \
> > +   audio \
> > +   $(NULL)
> > +endif
> > +endif
> > +
> > +
> >  if BUILD_TESTS
> >  test-list.txt: Makefile.sources
> > @echo TESTLIST > $@
> > @@ -134,6 +143,9 @@ vc4_wait_seqno_LDADD = $(LDADD) $(DRM_VC4_LIBS)
> >  chamelium_CFLAGS = $(AM_CFLAGS) $(XMLRPC_CFLAGS) $(LIBUDEV_CFLAGS)
> >  chamelium_LDADD = $(LDADD) $(XMLRPC_LIBS) $(LIBUDEV_LIBS)
> >  
> > +audio_CFLAGS = $(AM_CFLAGS) $(ALSA_CFLAGS)
> > +audio_LDADD = $(LDADD) $(ALSA_LIBS)
> > +
> >  amdgpu_amd_basic_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
> >  amdgpu_amd_basic_LDADD = $(LDADD) $(DRM_AMDGPU_LIBS)
> >  amdgpu_amd_cs_nop_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
> > diff --git a/tests/audio.c b/tests/audio.c
> > new file mode 100644
> > index ..7099dd99
> > --- /dev/null
> > +++ b/tests/audio.c
> > @@ -0,0 +1,170 @@
> > +/*
> > + * Copyright © 2017 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > obtaining a
> > + * copy of this software and associated documentation files (the
> > "Software"),
> > + * to deal in the Software without restriction, including without
> > limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > sublicense,
> > + * and/or sell copies of the Software, and to permit persons to
> > whom
> > the
> > + * Software is furnished to do so, subject to the following
> > conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > the next
> > + * paragraph) shall be included in all copies or substantial
> > portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> > EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> > EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> > DAMAGES
> > OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + *
> > + * Authors:
> > + *  Paul Kocialkowski 
> > + */
> > +
> > +#include "config.h"
> > +#include "igt.h"
> > +
> > +#define PLAYBACK_CHANNELS  2
> > +#define PLAYBACK_FRAMES1024
> > +
> > +#define CAPTURE_SAMPLE_RATE48000
> > +#define CAPTURE_CHANNELS   2
> > +#define CAPTURE_DEVICE_NAME"default"
> > +#define CAPTURE_FRAMES 2048
> 
> Might want to fix the indenting here

cat, git and my editor show them aligned, so I think this is due to
patch formating+email.

> > +
> > +#define RUN_TIMEOUT2000
> > +
> > +struct test_data {
> > +   struct alsa *alsa;
> > +   struct audio_signal *signal;
> > +
> > +   int streak;
> > +};
> > +
> > +static int sampling_rates[] = {
> > +   32000,
> > +   44100,
> > +   48000,
> > +   88200,
> > +   96000,
> > +   176400,
> > +   192000,
> > +};
> > +
> > +static int sampling_rates_count = sizeof(sampling_rates) /
> > sizeof(int);
> > +
> > +static int test_frequencies[] = {
> > +   300,
> > +   600,
> > +   1200,
> > +   8,
> > +   1,
> > +};
> > +
> > +static int test_frequencies_count = sizeof(test_frequencies) /
> > sizeof(int);
> > +
> > +static int output_callback(void *data, short *buffer, int frames)
> > +{
> > +   struct test_data *test_data = (struct test_data *) data;
> > +
> > +   audio_signal_fill(test_data->signal, buffer, frames);
> > +
> > +   return 0;
> > +}
> > +
> > +static int input_callback(void *data, 

Re: [Intel-gfx] [PATCH i-g-t 3/3] tests: Introduce audio tests, starting with HDMI signal integrity

[Lyude Paul]

Nice job! Only one small formatting change

On Thu, 2017-08-17 at 19:05 +0300, Paul Kocialkowski wrote:
> This introduces a new test for audio going through display
> connectors.
> It currently contains a single subtest for HDMI signal integrity, but
> other test cases will be added later on.
> 
> The test setup consists in using an HDMI-VGA bridge that separates
> the
> audio out (via a 3.5 mm jack) and feeding this back to the DUT's
> line-in
> where it can be recorded by ALSA with controls correctly configured.
> 
> The audio test makes use of the audio and ALSA igt libraries helpers.
> 
> Signed-off-by: Paul Kocialkowski 
> ---
>  tests/Makefile.am |  12 
>  tests/audio.c | 170
> ++
>  2 files changed, 182 insertions(+)
>  create mode 100644 tests/audio.c
> 
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index f9d11e6c..471f3818 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -20,6 +20,15 @@ TESTS_progs += \
>   $(NULL)
>  endif
>  
> +if HAVE_ALSA
> +if HAVE_GSL
> +TESTS_progs += \
> + audio \
> + $(NULL)
> +endif
> +endif
> +
> +
>  if BUILD_TESTS
>  test-list.txt: Makefile.sources
>   @echo TESTLIST > $@
> @@ -134,6 +143,9 @@ vc4_wait_seqno_LDADD = $(LDADD) $(DRM_VC4_LIBS)
>  chamelium_CFLAGS = $(AM_CFLAGS) $(XMLRPC_CFLAGS) $(LIBUDEV_CFLAGS)
>  chamelium_LDADD = $(LDADD) $(XMLRPC_LIBS) $(LIBUDEV_LIBS)
>  
> +audio_CFLAGS = $(AM_CFLAGS) $(ALSA_CFLAGS)
> +audio_LDADD = $(LDADD) $(ALSA_LIBS)
> +
>  amdgpu_amd_basic_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
>  amdgpu_amd_basic_LDADD = $(LDADD) $(DRM_AMDGPU_LIBS)
>  amdgpu_amd_cs_nop_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
> diff --git a/tests/audio.c b/tests/audio.c
> new file mode 100644
> index ..7099dd99
> --- /dev/null
> +++ b/tests/audio.c
> @@ -0,0 +1,170 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person
> obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom
> the
> + * Software is furnished to do so, subject to the following
> conditions:
> + *
> + * The above copyright notice and this permission notice (including
> the next
> + * paragraph) shall be included in all copies or substantial
> portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES
> OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *  Paul Kocialkowski 
> + */
> +
> +#include "config.h"
> +#include "igt.h"
> +
> +#define PLAYBACK_CHANNELS2
> +#define PLAYBACK_FRAMES  1024
> +
> +#define CAPTURE_SAMPLE_RATE  48000
> +#define CAPTURE_CHANNELS 2
> +#define CAPTURE_DEVICE_NAME  "default"
> +#define CAPTURE_FRAMES   2048
Might want to fix the indenting here
> +
> +#define RUN_TIMEOUT  2000
> +
> +struct test_data {
> + struct alsa *alsa;
> + struct audio_signal *signal;
> +
> + int streak;
> +};
> +
> +static int sampling_rates[] = {
> + 32000,
> + 44100,
> + 48000,
> + 88200,
> + 96000,
> + 176400,
> + 192000,
> +};
> +
> +static int sampling_rates_count = sizeof(sampling_rates) /
> sizeof(int);
> +
> +static int test_frequencies[] = {
> + 300,
> + 600,
> + 1200,
> + 8,
> + 1,
> +};
> +
> +static int test_frequencies_count = sizeof(test_frequencies) /
> sizeof(int);
> +
> +static int output_callback(void *data, short *buffer, int frames)
> +{
> + struct test_data *test_data = (struct test_data *) data;
> +
> + audio_signal_fill(test_data->signal, buffer, frames);
> +
> + return 0;
> +}
> +
> +static int input_callback(void *data, short *buffer, int frames)
> +{
> + struct test_data *test_data = (struct test_data *) data;
> + bool detect;
> +
> + detect = audio_signal_detect(test_data->signal,
> CAPTURE_CHANNELS,
> +  CAPTURE_SAMPLE_RATE, buffer,
> frames);
> + if (detect)
> + test_data->streak++;
> + else
> + test_data->streak = 0;
> +
> + /* A streak of 3 gives confidence that the signal is good.
> */
> + if (test_data->streak == 3)
> + return 1;
> +
> + return 0;
> +}
> +
> +static 

[Intel-gfx] [PATCH i-g-t 3/3] tests: Introduce audio tests, starting with HDMI signal integrity

[Paul Kocialkowski]

This introduces a new test for audio going through display connectors.
It currently contains a single subtest for HDMI signal integrity, but
other test cases will be added later on.

The test setup consists in using an HDMI-VGA bridge that separates the
audio out (via a 3.5 mm jack) and feeding this back to the DUT's line-in
where it can be recorded by ALSA with controls correctly configured.

The audio test makes use of the audio and ALSA igt libraries helpers.

Signed-off-by: Paul Kocialkowski 
---
 tests/Makefile.am |  12 
 tests/audio.c | 170 ++
 2 files changed, 182 insertions(+)
 create mode 100644 tests/audio.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index f9d11e6c..471f3818 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -20,6 +20,15 @@ TESTS_progs += \
$(NULL)
 endif
 
+if HAVE_ALSA
+if HAVE_GSL
+TESTS_progs += \
+   audio \
+   $(NULL)
+endif
+endif
+
+
 if BUILD_TESTS
 test-list.txt: Makefile.sources
@echo TESTLIST > $@
@@ -134,6 +143,9 @@ vc4_wait_seqno_LDADD = $(LDADD) $(DRM_VC4_LIBS)
 chamelium_CFLAGS = $(AM_CFLAGS) $(XMLRPC_CFLAGS) $(LIBUDEV_CFLAGS)
 chamelium_LDADD = $(LDADD) $(XMLRPC_LIBS) $(LIBUDEV_LIBS)
 
+audio_CFLAGS = $(AM_CFLAGS) $(ALSA_CFLAGS)
+audio_LDADD = $(LDADD) $(ALSA_LIBS)
+
 amdgpu_amd_basic_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
 amdgpu_amd_basic_LDADD = $(LDADD) $(DRM_AMDGPU_LIBS)
 amdgpu_amd_cs_nop_CFLAGS = $(AM_CFLAGS) $(DRM_AMDGPU_CFLAGS)
diff --git a/tests/audio.c b/tests/audio.c
new file mode 100644
index ..7099dd99
--- /dev/null
+++ b/tests/audio.c
@@ -0,0 +1,170 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *  Paul Kocialkowski 
+ */
+
+#include "config.h"
+#include "igt.h"
+
+#define PLAYBACK_CHANNELS  2
+#define PLAYBACK_FRAMES1024
+
+#define CAPTURE_SAMPLE_RATE48000
+#define CAPTURE_CHANNELS   2
+#define CAPTURE_DEVICE_NAME"default"
+#define CAPTURE_FRAMES 2048
+
+#define RUN_TIMEOUT2000
+
+struct test_data {
+   struct alsa *alsa;
+   struct audio_signal *signal;
+
+   int streak;
+};
+
+static int sampling_rates[] = {
+   32000,
+   44100,
+   48000,
+   88200,
+   96000,
+   176400,
+   192000,
+};
+
+static int sampling_rates_count = sizeof(sampling_rates) / sizeof(int);
+
+static int test_frequencies[] = {
+   300,
+   600,
+   1200,
+   8,
+   1,
+};
+
+static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
+
+static int output_callback(void *data, short *buffer, int frames)
+{
+   struct test_data *test_data = (struct test_data *) data;
+
+   audio_signal_fill(test_data->signal, buffer, frames);
+
+   return 0;
+}
+
+static int input_callback(void *data, short *buffer, int frames)
+{
+   struct test_data *test_data = (struct test_data *) data;
+   bool detect;
+
+   detect = audio_signal_detect(test_data->signal, CAPTURE_CHANNELS,
+CAPTURE_SAMPLE_RATE, buffer, frames);
+   if (detect)
+   test_data->streak++;
+   else
+   test_data->streak = 0;
+
+   /* A streak of 3 gives confidence that the signal is good. */
+   if (test_data->streak == 3)
+   return 1;
+
+   return 0;
+}
+
+static void test_integrity(const char *device_name)
+{
+   struct test_data data;
+   int sampling_rate;
+   bool run = false;
+   bool test;
+   int i, j;
+   int ret;
+
+   data.alsa = alsa_init();
+   igt_assert(data.alsa);
+
+   ret = alsa_open_output(data.alsa, device_name);
+   igt_assert(ret >= 0);
+
+   ret = alsa_open_input(data.alsa, CAPTURE_DEVICE_NAME);
+