[igt-dev] [PATCH i-g-t v4 1/5] tests/kms_chamelium: add dp-audio test

Ser, Simon simon.ser at intel.com
Wed Apr 17 08:40:22 UTC 2019


On Tue, 2019-04-16 at 15:02 +0300, Martin Peres wrote:
> On 11/04/2019 15:36, Simon Ser wrote:
> > This new test ensures DisplayPort audio works by using the Chamelium.
> > 
> > It enables the DisplayPort output and sends an audio signal containing a set of
> > frequencies we choose to all HDMI/DisplayPort audio devices. It starts
> > recording audio on the Chamelium device and uses the stream server to retrieve
> > captured audio pages. It then checks that the capture audio signal contains the
> > frequencies we sent, and only those, by computing a FFT.
> > 
> > A new library has been added to libigt to communicate with the stream server.
> > It implements a simple custom TCP protocol.
> > 
> > In case the test fails, a WAV file with the captured data is saved on disk.
> > 
> > Right now the test has a few limitations:
> > 
> > - Only the first channel is checked
> > - IGT only generates audio with a single sampling rate (48 KHz)
> > - Audio data is not captured in real-time
> > 
> > These limitations will be lifted in future patches.
> > 
> > PulseAudio must not run during the tests since ALSA is used directly. To ensure
> > this, edit /etc/pulse/client.conf and add `autospawn=no`. Then run
> > `pulseaudio --kill`.
> > 
> > This commit deletes the existing audio tests. They weren't run and required an
> > exotic configuration (HDMI audio splitter, dummy HDMI sink and a line-in port
> > on the DUT).
> > 
> > This patch depends on the following Chameleon bugs:
> > 
> > - https://crbug.com/948060
> > - https://crbug.com/950857
> 
> Seems like your fixes landed already! Great!
> 
> > Signed-off-by: Simon Ser <simon.ser at intel.com>
> > ---
> >  docs/audio.txt             |  45 ---
> >  docs/chamelium.txt         |  32 +-
> >  lib/igt.h                  |   1 +
> >  lib/igt_alsa.c             |  42 ++-
> >  lib/igt_alsa.h             |   1 +
> >  lib/igt_audio.c            | 285 ++++++++++++------
> >  lib/igt_audio.h            |  12 +-
> >  lib/igt_aux.c              |  31 ++
> >  lib/igt_aux.h              |   1 +
> >  lib/igt_chamelium.c        | 101 +++++++
> >  lib/igt_chamelium.h        |  11 +
> >  lib/igt_chamelium_stream.c | 592 +++++++++++++++++++++++++++++++++++++
> >  lib/igt_chamelium_stream.h |  52 ++++
> >  lib/meson.build            |   5 +-
> >  meson.build                |  52 ++--
> >  meson_options.txt          |   6 -
> >  tests/audio.c              | 193 ------------
> >  tests/kms_chamelium.c      | 276 ++++++++++++++++-
> >  tests/meson.build          |   9 +-
> >  19 files changed, 1358 insertions(+), 389 deletions(-)
> >  delete mode 100644 docs/audio.txt
> >  create mode 100644 lib/igt_chamelium_stream.c
> >  create mode 100644 lib/igt_chamelium_stream.h
> >  delete mode 100644 tests/audio.c
> > 
> > diff --git a/docs/audio.txt b/docs/audio.txt
> > deleted file mode 100644
> > index 158ad5d1..00000000
> > --- a/docs/audio.txt
> > +++ /dev/null
> > @@ -1,45 +0,0 @@
> > -Audio Support in IGT
> > -====================
> > -
> > -This document provides information and instructions about audio support in IGT.
> > -
> > -Introduction
> > -------------
> > -
> > -The audio test is aimed at testing the audio features of display connectors,
> > -such as HDMI.
> > -
> > -Test setup
> > -----------
> > -
> > -The setup required for the audio test consists of using an HDMI-VGA adapter with
> > -an audio-out 3.5 mm jack to extract the audio from the HDMI interface.
> > -The audio-out jack is connected back to the device-under-test's line-in.
> > -
> > -Depending on the behavior of the adapter, it may be necessary to connect a
> > -ghost VGA dongle to it (in order to emulate a connected display) to enable the
> > -audio output. There are guides available detailing how to build these.
> > -
> > -When executed, the test will automatically send the test audio signal to all
> > -ALSA audio HDMI outputs and record from the standard ALSA capture device.
> > -
> > -Configuration
> > --------------
> > -
> > -In order to deploy the test, ALSA controls have to be configured to set the
> > -ALSA capture source to line-in. On Intel x86 systems, this can be achieved
> > -with the following calls to the amixer utility:
> > -# amixer sset Line 31 on
> > -# amixer sset "Input Source" Line
> > -
> > -It is then useful to store the ALSA state permanently with the alsactl utility:
> > -# alsactl store
> > -
> > -These settings can be restored with the alsactl utility:
> > -# alsactl restore
> > -
> > -It is desirable to ensure that the alsa-restore and alsa-state systemd services
> > -are enabled to do this job automatically, especially in the case of an
> > -automated testing system:
> > -# systemctl enable alsa-restore
> > -# systemctl enable alsa-state
> > diff --git a/docs/chamelium.txt b/docs/chamelium.txt
> > index 0cabcdc6..316dd607 100644
> > --- a/docs/chamelium.txt
> > +++ b/docs/chamelium.txt
> > @@ -139,6 +139,23 @@ $ make remote-install CHAMELEON_HOST=192.168.72.1
> >  
> >  The process requires the Chamelium to be connected to the Internet to succeed.
> >  
> > +Audio Capture
> > +-------------
> > +
> > +The Chamelium supports audio capture. IGT tests take advantage of the
> > +Chamelium streaming server to download audio samples from the Chamelium.
> > +
> > +IGT needs direct access to audio devices through ALSA, so PulseAudio needs to
> > +be stopped (otherwise audio tests will automatically get skipped). To make sure
> > +PulseAudio isn't running:
> > +
> > +- Edit /etc/pulse/client.conf and add autospawn=no
> > +- Run `pulseaudio --kill` (if it succeeds, it means PulseAudio was running)
> > +- Make sure a DE that automatically spawns PulseAudio isn't running
> > +
> > +In case a test fails, the raw captured audio files will be dumped in a WAV
> > +file.
> > +
> >  Contributing Changes to the Daemon
> >  ----------------------------------
> >  
> > @@ -146,10 +163,11 @@ Contributions to the Chamelium daemon, just like any contribution to ChromiumOS,
> >  are submitted and reviewed at: https://chromium-review.googlesource.com/
> >  
> >  The ChromiumOS project provides an extensive developer guide:
> > -https://www.chromium.org/chromium-os/developer-guide that assumes running within
> > -the ChromiumOS build system. Since this is likely not the case for contributing
> > -to the Chamelium daemon, only the part about uploading changes is relevant:
> > -https://www.chromium.org/chromium-os/developer-guide#TOC-Upload-your-changes-and-get-a-code-review
> > +https://chromium.googlesource.com/chromiumos/docs/+/master/developer_guide.md
> > +It that assumes running within the ChromiumOS build system. Since this is
> 
> The word "that" seems out of place.
> 
> > +likely not the case for contributing to the Chamelium daemon, only the part
> > +about uploading changes is relevant:
> > +https://chromium.googlesource.com/chromiumos/docs/+/master/developer_guide.md#Upload-your-changes-and-get-a-code-review
> >  
> >  Most of the process is about using the Gerrit web interface for submitting and
> >  having the change reviewed and not forgetting the Change-Id, TEST= and BUG=
> > @@ -162,7 +180,7 @@ Support for the Chamelium platform in IGT is found in the following places:
> >  * lib/igt_chamelium.c: library with Chamelium-related helpers
> >  * tests/kms_chamelium.c: sub-tests using the Chamelium
> >  
> > -As of late August 2017, the following features are tested by IGT:
> > +As of early April 2019, the following features are tested by IGT:
> >  * Pixel-by-pixel frame integrity tests for DP and HDMI
> >  * Error-trend-based frame integrity tests for VGA
> >  * CRC-based frame integrity tests for DP and HDMI
> > @@ -173,6 +191,7 @@ As of late August 2017, the following features are tested by IGT:
> >    each interface or combined
> >  * EDID display identifier integrity check for all interfaces
> >  * EDID display identifier change during suspend for all interfaces
> > +* Audio Fourier-based tests for DP at 48KHz
> 
> Maybe an extra patch will be needed to update the list of
> capabilities... or we just implement them :p
> > 
> >  
> >  Future Developments
> >  -------------------
> > @@ -180,7 +199,8 @@ Future Developments
> >  With the current generation of the hardware platform, support for testing a
> >  number of additional display features could be included as future developments,
> >  including:
> > -* Audio capture from HDMI and DP
> > +* Audio capture from HDMI, with multiple channels and with other playback
> > +  sampling rates
> 
> Don't remove DP since it does not yet have the features you are talking
> about.

Hmm. The meaning of that comma was a logical "and". Instead, I'll just
split this into multiple bullet points.

> >  * High-bandwidth Digital Content Protection (HDCP) streaming to the display
> >  * Remote control forwarding (CEC) sent from the display
> >  * YUV colorspace for HDMI, instead of RGB
> > diff --git a/lib/igt.h b/lib/igt.h
> > index 6654a659..5852d557 100644
> > --- a/lib/igt.h
> > +++ b/lib/igt.h
> > @@ -43,6 +43,7 @@
> >  #include "igt_stats.h"
> >  #ifdef HAVE_CHAMELIUM
> >  #include "igt_chamelium.h"
> > +#include "igt_chamelium_stream.h"
> >  #endif
> >  #include "instdone.h"
> >  #include "intel_batchbuffer.h"
> > diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
> > index bb6682cc..456c0c85 100644
> > --- a/lib/igt_alsa.c
> > +++ b/lib/igt_alsa.c
> > @@ -26,9 +26,11 @@
> >  
> >  #include "config.h"
> >  
> > +#include <limits.h>
> >  #include <alsa/asoundlib.h>
> >  
> >  #include "igt_alsa.h"
> > +#include "igt_aux.h"
> >  #include "igt_core.h"
> >  
> >  #define HANDLES_MAX	8
> > @@ -61,6 +63,25 @@ struct alsa {
> >  	int input_samples_trigger;
> >  };
> >  
> > +/**
> > + * alsa_has_exclusive_access:
> > + * Check whether ALSA has exclusive access to audio devices. Fails if
> > + * PulseAudio is running.
> > + */
> > +bool alsa_has_exclusive_access(void)
> > +{
> > +	if (igt_is_process_running("pulseaudio")) {
> > +		igt_warn("It seems that PulseAudio is running. Audio tests "
> > +			 "need direct access to audio devices, so PulseAudio "
> > +			 "needs to be stopped. You can do so by running "
> > +			 "`pulseaudio --kill`. Also make sure to add "
> > +			 "autospawn=no to /etc/pulse/client.conf\n");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> >  static void alsa_error_handler(const char *file, int line, const char *function,
> >  			       int err, const char *fmt, ...)
> >  {
> > @@ -78,6 +99,11 @@ struct alsa *alsa_init(void)
> >  {
> >  	struct alsa *alsa;
> >  
> > +	if (!alsa_has_exclusive_access()) {
> > +		igt_warn("alsa doesn't have exclusive access to audio devices\n");
> 
> How about folding the second warn in alsa_has_exclusive_access()? It
> would improve the chances of having a consistent string to grep on in
> cibuglog in case other users decide to use the function.
> 
> > +		return NULL;
> > +	}
> > +
> >  	alsa = malloc(sizeof(struct alsa));
> >  	memset(alsa, 0, sizeof(struct alsa));
> >  
> > @@ -553,16 +579,20 @@ int alsa_run(struct alsa *alsa, int duration_ms)
> >  					if (ret < 0) {
> >  						ret = snd_pcm_recover(handle,
> >  								      ret, 0);
> > -						if (ret < 0)
> > +						if (ret < 0) {
> > +							igt_debug("snd_pcm_recover after snd_pcm_writei failed");
> >  							goto complete;
> > +						}
> >  					}
> >  
> >  					output_counts[i] += ret;
> >  				} else if (output_counts[i] < output_trigger &&
> >  					   ret < 0) {
> >  					ret = snd_pcm_recover(handle, ret, 0);
> > -					if (ret < 0)
> > +					if (ret < 0) {
> > +						igt_debug("snd_pcm_recover failed");
> >  						goto complete;
> > +					}
> >  				}
> >  			}
> >  
> > @@ -609,16 +639,20 @@ int alsa_run(struct alsa *alsa, int duration_ms)
> >  					ret = 0;
> >  				} else if (ret < 0) {
> >  					ret = snd_pcm_recover(handle, ret, 0);
> > -					if (ret < 0)
> > +					if (ret < 0) {
> > +						igt_debug("snd_pcm_recover after snd_pcm_readi failed");
> >  						goto complete;
> > +					}
> >  				}
> >  
> >  				input_count += ret;
> >  				input_total += ret;
> >  			} else if (input_count < input_trigger && ret < 0) {
> >  				ret = snd_pcm_recover(handle, ret, 0);
> > -				if (ret < 0)
> > +				if (ret < 0) {
> > +					igt_debug("snd_pcm_recover failed");
> >  					goto complete;
> > +				}
> >  			}
> >  		}
> >  	} while (!reached);
> > diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
> > index 50795130..5c804b46 100644
> > --- a/lib/igt_alsa.h
> > +++ b/lib/igt_alsa.h
> > @@ -33,6 +33,7 @@
> >  
> >  struct alsa;
> >  
> > +bool alsa_has_exclusive_access(void);
> >  struct alsa *alsa_init(void);
> >  int alsa_open_output(struct alsa *alsa, const char *device_name);
> >  int alsa_open_input(struct alsa *alsa, const char *device_name);
> > diff --git a/lib/igt_audio.c b/lib/igt_audio.c
> > index a0592d53..4cc9bdf0 100644
> > --- a/lib/igt_audio.c
> > +++ b/lib/igt_audio.c
> > @@ -26,8 +26,11 @@
> >  
> >  #include "config.h"
> >  
> > -#include <math.h>
> > +#include <errno.h>
> > +#include <fcntl.h>
> >  #include <gsl/gsl_fft_real.h>
> > +#include <math.h>
> > +#include <unistd.h>
> >  
> >  #include "igt_audio.h"
> >  #include "igt_core.h"
> > @@ -128,7 +131,7 @@ int audio_signal_add_frequency(struct audio_signal *signal, int frequency)
> >   */
> >  void audio_signal_synthesize(struct audio_signal *signal)
> >  {
> > -	short *period;
> > +	int16_t *period;
> >  	double value;
> >  	int frames;
> >  	int freq;
> > @@ -145,9 +148,9 @@ void audio_signal_synthesize(struct audio_signal *signal)
> >  
> >  		for (j = 0; j < frames; j++) {
> >  			value = 2.0 * M_PI * freq / signal->sampling_rate * j;
> > -			value = sin(value) * SHRT_MAX / signal->freqs_count;
> > +			value = sin(value) * INT16_MAX / signal->freqs_count;
> >  
> > -			period[j] = (short) value;
> > +			period[j] = (int16_t) value;
> 
> Seems like all these alsa changes should be in their own patches so as
> you could explain why these changes are needed.
> 
> Are you afraid that some platforms would not have SHORT == INT16?
> 
> If these changes would have required changes in code that you would
> change anyway right after, then at least explain the changes in the
> commit log :)

Yeah, sorry about this. I just completely forgot about this change when
writing the commit log. TBH when writing this patch I was mainly
concerned about getting something working _at all_, and it took quite a
few days. :P

The rationale is:

- The standard says a short is at least 16 bit wide, but a short can be
  larger (in practice it won't happen, but better use types correctly)
- It makes it clearer that the audio format is S16_LE, since "16" is 
  in the type name.

I can extract these changes to an individual commit if needed.

> >  		}
> >  
> >  		signal->freqs[i].period = period;
> > @@ -186,17 +189,16 @@ void audio_signal_clean(struct audio_signal *signal)
> >   * signal data (in interleaved S16_LE format), at the requested sampling rate
> >   * and number of channels.
> >   */
> > -void audio_signal_fill(struct audio_signal *signal, short *buffer, int frames)
> > +void audio_signal_fill(struct audio_signal *signal, int16_t *buffer, int frames)
> >  {
> > -	short *destination;
> > -	short *source;
> > +	int16_t *destination, *source;
> >  	int total;
> >  	int freq_frames;
> >  	int freq_offset;
> >  	int count;
> >  	int i, j, k;
> >  
> > -	memset(buffer, 0, sizeof(short) * signal->channels * frames);
> > +	memset(buffer, 0, sizeof(int16_t) * signal->channels * frames);
> >  
> >  	for (i = 0; i < signal->freqs_count; i++) {
> >  		total = 0;
> > @@ -229,97 +231,214 @@ void audio_signal_fill(struct audio_signal *signal, short *buffer, int frames)
> >  }
> >  
> >  /**
> > - * audio_signal_detect:
> > - * @signal: The target signal structure
> > - * @channels: The input data's number of channels
> > - * @sampling_rate: The input data's sampling rate
> > - * @buffer: The input data's buffer
> > - * @frames: The input data's number of frames
> > - *
> > - * Detect that the frequencies specified in @signal, and only those, are
> > - * present in the input data. The input data's format is required to be S16_LE.
> > + * Checks that frequencies specified in signal, and only those, are included
> > + * in the input data.
> >   *
> > - * Returns: A boolean indicating whether the detection was successful
> > + * sampling_rate is given in Hz. data_len is the number of elements in data.
> >   */
> > -bool audio_signal_detect(struct audio_signal *signal, int channels,
> > -			 int sampling_rate, short *buffer, int frames)
> > +bool audio_signal_detect(struct audio_signal *signal, int sampling_rate,
> > +			 double *data, size_t data_len)
> >  {
> > -	double data[frames];
> > -	int amplitude[frames / 2];
> > +	size_t amplitude_len = data_len / 2 + 1;
> > +	double amplitude[amplitude_len];
> 
> bin_power?

Hmm, "amplitude" is used too it seems [1]. But bin_power LGTM, let's
switch to this wording.

[1]: https://en.wikipedia.org/wiki/Fast_Fourier_transform#/media/File:FFT_of_Cosine_Summation_Function.png

> >  	bool detected[signal->freqs_count];
> > -	int threshold;
> > -	bool above;
> > -	int error;
> > -	int freq = 0;
> > -	int max;
> > -	int c, i, j;
> > -
> > -	/* Allowed error in Hz due to FFT step. */
> > -	error = sampling_rate / frames;
> > +	int ret, epsilon, freq, max_freq;
> > +	double max, threshold;
> > +	size_t i, j;
> > +	bool above, success;
> > +
> > +	/* Allowed error in Hz due to FFT step */
> > +	epsilon = sampling_rate / data_len;
> 
> freq_accuracy would be a nicer name.

Agreed.

> > +	igt_debug("allowed freq. error: %d Hz\n", epsilon);
> > +
> > +	ret = gsl_fft_real_radix2_transform(data, 1, data_len);
> > +	igt_assert(ret == 0);
> > +
> > +	/* For i < data_len / 2, the real part of the i-th term is stored at
> > +	 * data[i] and its imaginary part is stored at data[data_len - i].
> > +	 * i = 0 and i = data_len / 2 are special cases, they are purely real
> > +	 * so their imaginary part isn't stored.
> > +	 *
> > +	 * The amplitude is encoded as the magnitude of the complex number and
> > +	 * the phase is encoded as its angle.
> > +	 */
> 
> Thanks for documenting the idiosyncrasies of GSL!
> 
> /* compute the power received at every bin of the FFT, and record the
>  * maximum power received as a way to normalize all the others.
>  */
> 
> > +	max = 0;
> > +	amplitude[0] = data[0];
> > +	for (i = 1; i < amplitude_len - 1; i++) {
> > +		amplitude[i] = hypot(data[i], data[data_len - i]);
> > +		if (amplitude[i] > max)
> > +			max = amplitude[i];
> > +	}
> > +	amplitude[amplitude_len - 1] = data[data_len / 2];
> 
> What's the coding style of IGT wrt spaces vs tabs?

Always use tabs? IDGI, I only see tabs in there. I'm probably missing
something.

> >  
> > -	for (c = 0; c < channels; c++) {
> > -		for (i = 0; i < frames; i++)
> > -			data[i] = (double) buffer[i * channels + c];
> > +	for (i = 0; i < signal->freqs_count; i++)
> > +		detected[i] = false;
> 
> This could have been a bitfield, but meh!

Right. Still have to be a little careful because in the future we'll
probably test 8 channels with a couple of different frequencies on each
(we test 5 frequencies ATM, so that would be 40 total), so it could
somewhat approach a 64 frequencies limit.

> >  
> > -		gsl_fft_real_radix2_transform(data, 1, frames);
> > +	/* We want to detect peaks above a given threshold. */
> 
> /* Do a linear search through the FFT bins' power to find the the local
>  * maximums that exceed half of the absolute maximum that we previously
>  * calculated.
>  *
>  * Since the frequencies might not be perfectly aligned with the bins of
>  * the FFT, we need to find the local maximum across some consecutive
>  * bins. Once the power returns under the power threshold, we compare
>  * the frequency of the bin that received the maximum power to the
>  * expected frequencies. If found, we mark this frequency as such,
>  * otherwise we warn that an unexpected frequency was found.
>  */
> 
> > +	threshold = max / 2;
> > +	success = true;
> > +	above = false;
> > +	max = 0;
> > +	max_freq = -1;
> 
> local_max = 0;
> local_max_freq = -1;

That's better indeed.

> > +	for (i = 0; i < amplitude_len; i++) {
> > +		freq = sampling_rate * i / data_len;
> >  
> > -		max = 0;
> > +		if (amplitude[i] > threshold)
> > +			above = true;
> >  
> > -		for (i = 0; i < frames / 2; i++) {
> > -			amplitude[i] = hypot(data[i], data[frames - i]);
> > -			if (amplitude[i] > max)
> > -				max = amplitude[i];
> > +		if (!above) {
> > +			continue;
> >  		}
> >  
> > -		for (i = 0; i < signal->freqs_count; i++)
> > -			detected[i] = false;
> > -
> > -		threshold = max / 2;
> > -		above = false;
> > -		max = 0;
> > -
> > -		for (i = 0; i < frames / 2; i++) {
> > -			if (amplitude[i] > threshold)
> > -				above = true;
> > -
> > -			if (above) {
> > -				if (amplitude[i] < threshold) {
> > -					above = false;
> > -					max = 0;
> > -
> > -					for (j = 0; j < signal->freqs_count; j++) {
> > -						if (signal->freqs[j].freq >
> > -						    freq - error &&
> > -						    signal->freqs[j].freq <
> > -						    freq + error) {
> > -							detected[j] = true;
> > -							break;
> > -						}
> > -					}
> > -
> > -					/* Detected frequency was not generated. */
> > -					if (j == signal->freqs_count) {
> > -						igt_debug("Detected additional frequency: %d\n",
> > -							  freq);
> > -						return false;
> > -					}
> > +		/* If we were above the threshold and we're not anymore, it's
> > +		 * time to decide whether the peak frequency is correct or
> > +		 * invalid. */
> > +		if (amplitude[i] < threshold) {
> > +			for (j = 0; j < signal->freqs_count; j++) {
> > +				if (signal->freqs[j].freq >
> > +				    max_freq - epsilon &&
> > +				    signal->freqs[j].freq <
> > +				    max_freq + epsilon) {
> > +					detected[j] = true;
> > +					igt_debug("Frequency %d detected\n",
> > +						  max_freq);
> > +					break;
> >  				}
> > +			}
> >  
> > -				if (amplitude[i] > max) {
> > -					max = amplitude[i];
> > -					freq = sampling_rate * i / frames;
> > -				}
> > +			/* We haven't generated this frequency, but we detected
> > +			 * it. */
> > +			if (j == signal->freqs_count) {
> > +				igt_debug("Detected additional frequency: %d\n",
> > +					  max_freq);
> > +				success = false;
> >  			}
> > +
> > +			above = false;
> > +			max = 0;
> > +			max_freq = -1;
> >  		}
> >  
> > -		for (i = 0; i < signal->freqs_count; i++) {
> > -			if (!detected[i]) {
> > -				igt_debug("Missing frequency: %d\n",
> > -					  signal->freqs[i].freq);
> > -				return false;
> > -			}
> > +		if (amplitude[i] > max) {
> > +			max = amplitude[i];
> > +			max_freq = freq;
> > +		}
> > +	}
> > +
> > +	/* Check that all frequencies we generated have been detected. */
> > +	for (i = 0; i < signal->freqs_count; i++) {
> > +		if (!detected[i]) {
> > +			igt_debug("Missing frequency: %d\n",
> > +				  signal->freqs[i].freq);
> > +			success = false;
> >  		}
> >  	}
> >  
> > -	return true;
> > +	return success;
> > +}
> > +
> > +/**
> > + * Extracts a single channel from a multi-channel S32_LE input buffer.
> > + */
> > +size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
> > +				    int32_t *src, size_t src_len,
> > +				    int n_channels, int channel)
> > +{
> > +	size_t dst_len, i;
> > +
> > +	igt_assert(channel < n_channels);
> > +	igt_assert(src_len % n_channels == 0);
> > +	dst_len = src_len / n_channels;
> > +	igt_assert(dst_len <= dst_cap);
> > +	for (i = 0; i < dst_len; i++)
> > +		dst[i] = (double) src[i * n_channels + channel];
> > +
> > +	return dst_len;
> > +}
> > +
> > +#define RIFF_TAG "RIFF"
> > +#define WAVE_TAG "WAVE"
> > +#define FMT_TAG "fmt "
> > +#define DATA_TAG "data"
> > +
> > +static void
> > +append_to_buffer(char *dst, size_t *i, const void *src, size_t src_size)
> > +{
> > +	memcpy(&dst[*i], src, src_size);
> > +	*i += src_size;
> > +}
> > +
> > +/**
> > + * Creates a new WAV file. sample_rate is in Hz. If path is not NULL, it will
> > + * be set to the new file path (the caller is responsible for free-ing it).
> > + *
> > + * After calling this function, the caller is expected to write S32_LE PCM data
> > + * to the returned file descriptor.
> > + *
> > + * See http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html for
> > + * a WAV file format specification.
> > + */
> > +int audio_create_wav_file_s32_le(const char *qualifier, uint32_t sample_rate,
> > +				 uint16_t channels, char **path)
> 
> What is qualifier?

This is taken from igt_write_frame_to_png. I guess this could use some
more docs.

/**
 * audio_create_wav_file_s32_le:
 * @qualifier: the basename of the file (the test name will be prepended, and
 * the file extension will be appended)
 * @sample_rate: the sample rate in Hz
 * @channels: the number of channels
 * @path: if non-NULL, will be set to a pointer to the new file path (the
 * caller is responsible for free-ing it)
 *
 * Creates a new WAV file.
 *
 * After calling this function, the caller is expected to write S32_LE PCM data
 * to the returned file descriptor.
 *
 * See http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/WAVE.html for
 * a WAV file format specification.
 *
 * Returns: a file descriptor to the newly created file, or -1 on error.
 */

> Maybe we can rename that to channel_count?

I want to shove in the playback frequency too in the future, so I'd
rather not.

> > +{
> > +	char _path[PATH_MAX];
> > +	const char *test_name, *subtest_name;
> > +	int fd;
> > +	char header[44];
> > +	size_t i = 0;
> > +	uint32_t file_size, chunk_size, byte_rate;
> > +	uint16_t format, block_align, bits_per_sample;
> > +
> > +	test_name = igt_test_name();
> > +	subtest_name = igt_subtest_name();
> > +
> > +	igt_assert(igt_frame_dump_path);
> > +	snprintf(_path, sizeof(_path), "%s/audio-%s-%s-%s.wav",
> > +		 igt_frame_dump_path, test_name, subtest_name, qualifier);
> > +
> > +	if (path)
> > +		*path = strdup(_path);
> > +
> > +	igt_debug("Dumping %s audio to %s\n", qualifier, _path);
> > +	fd = open(_path, O_WRONLY | O_CREAT | O_TRUNC);
> > +	if (fd < 0) {
> > +		igt_warn("open failed: %s\n", strerror(errno));
> > +		return -1;
> > +	}
> > +
> > +	/* File header */
> > +	file_size = UINT32_MAX; /* unknown file size */
> > +	append_to_buffer(header, &i, RIFF_TAG, strlen(RIFF_TAG));
> > +	append_to_buffer(header, &i, &file_size, sizeof(file_size));
> > +	append_to_buffer(header, &i, WAVE_TAG, strlen(WAVE_TAG));
> > +
> > +	/* Format chunk */
> > +	chunk_size = 16;
> > +	format = 1; /* PCM */
> > +	bits_per_sample = 32; /* S32_LE */
> > +	byte_rate = sample_rate * channels * bits_per_sample / 8;
> > +	block_align = channels * bits_per_sample / 8;
> > +	append_to_buffer(header, &i, FMT_TAG, strlen(FMT_TAG));
> > +	append_to_buffer(header, &i, &chunk_size, sizeof(chunk_size));
> > +	append_to_buffer(header, &i, &format, sizeof(format));
> > +	append_to_buffer(header, &i, &channels, sizeof(channels));
> > +	append_to_buffer(header, &i, &sample_rate, sizeof(sample_rate));
> > +	append_to_buffer(header, &i, &byte_rate, sizeof(byte_rate));
> > +	append_to_buffer(header, &i, &block_align, sizeof(block_align));
> > +	append_to_buffer(header, &i, &bits_per_sample, sizeof(bits_per_sample));
> > +
> > +	/* Data chunk */
> > +	chunk_size = UINT32_MAX; /* unknown chunk size */
> > +	append_to_buffer(header, &i, DATA_TAG, strlen(DATA_TAG));
> > +	append_to_buffer(header, &i, &chunk_size, sizeof(chunk_size));
> > +
> > +	igt_assert(i == sizeof(header));
> > +
> > +	if (write(fd, header, sizeof(header)) != sizeof(header)) {
> > +		igt_warn("write failed: %s'n", strerror(errno));
> > +		close(fd);
> > +		return -1;
> > +	}
> > +
> > +	return fd;
> >  }
> > diff --git a/lib/igt_audio.h b/lib/igt_audio.h
> > index b3b658a4..4aa43e69 100644
> > --- a/lib/igt_audio.h
> > +++ b/lib/igt_audio.h
> > @@ -30,6 +30,7 @@
> >  #include "config.h"
> >  
> >  #include <stdbool.h>
> > +#include <stdint.h>
> >  
> >  struct audio_signal;
> >  
> > @@ -37,8 +38,13 @@ struct audio_signal *audio_signal_init(int channels, int sampling_rate);
> >  int audio_signal_add_frequency(struct audio_signal *signal, int frequency);
> >  void audio_signal_synthesize(struct audio_signal *signal);
> >  void audio_signal_clean(struct audio_signal *signal);
> > -void audio_signal_fill(struct audio_signal *signal, short *buffer, int frames);
> > -bool audio_signal_detect(struct audio_signal *signal, int channels,
> > -			 int sampling_rate, short *buffer, int frames);
> > +void audio_signal_fill(struct audio_signal *signal, int16_t *buffer, int frames);
> > +bool audio_signal_detect(struct audio_signal *signal, int sampling_rate,
> > +			 double *data, size_t data_len);
> > +size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
> > +				    int32_t *src, size_t src_len,
> > +				    int n_channels, int channel);
> > +int audio_create_wav_file_s32_le(const char *qualifier, uint32_t sample_rate,
> > +				 uint16_t channels, char **path);
> >  
> >  #endif
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index 05528352..95ceb845 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -1259,6 +1259,37 @@ void igt_set_module_param_int(const char *name, int val)
> >  	igt_set_module_param(name, str);
> >  }
> >  
> > +/**
> > + * igt_is_process_running:
> > + * @comm: Name of process in the form found in /proc/pid/comm (limited to 15
> > + * chars)
> > + *
> > + * Returns: true in case the process has been found, false otherwise.
> > + *
> > + * This function checks in the process table for an entry with the name @comm.
> > + */
> > +int igt_is_process_running(const char *comm)
> > +{
> > +	PROCTAB *proc;
> > +	proc_t *proc_info;
> > +	bool found = false;
> > +
> > +	proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> 
> Seems like you can drop FILLARG:
> 
> From man:
>        PROC_FILLARG
>             equivalent to PROC_FILLCOM

Good catch

> > +	igt_assert(proc != NULL);
> > +
> > +	while ((proc_info = readproc(proc, NULL))) {
> > +		if (!strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> > +			freeproc(proc_info);
> > +			found = true;
> > +			break;
> > +		}
> > +		freeproc(proc_info);
> > +	}
> > +
> > +	closeproc(proc);
> > +	return found;
> > +}
> > +
> >  /**
> >   * igt_terminate_process:
> >   * @sig: Signal to send
> > diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> > index 55392790..dbd88b67 100644
> > --- a/lib/igt_aux.h
> > +++ b/lib/igt_aux.h
> > @@ -279,6 +279,7 @@ bool igt_allow_unlimited_files(void);
> >  void igt_set_module_param(const char *name, const char *val);
> >  void igt_set_module_param_int(const char *name, int val);
> >  
> > +int igt_is_process_running(const char *comm);
> >  int igt_terminate_process(int sig, const char *comm);
> >  void igt_lsof(const char *dpath);
> >  
> > diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> > index 02cc9b2c..7c9030d1 100644
> > --- a/lib/igt_chamelium.c
> > +++ b/lib/igt_chamelium.c
> > @@ -218,6 +218,12 @@ void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump)
> >  	free(dump);
> >  }
> >  
> > +void chamelium_destroy_audio_file(struct chamelium_audio_file *audio_file)
> > +{
> > +	free(audio_file->path);
> > +	free(audio_file);
> > +}
> > +
> >  struct fsm_monitor_args {
> >  	struct chamelium *chamelium;
> >  	struct chamelium_port *port;
> > @@ -924,6 +930,101 @@ int chamelium_get_captured_frame_count(struct chamelium *chamelium)
> >  	return ret;
> >  }
> >  
> > +/**
> > + * chamelium_start_capturing_audio:
> > + * @chamelium: the Chamelium instance
> > + * @port: the port to capture audio from (it must support audio)
> > + * @save_to_file: whether the captured audio data should be saved to a file on
> > + * the Chamelium device
> > + *
> > + * Starts capturing audio from a Chamelium port. To stop the capture, use
> > + * #chamelium_stop_capturing_audio. To retrieve the audio data, either use the
> > + * stream server or enable @save_to_file (the latter is mainly useful for
> > + * debugging purposes).
> > + *
> > + * It isn't possible to capture audio from multiple ports at the same time.
> > + */
> > +void chamelium_start_capturing_audio(struct chamelium *chamelium,
> > +				    struct chamelium_port *port,
> > +				    bool save_to_file)
> > +{
> > +	xmlrpc_value *res;
> > +
> > +	res = chamelium_rpc(chamelium, port, "StartCapturingAudio", "(ib)",
> > +			    port->id, save_to_file);
> > +	xmlrpc_DECREF(res);
> > +}
> > +
> > +static void audio_format_from_xml(struct chamelium *chamelium,
> > +				  xmlrpc_value *res, int *rate, int *channels)
> > +{
> > +	xmlrpc_value *res_type, *res_rate, *res_sample_format, *res_channel;
> > +	char *type, *sample_format;
> > +
> > +	xmlrpc_struct_find_value(&chamelium->env, res, "file_type", &res_type);
> > +	xmlrpc_struct_find_value(&chamelium->env, res, "rate", &res_rate);
> > +	xmlrpc_struct_find_value(&chamelium->env, res, "sample_format", &res_sample_format);
> > +	xmlrpc_struct_find_value(&chamelium->env, res, "channel", &res_channel);
> > +
> > +	xmlrpc_read_string(&chamelium->env, res_type, (const char **) &type);
> > +	igt_assert(strcmp(type, "raw") == 0);
> > +	free(type);
> > +
> > +	xmlrpc_read_string(&chamelium->env, res_sample_format, (const char **) &sample_format);
> > +	igt_assert(strcmp(sample_format, "S32_LE") == 0);
> > +	free(sample_format);
> > +
> > +	xmlrpc_read_int(&chamelium->env, res_rate, rate);
> > +	xmlrpc_read_int(&chamelium->env, res_channel, channels);
> > +
> > +	xmlrpc_DECREF(res_channel);
> > +	xmlrpc_DECREF(res_sample_format);
> > +	xmlrpc_DECREF(res_rate);
> > +	xmlrpc_DECREF(res_type);
> > +}
> > +
> > +/**
> > + * chamelium_stop_capturing_audio:
> > + * @chamelium: the Chamelium instance
> > + * @port: the port from which audio is being captured
> > + *
> > + * Stops capturing audio from a Chamelium port. If
> > + * #chamelium_start_capturing_audio has been called with @save_to_file enabled,
> > + * this function will return a #chamelium_audio_file struct containing details
> > + * about the audio file. Once the caller is done with the struct, they should
> > + * release it with #chamelium_destroy_audio_file.
> > + */
> > +struct chamelium_audio_file *chamelium_stop_capturing_audio(struct chamelium *chamelium,
> > +							    struct chamelium_port *port)
> > +{
> > +	xmlrpc_value *res, *res_path, *res_props;
> > +	struct chamelium_audio_file *file = NULL;
> > +	char *path;
> > +
> > +	res = chamelium_rpc(chamelium, NULL, "StopCapturingAudio", "(i)",
> > +			    port->id);
> > +	xmlrpc_array_read_item(&chamelium->env, res, 0, &res_path);
> > +	xmlrpc_array_read_item(&chamelium->env, res, 1, &res_props);
> > +
> > +	xmlrpc_read_string(&chamelium->env, res_path, (const char **) &path);
> > +
> > +	if (strlen(path) > 0) {
> > +		file = calloc(1, sizeof(*file));
> > +		file->path = path;
> > +
> > +		audio_format_from_xml(chamelium, res_props,
> > +				      &file->rate, &file->channels);
> > +	} else {
> > +		free(path);
> > +	}
> > +
> > +	xmlrpc_DECREF(res_props);
> > +	xmlrpc_DECREF(res_path);
> > +	xmlrpc_DECREF(res);
> > +
> > +	return file;
> > +}
> > +
> >  static pixman_image_t *convert_frame_format(pixman_image_t *src,
> >  					    int format)
> >  {
> > diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> > index 233ead85..047f8c5d 100644
> > --- a/lib/igt_chamelium.h
> > +++ b/lib/igt_chamelium.h
> > @@ -53,6 +53,12 @@ enum chamelium_check {
> >  	CHAMELIUM_CHECK_CRC,
> >  };
> >  
> > +struct chamelium_audio_file {
> > +	char *path;
> > +	int rate; /* Hz */
> > +	int channels;
> > +};
> > +
> >  struct chamelium *chamelium_init(int drm_fd);
> >  void chamelium_deinit(struct chamelium *chamelium);
> >  void chamelium_reset(struct chamelium *chamelium);
> > @@ -100,6 +106,10 @@ void chamelium_start_capture(struct chamelium *chamelium,
> >  void chamelium_stop_capture(struct chamelium *chamelium, int frame_count);
> >  void chamelium_capture(struct chamelium *chamelium, struct chamelium_port *port,
> >  		       int x, int y, int w, int h, int frame_count);
> > +void chamelium_start_capturing_audio(struct chamelium *chamelium,
> > +				    struct chamelium_port *port, bool save_to_file);
> > +struct chamelium_audio_file *chamelium_stop_capturing_audio(struct chamelium *chamelium,
> > +							    struct chamelium_port *port);
> >  igt_crc_t *chamelium_read_captured_crcs(struct chamelium *chamelium,
> >  					int *frame_count);
> >  struct chamelium_frame_dump *chamelium_read_captured_frame(struct chamelium *chamelium,
> > @@ -131,5 +141,6 @@ void chamelium_assert_frame_match_or_dump(struct chamelium *chamelium,
> >  void chamelium_crop_analog_frame(struct chamelium_frame_dump *dump, int width,
> >  				 int height);
> >  void chamelium_destroy_frame_dump(struct chamelium_frame_dump *dump);
> > +void chamelium_destroy_audio_file(struct chamelium_audio_file *audio_file);
> >  
> >  #endif /* IGT_CHAMELIUM_H */
> > diff --git a/lib/igt_chamelium_stream.c b/lib/igt_chamelium_stream.c
> > new file mode 100644
> > index 00000000..9e1ba1ca
> > --- /dev/null
> > +++ b/lib/igt_chamelium_stream.c
> > @@ -0,0 +1,592 @@
> > +/*
> > + * Copyright © 2019 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: Simon Ser <simon.ser at intel.com>
> > + */
> > +
> > +#include "config.h"
> > +
> > +#include <arpa/inet.h>
> > +#include <errno.h>
> > +#include <netdb.h>
> > +#include <stdbool.h>
> > +#include <stdlib.h>
> > +#include <sys/types.h>
> > +#include <sys/socket.h>
> > +
> > +#include "igt_chamelium_stream.h"
> > +#include "igt_core.h"
> > +#include "igt_rc.h"
> > +
> > +#define STREAM_PORT 9994
> > +#define STREAM_VERSION_MAJOR 1
> > +#define STREAM_VERSION_MINOR 0
> > +
> > +enum stream_error {
> > +	STREAM_ERROR_NONE = 0,
> > +	STREAM_ERROR_COMMAND = 1,
> > +	STREAM_ERROR_ARGUMENT = 2,
> > +	STREAM_ERROR_EXISTS = 3,
> > +	STREAM_ERROR_VIDEO_MEM_OVERFLOW_STOP = 4,
> > +	STREAM_ERROR_VIDEO_MEM_OVERFLOW_DROP = 5,
> > +	STREAM_ERROR_AUDIO_MEM_OVERFLOW_STOP = 6,
> > +	STREAM_ERROR_AUDIO_MEM_OVERFLOW_DROP = 7,
> > +	STREAM_ERROR_NO_MEM = 8,
> > +};
> > +
> > +enum stream_message_kind {
> > +	STREAM_MESSAGE_REQUEST = 0,
> > +	STREAM_MESSAGE_RESPONSE = 1,
> > +	STREAM_MESSAGE_DATA = 2,
> > +};
> > +
> > +enum stream_message_type {
> > +	STREAM_MESSAGE_RESET = 0,
> > +	STREAM_MESSAGE_GET_VERSION = 1,
> > +	STREAM_MESSAGE_VIDEO_STREAM = 2,
> > +	STREAM_MESSAGE_SHRINK_VIDEO = 3,
> > +	STREAM_MESSAGE_VIDEO_FRAME = 4,
> > +	STREAM_MESSAGE_DUMP_REALTIME_VIDEO = 5,
> > +	STREAM_MESSAGE_STOP_DUMP_VIDEO = 6,
> > +	STREAM_MESSAGE_DUMP_REALTIME_AUDIO = 7,
> > +	STREAM_MESSAGE_STOP_DUMP_AUDIO = 8,
> > +};
> > +
> > +struct chamelium_stream {
> > +	char *host;
> > +	unsigned int port;
> > +
> > +	int fd;
> > +};
> > +
> > +static const char *stream_error_str(enum stream_error err)
> > +{
> > +	switch (err) {
> > +	case STREAM_ERROR_NONE:
> > +		return "no error";
> > +	case STREAM_ERROR_COMMAND:
> > +		return "invalid command";
> > +	case STREAM_ERROR_ARGUMENT:
> > +		return "invalid arguments";
> > +	case STREAM_ERROR_EXISTS:
> > +		return "dump already started";
> > +	case STREAM_ERROR_VIDEO_MEM_OVERFLOW_STOP:
> > +		return "video dump stopped after overflow";
> > +	case STREAM_ERROR_VIDEO_MEM_OVERFLOW_DROP:
> > +		return "video frame dropped after overflow";
> > +	case STREAM_ERROR_AUDIO_MEM_OVERFLOW_STOP:
> > +		return "audio dump stoppred after overflow";
> > +	case STREAM_ERROR_AUDIO_MEM_OVERFLOW_DROP:
> > +		return "audio page dropped after overflow";
> > +	case STREAM_ERROR_NO_MEM:
> > +		return "out of memory";
> > +	}
> > +	return "unknown error";
> > +}
> > +
> > +/**
> > + * The Chamelium URL is specified in the configuration file. We need to extract
> > + * the host to connect to the stream server.
> > + */
> > +static char *parse_url_host(const char *url)
> > +{
> > +	static const char prefix[] = "http://";
> > +	char *colon;
> > +
> > +	if (strstr(url, prefix) != url)
> > +		return NULL;
> > +	url += strlen(prefix);
> > +
> > +	colon = strchr(url, ':');
> > +	if (!colon)
> > +		return NULL;
> > +
> > +	return strndup(url, colon - url);
> > +}
> > +
> > +static bool chamelium_stream_read_config(struct chamelium_stream *client)
> > +{
> > +	GError *error = NULL;
> > +	gchar *chamelium_url;
> > +
> > +	if (!igt_key_file) {
> > +		igt_warn("No configuration file available for chamelium\n");
> > +		return false;
> > +	}
> > +
> > +	chamelium_url = g_key_file_get_string(igt_key_file, "Chamelium", "URL",
> > +					      &error);
> > +	if (!chamelium_url) {
> > +		igt_warn("Couldn't read Chamelium URL from config file: %s\n",
> > +			 error->message);
> > +		return false;
> > +	}
> > +
> > +	client->host = parse_url_host(chamelium_url);
> > +	if (!client->host) {
> > +		igt_warn("Invalid Chamelium URL in config file: %s\n",
> > +			 chamelium_url);
> > +		return false;
> > +	}
> > +	client->port = STREAM_PORT;
> > +
> > +	return true;
> > +}
> > +
> > +static bool chamelium_stream_connect(struct chamelium_stream *client)
> > +{
> > +	int ret;
> > +	char port_str[16];
> > +	struct addrinfo hints = {};
> > +	struct addrinfo *results, *ai;
> > +	struct timeval tv = {};
> > +
> > +	igt_debug("Connecting to Chamelium stream server: tcp://%s:%u\n",
> > +		  client->host, client->port);
> > +
> > +	snprintf(port_str, sizeof(port_str), "%u", client->port);
> > +
> > +	hints.ai_family = AF_UNSPEC;
> > +	hints.ai_socktype = SOCK_STREAM;
> > +	ret = getaddrinfo(client->host, port_str, &hints, &results);
> > +	if (ret != 0) {
> > +		igt_warn("getaddrinfo failed: %s\n", gai_strerror(ret));
> > +		return false;
> > +	}
> > +
> > +	client->fd = -1;
> > +	for (ai = results; ai != NULL; ai = ai->ai_next) {
> > +		client->fd = socket(ai->ai_family, ai->ai_socktype,
> > +				    ai->ai_protocol);
> > +		if (client->fd == -1)
> > +			continue;
> > +
> > +		if (connect(client->fd, ai->ai_addr, ai->ai_addrlen) == -1) {
> > +			close(client->fd);
> > +			client->fd = -1;
> > +			continue;
> > +		}
> > +
> > +		break;
> > +	}
> > +
> > +	freeaddrinfo(results);
> > +
> > +	if (client->fd < 0) {
> > +		igt_warn("Failed to connect to Chamelium stream server\n");
> > +		return false;
> > +	}
> > +
> > +	/* Set a read and write timeout of 5 seconds. */
> > +	tv.tv_sec = 5;
> > +	setsockopt(client->fd, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv));
> > +	setsockopt(client->fd, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv));
> > +
> > +	return true;
> > +}
> > +
> > +static bool read_whole(int fd, void *buf, size_t buf_len)
> > +{
> > +	ssize_t ret;
> > +	size_t n = 0;
> > +	char *ptr;
> > +
> > +	while (n < buf_len) {
> > +		ptr = (char *) buf + n;
> > +		ret = read(fd, ptr, buf_len - n);
> > +		if (ret < 0) {
> > +			igt_warn("read failed: %s\n", strerror(errno));
> > +			return false;
> > +		} else if (ret == 0) {
> > +			igt_warn("short read\n");
> > +			return false;
> > +		}
> > +		n += ret;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static bool write_whole(int fd, void *buf, size_t buf_len)
> > +{
> > +	ssize_t ret;
> > +	size_t n = 0;
> > +	char *ptr;
> > +
> > +	while (n < buf_len) {
> > +		ptr = (char *) buf + n;
> > +		ret = write(fd, ptr, buf_len - n);
> > +		if (ret < 0) {
> > +			igt_warn("write failed: %s\n", strerror(errno));
> > +			return false;
> > +		} else if (ret == 0) {
> > +			igt_warn("short write\n");
> > +			return false;
> > +		}
> > +		n += ret;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static bool read_and_discard(int fd, size_t len)
> > +{
> > +	char buf[1024];
> > +	size_t n;
> > +
> > +	while (len > 0) {
> > +		n = len;
> > +		if (n > sizeof(buf))
> > +			n = sizeof(buf);
> > +
> > +		if (!read_whole(fd, buf, n))
> > +			return false;
> > +
> > +		len -= n;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +/** Read a message header from the socket.
> > + *
> > + * The header is laid out as follows:
> > + * - u16: message type
> > + * - u16: error code
> > + * - u32: message length
> > + */
> > +static bool chamelium_stream_read_header(struct chamelium_stream *client,
> > +					 enum stream_message_kind *kind,
> > +					 enum stream_message_type *type,
> > +					 enum stream_error *err,
> > +					 size_t *len)
> > +{
> > +	uint16_t _type;
> > +	char buf[8];
> > +
> > +	if (!read_whole(client->fd, buf, sizeof(buf)))
> > +		return false;
> > +
> > +	_type = ntohs(*(uint16_t *) &buf[0]);
> > +	*type = _type & 0xFF;
> > +	*kind = _type >> 8;
> > +	*err = ntohs(*(uint16_t *) &buf[2]);
> > +	*len = ntohl(*(uint32_t *) &buf[4]);
> > +
> > +	//igt_debug("received message: kind=%d type=%d err=%d len=%zu\n",
> > +	//	  *kind, *type, *err, *len);
> 
> You probably want to keep this debug message. If not, then comment it
> properly with /* */ or delete it.

Ah, seems like I forgot to remove it. I don't think we want to keep it,
because it's extremely chatty when receiving audio pages.

> > +
> > +	return true;
> > +}
> > +
> > +static bool chamelium_stream_write_header(struct chamelium_stream *client,
> > +					  enum stream_message_type type,
> > +					  enum stream_error err,
> > +					  size_t len)
> > +{
> > +	char buf[8];
> > +	uint16_t _type;
> > +
> > +	_type = type | (STREAM_MESSAGE_REQUEST << 8);
> > +
> > +	*(uint16_t *) &buf[0] = htons(_type);
> > +	*(uint16_t *) &buf[2] = htons(err);
> > +	*(uint32_t *) &buf[4] = htonl(len);
> > +
> > +	return write_whole(client->fd, buf, sizeof(buf));
> > +}
> > +
> > +static bool chamelium_stream_read_response(struct chamelium_stream *client,
> > +					   enum stream_message_type type,
> > +					   void *buf, size_t buf_len)
> > +{
> > +	enum stream_message_kind read_kind;
> > +	enum stream_message_type read_type;
> > +	enum stream_error read_err;
> > +	size_t read_len;
> > +
> > +	if (!chamelium_stream_read_header(client, &read_kind, &read_type,
> > +					  &read_err, &read_len))
> > +		return false;
> > +
> > +	if (read_kind != STREAM_MESSAGE_RESPONSE) {
> > +		igt_warn("Expected a response, got kind %d\n", read_kind);
> > +		return false;
> > +	}
> > +	if (read_type != type) {
> > +		igt_warn("Expected message type %d, got %d\n",
> > +			 type, read_type);
> > +		return false;
> > +	}
> > +	if (read_err != STREAM_ERROR_NONE) {
> > +		igt_warn("Received error: %s (%d)\n",
> > +			 stream_error_str(read_err), read_err);
> > +		return false;
> > +	}
> > +	if (buf_len != read_len) {
> > +		igt_warn("Received invalid message body size "
> > +			 "(got %zu bytes, want %zu bytes)\n",
> > +			 read_len, buf_len);
> > +		return false;
> > +	}
> > +
> > +	return read_whole(client->fd, buf, buf_len);
> > +}
> > +
> > +static bool chamelium_stream_write_request(struct chamelium_stream *client,
> > +					   enum stream_message_type type,
> > +					   void *buf, size_t buf_len)
> > +{
> > +	if (!chamelium_stream_write_header(client, type, STREAM_ERROR_NONE,
> > +					   buf_len))
> > +		return false;
> > +
> > +	if (buf_len == 0)
> > +		return true;
> > +
> > +	return write_whole(client->fd, buf, buf_len);
> > +}
> > +
> > +static bool chamelium_stream_call(struct chamelium_stream *client,
> > +				  enum stream_message_type type,
> > +				  void *req_buf, size_t req_len,
> > +				  void *resp_buf, size_t resp_len)
> > +{
> > +	if (!chamelium_stream_write_request(client, type, req_buf, req_len))
> > +		return false;
> > +
> > +	return chamelium_stream_read_response(client, type, resp_buf, resp_len);
> > +}
> > +
> > +static bool chamelium_stream_check_version(struct chamelium_stream *client)
> > +{
> > +	char resp[2];
> > +	uint8_t major, minor;
> > +
> > +	if (!chamelium_stream_call(client, STREAM_MESSAGE_GET_VERSION,
> > +				   NULL, 0, resp, sizeof(resp)))
> > +		return false;
> > +
> > +	major = resp[0];
> > +	minor = resp[1];
> > +	if (major != STREAM_VERSION_MAJOR || minor < STREAM_VERSION_MINOR) {
> > +		igt_warn("Version mismatch (want %d.%d, got %d.%d)\n",
> > +			 STREAM_VERSION_MAJOR, STREAM_VERSION_MINOR,
> > +			 major, minor);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * chamelium_stream_dump_realtime_audio:
> > + *
> > + * Starts audio capture. The caller can then call
> > + * #chamelium_stream_receive_realtime_audio to receive audio pages.
> > + */
> > +bool chamelium_stream_dump_realtime_audio(struct chamelium_stream *client,
> > +					  enum chamelium_stream_realtime_mode mode)
> > +{
> > +	char req[1];
> > +
> > +	igt_debug("Starting real-time audio capture\n");
> > +
> > +	req[0] = mode;
> > +	return chamelium_stream_call(client, STREAM_MESSAGE_DUMP_REALTIME_AUDIO,
> > +				     req, sizeof(req), NULL, 0);
> > +}
> > +
> > +/**
> > + * chamelium_stream_receive_realtime_audio:
> > + * @page_count: if non-NULL, will be set to the dumped page number
> > + * @buf: must either point to a dynamically allocated memory region or NULL
> > + * @buf_len: number of elements of *@buf, for zero if @buf is NULL
> > + *
> > + * Receives one audio page from the streaming server.
> > + *
> > + * In "best effort" mode, some pages can be dropped. This can be detected via
> > + * the page count.
> > + *
> > + * buf_len will be set to the size of the page. The caller is responsible for
> > + * calling free(3) on *buf.
> > + */
> > +bool chamelium_stream_receive_realtime_audio(struct chamelium_stream *client,
> > +					     size_t *page_count,
> > +					     int32_t **buf, size_t *buf_len)
> > +{
> > +	enum stream_message_kind kind;
> > +	enum stream_message_type type;
> > +	enum stream_error err;
> > +	size_t body_len;
> > +	char page_count_buf[4];
> > +	int32_t *ptr;
> > +
> > +	while (true) {
> > +		if (!chamelium_stream_read_header(client, &kind, &type,
> > +						  &err, &body_len))
> > +			return false;
> > +
> > +		if (kind != STREAM_MESSAGE_DATA) {
> > +			igt_warn("Expected a data message, got kind %d\n", kind);
> > +			return false;
> > +		}
> > +		if (type != STREAM_MESSAGE_DUMP_REALTIME_AUDIO) {
> > +			igt_warn("Expected real-time audio dump message, "
> > +				 "got type %d\n", type);
> > +			return false;
> > +		}
> > +
> > +		if (err == STREAM_ERROR_NONE)
> > +			break;
> > +		else if (err != STREAM_ERROR_AUDIO_MEM_OVERFLOW_DROP) {
> > +			igt_warn("Received error: %s (%d)\n",
> > +				 stream_error_str(err), err);
> > +			return false;
> > +		}
> > +
> > +		igt_debug("Dropped an audio page because of an overflow\n");
> > +		igt_assert(body_len == 0);
> > +	}
> > +
> > +	igt_assert(body_len >= sizeof(page_count_buf));
> > +
> > +	if (!read_whole(client->fd, page_count_buf, sizeof(page_count_buf)))
> > +		return false;
> > +	if (page_count)
> > +		*page_count = ntohl(*(uint32_t *) &page_count_buf[0]);
> > +	body_len -= sizeof(page_count_buf);
> > +
> > +	igt_assert(body_len % sizeof(int32_t) == 0);
> > +	if (*buf_len * sizeof(int32_t) != body_len) {
> > +		ptr = realloc(*buf, body_len);
> > +		if (!ptr) {
> > +			igt_warn("realloc failed: %s\n", strerror(errno));
> > +			return false;
> > +		}
> > +		*buf = ptr;
> > +		*buf_len = body_len / sizeof(int32_t);
> > +	}
> > +
> > +	return read_whole(client->fd, *buf, body_len);
> > +}
> > +
> > +/**
> > + * chamelium_stream_stop_realtime_audio:
> > + *
> > + * Stops real-time audio capture. This also drops any buffered audio pages.
> > + * The caller shouldn't call #chamelium_stream_receive_realtime_audio after
> > + * stopping audio capture.
> > + */
> > +bool chamelium_stream_stop_realtime_audio(struct chamelium_stream *client)
> > +{
> > +	enum stream_message_kind kind;
> > +	enum stream_message_type type;
> > +	enum stream_error err;
> > +	size_t len;
> > +
> > +	igt_debug("Stopping real-time audio capture\n");
> > +
> > +	if (!chamelium_stream_write_request(client,
> > +					    STREAM_MESSAGE_STOP_DUMP_AUDIO,
> > +					    NULL, 0))
> > +		return false;
> > +
> > +	while (true) {
> > +		if (!chamelium_stream_read_header(client, &kind, &type,
> > +						  &err, &len))
> > +			return false;
> > +
> > +		if (kind == STREAM_MESSAGE_RESPONSE)
> > +			break;
> > +
> > +		if (!read_and_discard(client->fd, len))
> > +			return false;
> > +	}
> > +
> > +	if (type != STREAM_MESSAGE_STOP_DUMP_AUDIO) {
> > +		igt_warn("Unexpected response type %d\n", type);
> > +		return false;
> > +	}
> > +	if (err != STREAM_ERROR_NONE) {
> > +		igt_warn("Received error: %s (%d)\n",
> > +			 stream_error_str(err), err);
> > +		return false;
> > +	}
> > +	if (len != 0) {
> > +		igt_warn("Expected an empty response, got %zu bytes\n", len);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +/**
> > + * chamelium_stream_audio_format:
> > + *
> > + * Gets the format used for audio pages.
> > + *
> > + * Data will always be captured in raw pages of S32_LE elements. This function
> > + * exposes the sampling rate and the number of channels.
> > + */
> > +void chamelium_stream_audio_format(struct chamelium_stream *stream,
> > +				   int *rate, int *channels)
> > +{
> > +	/* TODO: the Chamelium streaming server doesn't expose those yet.
> > +	 * Just hardcode the values for now. */
> > +	*rate = 48000;
> > +	*channels = 8;
> > +}
> > +
> > +/**
> > + * chamelium_stream_init:
> > + *
> > + * Connects to the Chamelium streaming server.
> > + */
> > +struct chamelium_stream *chamelium_stream_init(void)
> > +{
> > +	struct chamelium_stream *client;
> > +
> > +	client = calloc(1, sizeof(*client));
> > +
> > +	if (!chamelium_stream_read_config(client))
> > +		goto error_client;
> > +	if (!chamelium_stream_connect(client))
> > +		goto error_client;
> > +	if (!chamelium_stream_check_version(client))
> > +		goto error_fd;
> > +
> > +	return client;
> > +
> > +error_fd:
> > +	close(client->fd);
> > +error_client:
> > +	free(client);
> > +	return NULL;
> > +}
> > +
> > +void chamelium_stream_deinit(struct chamelium_stream *client)
> > +{
> > +	if (close(client->fd) != 0)
> > +		igt_warn("close failed: %s\n", strerror(errno));
> > +	free(client);
> > +}
> > diff --git a/lib/igt_chamelium_stream.h b/lib/igt_chamelium_stream.h
> > new file mode 100644
> > index 00000000..de4e9931
> > --- /dev/null
> > +++ b/lib/igt_chamelium_stream.h
> > @@ -0,0 +1,52 @@
> > +/*
> > + * Copyright © 2019 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: Simon Ser <simon.ser at intel.com>
> > + */
> > +
> > +#ifndef IGT_CHAMELIUM_STREAM_H
> > +#define IGT_CHAMELIUM_STREAM_H
> > +
> > +#include "config.h"
> > +
> > +enum chamelium_stream_realtime_mode {
> > +	CHAMELIUM_STREAM_REALTIME_NONE = 0,
> > +	/* stop dumping when overflow */
> > +	CHAMELIUM_STREAM_REALTIME_STOP_WHEN_OVERFLOW = 1,
> > +	/* drop data on overflow */
> > +	CHAMELIUM_STREAM_REALTIME_BEST_EFFORT = 2,
> > +};
> > +
> > +struct chamelium_stream;
> > +
> > +struct chamelium_stream *chamelium_stream_init(void);
> > +void chamelium_stream_deinit(struct chamelium_stream *client);
> > +bool chamelium_stream_dump_realtime_audio(struct chamelium_stream *client,
> > +					  enum chamelium_stream_realtime_mode mode);
> > +void chamelium_stream_audio_format(struct chamelium_stream *stream,
> > +				   int *rate, int *channels);
> > +bool chamelium_stream_receive_realtime_audio(struct chamelium_stream *client,
> > +					     size_t *page_count,
> > +					     int32_t **buf, size_t *buf_len);
> > +bool chamelium_stream_stop_realtime_audio(struct chamelium_stream *client);
> > +
> > +#endif
> > diff --git a/lib/meson.build b/lib/meson.build
> > index a8462933..eead0afb 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -92,7 +92,7 @@ if valgrind.found()
> >  endif
> >  
> >  if gsl.found()
> > -	lib_deps += [ gsl ]
> > +	lib_deps += gsl
> >  	lib_sources += [ 'igt_frame.c', 'igt_audio.c' ]
> >  endif
> >  
> > @@ -101,9 +101,10 @@ if alsa.found()
> >  	lib_sources += 'igt_alsa.c'
> >  endif
> >  
> > -if chamelium.found()
> > +if chamelium_found
> >  	lib_deps += chamelium
> >  	lib_sources += 'igt_chamelium.c'
> > +	lib_sources += 'igt_chamelium_stream.c'
> >  endif
> >  
> >  srcdir = join_paths(meson.source_root(), 'tests')
> > diff --git a/meson.build b/meson.build
> > index 557400a5..be6dff9d 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -64,8 +64,6 @@ _build_overlay = false
> >  _overlay_required = false
> >  _build_man = false
> >  _man_required = false
> > -_build_audio = false
> > -_audio_required = false
> >  _build_chamelium = false
> >  _chamelium_required = false
> >  _build_docs = false
> > @@ -79,7 +77,6 @@ build_overlay = get_option('build_overlay')
> >  overlay_backends = get_option('overlay_backends')
> >  build_man = get_option('build_man')
> >  with_valgrind = get_option('with_valgrind')
> > -build_audio = get_option('build_audio')
> >  build_chamelium = get_option('build_chamelium')
> >  build_docs = get_option('build_docs')
> >  build_tests = get_option('build_tests')
> > @@ -91,8 +88,6 @@ _build_overlay = build_overlay != 'false'
> >  _overlay_required = build_overlay == 'true'
> >  _build_man = build_man != 'false'
> >  _man_required = build_man == 'true'
> > -_build_audio = build_audio != 'false'
> > -_audio_required = build_audio == 'true'
> >  _build_chamelium = build_chamelium != 'false'
> >  _chamelium_required = build_chamelium == 'true'
> >  _build_docs = build_docs != 'false'
> > @@ -166,26 +161,6 @@ cairo = dependency('cairo', version : '>1.12.0', required : true)
> >  libudev = dependency('libudev', required : true)
> >  glib = dependency('glib-2.0', required : true)
> >  
> > -gsl = null_dep
> > -alsa = null_dep
> > -if _build_audio or _build_chamelium
> > -	gsl = dependency('gsl', required : _audio_required or _chamelium_required)
> > -endif
> > -if _build_audio
> > -	alsa = dependency('alsa', required : _audio_required)
> > -endif
> > -
> > -audioinfo = 'No'
> > -if _build_audio and alsa.found() and gsl.found()
> > -	audioinfo = 'Yes'
> > -else
> > -	if _audio_required
> > -		error('Cannot build audio test due to missing dependencies')
> > -	endif
> > -	_build_audio = false
> > -endif
> > -build_info += 'Build audio test: ' + audioinfo
> > -
> >  xmlrpc = dependency('xmlrpc', required : false)
> >  xmlrpc_util = dependency('xmlrpc_util', required : false)
> >  xmlrpc_client = dependency('xmlrpc_client', required : false)
> > @@ -197,21 +172,32 @@ if not xmlrpc.found() and xmlrpc_cmd.found()
> >  
> >  	if libs_cmd.returncode() == 0 and cflags_cmd.returncode() == 0
> >  		xmlrpc = declare_dependency(compile_args: cflags_cmd.stdout().strip().split(),
> > -					   link_args : libs_cmd.stdout().strip().split())
> > +					    link_args : libs_cmd.stdout().strip().split())
> >  		xmlrpc_util = declare_dependency()
> >  		xmlrpc_client = declare_dependency()
> >  	endif
> >  endif
> >  
> > +gsl = null_dep
> > +alsa = null_dep
> >  chamelium = null_dep
> > +chamelium_found = false # TODO: use a disabler object instead
> >  chameliuminfo = 'No'
> > -if _build_chamelium and gsl.found() and xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found()
> > -	chamelium = declare_dependency(dependencies : [ xmlrpc,
> > -							xmlrpc_util, xmlrpc_client])
> > -	config.set('HAVE_CHAMELIUM', 1)
> > -	chameliuminfo = 'Yes'
> > -elif _chamelium_required
> > -	error('Cannot build chamelium test due to missing dependencies')
> > +if _build_chamelium
> > +	gsl = dependency('gsl', required : _chamelium_required)
> > +	alsa = dependency('alsa', required : _chamelium_required)
> > +	chamelium = declare_dependency(dependencies : [
> > +		xmlrpc,
> > +		xmlrpc_util,
> > +		xmlrpc_client,
> > +		gsl,
> > +		alsa,
> > +	], required : _chamelium_required)
> > +	if xmlrpc.found() and xmlrpc_util.found() and xmlrpc_client.found() and gsl.found() and alsa.found()
> > +		config.set('HAVE_CHAMELIUM', 1)
> > +		chameliuminfo = 'Yes'
> > +		chamelium_found = true
> > +	endif
> >  endif
> >  build_info += 'Build Chamelium test: ' + chameliuminfo
> >  
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 0cd3b350..888efe56 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -10,12 +10,6 @@ option('overlay_backends',
> >         choices : [ 'auto', 'x', 'xv' ],
> >         description : 'Overlay backends to enable')
> >  
> > -option('build_audio',
> > -       type : 'combo',
> > -       value : 'auto',
> > -       choices : ['auto', 'true', 'false'],
> > -       description : 'Build audio test')
> > -
> >  option('build_chamelium',
> >         type : 'combo',
> >         value : 'auto',
> > diff --git a/tests/audio.c b/tests/audio.c
> > deleted file mode 100644
> > index 560876a3..00000000
> > --- a/tests/audio.c
> > +++ /dev/null
> > @@ -1,193 +0,0 @@
> > -/*
> > - * 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 <paul.kocialkowski at linux.intel.com>
> > - */
> > -
> > -#include "config.h"
> > -#include "igt.h"
> > -
> > -#define PLAYBACK_CHANNELS	2
> > -#define PLAYBACK_FRAMES		1024
> > -
> > -#define CAPTURE_SAMPLE_RATE	48000
> > -#define CAPTURE_CHANNELS	2
> > -#define CAPTURE_DEVICE_NAME	"default"
> > -#define CAPTURE_FRAMES		2048
> > -
> > -#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,
> > -	80000,
> > -	10000,
> > -};
> > -
> > -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_input(data.alsa, CAPTURE_DEVICE_NAME);
> > -	igt_assert(ret >= 0);
> > -
> > -	alsa_configure_input(data.alsa, CAPTURE_CHANNELS,
> > -			     CAPTURE_SAMPLE_RATE);
> > -
> > -	alsa_register_input_callback(data.alsa, input_callback, &data,
> > -				     CAPTURE_FRAMES);
> > -
> > -	for (i = 0; i < sampling_rates_count; i++) {
> > -		ret = alsa_open_output(data.alsa, device_name);
> > -		igt_assert(ret >= 0);
> > -
> > -		sampling_rate = sampling_rates[i];
> > -
> > -		test = alsa_test_output_configuration(data.alsa,
> > -						      PLAYBACK_CHANNELS,
> > -						      sampling_rate);
> > -		if (!test) {
> > -			alsa_close_output(data.alsa);
> > -			continue;
> > -		}
> > -
> > -		igt_debug("Testing with sampling rate %d\n", sampling_rate);
> > -
> > -		alsa_configure_output(data.alsa, PLAYBACK_CHANNELS,
> > -				       sampling_rate);
> > -
> > -		data.signal = audio_signal_init(PLAYBACK_CHANNELS,
> > -						sampling_rate);
> > -		igt_assert(data.signal);
> > -
> > -		for (j = 0; j < test_frequencies_count; j++)
> > -			audio_signal_add_frequency(data.signal,
> > -						   test_frequencies[j]);
> > -
> > -		audio_signal_synthesize(data.signal);
> > -
> > -		alsa_register_output_callback(data.alsa, output_callback,
> > -					      &data, PLAYBACK_FRAMES);
> > -
> > -		data.streak = 0;
> > -
> > -		ret = alsa_run(data.alsa, RUN_TIMEOUT);
> > -		igt_assert(ret > 0);
> > -
> > -		audio_signal_clean(data.signal);
> > -		free(data.signal);
> > -
> > -		alsa_close_output(data.alsa);
> > -
> > -		run = true;
> > -	}
> > -
> > -	/* Make sure we tested at least one frequency */
> > -	igt_assert(run);
> > -
> > -	alsa_close_input(data.alsa);
> > -	free(data.alsa);
> > -}
> > -
> > -static void test_suspend_resume_integrity(const char *device_name,
> > -					  enum igt_suspend_state state,
> > -					  enum igt_suspend_test test)
> > -{
> > -	test_integrity(device_name);
> > -
> > -	igt_system_suspend_autoresume(state, test);
> > -
> > -	test_integrity(device_name);
> > -}
> > -
> > -igt_main
> > -{
> > -	igt_subtest("hdmi-integrity")
> > -		test_integrity("HDMI");
> > -
> > -	igt_subtest("hdmi-integrity-after-suspend")
> > -		test_suspend_resume_integrity("HDMI", SUSPEND_STATE_MEM,
> > -					      SUSPEND_TEST_NONE);
> > -
> > -	igt_subtest("hdmi-integrity-after-hibernate")
> > -		test_suspend_resume_integrity("HDMI", SUSPEND_STATE_DISK,
> > -					      SUSPEND_TEST_DEVICES);
> > -}
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 2dc1049d..2974ff69 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -413,7 +413,7 @@ test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
> >  
> >  static igt_output_t *
> >  prepare_output(data_t *data,
> > -	       struct chamelium_port *port)
> > +	       struct chamelium_port *port, bool set_edid)
> >  {
> >  	igt_display_t *display = &data->display;
> >  	igt_output_t *output;
> > @@ -428,7 +428,8 @@ prepare_output(data_t *data,
> >  	/* The chamelium's default EDID has a lot of resolutions, way more then
> >  	 * we need to test
> >  	 */
> > -	chamelium_port_set_edid(data->chamelium, port, data->edid_id);
> > +	if (set_edid)
> > +		chamelium_port_set_edid(data->chamelium, port, data->edid_id);
> >  
> >  	chamelium_plug(data->chamelium, port);
> >  	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> > @@ -613,7 +614,7 @@ static void test_display_one_mode(data_t *data, struct chamelium_port *port,
> >  
> >  	reset_state(data, port);
> >  
> > -	output = prepare_output(data, port);
> > +	output = prepare_output(data, port, true);
> >  	connector = chamelium_port_get_connector(data->chamelium, port, false);
> >  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> >  	igt_assert(primary);
> > @@ -644,7 +645,7 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
> >  
> >  	reset_state(data, port);
> >  
> > -	output = prepare_output(data, port);
> > +	output = prepare_output(data, port, true);
> >  	connector = chamelium_port_get_connector(data->chamelium, port, false);
> >  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> >  	igt_assert(primary);
> > @@ -679,7 +680,7 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> >  
> >  	reset_state(data, port);
> >  
> > -	output = prepare_output(data, port);
> > +	output = prepare_output(data, port, true);
> >  	connector = chamelium_port_get_connector(data->chamelium, port, false);
> >  	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> >  	igt_assert(primary);
> > @@ -710,6 +711,266 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> >  	drmModeFreeConnector(connector);
> >  }
> >  
> > +
> > +/* Playback parameters control the audio signal we synthesize and send */
> > +#define PLAYBACK_CHANNELS 2
> > +#define PLAYBACK_SAMPLES 1024
> > +
> > +/* Capture paremeters control the audio signal we receive */
> > +#define CAPTURE_SAMPLES 2048
> > +
> > +#define AUDIO_DURATION 2000 /* ms */
> > +/* A streak of 3 gives confidence that the signal is good. */
> > +#define MIN_STREAK 3
> > +
> > +/* TODO: Chamelium only supports 48KHz for now */
> > +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,
> > +	80000,
> > +	10000,
> > +};
> > +
> > +static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
> > +
> > +static int
> > +output_callback(void *data, short *buffer, int frames)
> > +{
> > +	struct audio_signal *signal = (struct audio_signal *) data;
> > +
> > +	audio_signal_fill(signal, buffer, frames);
> > +
> > +	return 0;
> > +}
> > +
> > +static bool
> > +do_test_display_audio(data_t *data, struct chamelium_port *port,
> > +		      struct alsa *alsa, int playback_channels,
> > +		      int playback_rate)
> > +{
> > +	int ret, capture_rate, capture_channels, msec;
> > +	struct chamelium_audio_file *audio_file;
> > +	struct chamelium_stream *stream;
> > +	enum chamelium_stream_realtime_mode stream_mode;
> > +	struct audio_signal *signal;
> > +	int32_t *recv, *buf;
> > +	double *channel;
> > +	size_t i, streak, page_count;
> > +	size_t recv_len, buf_len, buf_cap, buf_size, channel_len;
> > +	bool ok;
> > +	char dump_suffix[64];
> > +	char *dump_path = NULL;
> > +	int dump_fd = -1;
> > +
> > +	if (!alsa_test_output_configuration(alsa, playback_channels,
> > +					    playback_rate))
> > +		return false;
> > +
> > +	igt_debug("Testing with playback sampling rate %d\n", playback_rate);
> > +	alsa_configure_output(alsa, playback_channels, playback_rate);
> > +
> > +	chamelium_start_capturing_audio(data->chamelium, port, false);
> > +
> > +	stream = chamelium_stream_init();
> > +	igt_assert(stream);
> > +
> > +	stream_mode = CHAMELIUM_STREAM_REALTIME_STOP_WHEN_OVERFLOW;
> > +	ok = chamelium_stream_dump_realtime_audio(stream, stream_mode);
> > +	igt_assert(ok);
> > +
> > +	chamelium_stream_audio_format(stream, &capture_rate, &capture_channels);
> > +
> > +	if (igt_frame_dump_is_enabled()) {
> > +		snprintf(dump_suffix, sizeof(dump_suffix), "capture-%dch-%d",
> > +			 playback_channels, playback_rate);
> > +
> > +		dump_fd = audio_create_wav_file_s32_le(dump_suffix,
> > +						       capture_rate,
> > +						       capture_channels,
> > +						       &dump_path);
> > +		igt_assert(dump_fd >= 0);
> > +	}
> > +
> > +	signal = audio_signal_init(playback_channels, playback_rate);
> > +	igt_assert(signal);
> > +
> > +	for (i = 0; i < test_frequencies_count; i++)
> > +		audio_signal_add_frequency(signal, test_frequencies[i]);
> > +	audio_signal_synthesize(signal);
> > +
> > +	alsa_register_output_callback(alsa, output_callback, signal,
> > +				      PLAYBACK_SAMPLES);
> > +
> > +	/* TODO: detect signal in real-time */
> > +	ret = alsa_run(alsa, AUDIO_DURATION);
> > +	igt_assert(ret == 0);
> > +
> > +	alsa_close_output(alsa);
> > +
> > +	/* Needs to be a multiple of 128, because that's the number of samples
> > +	 * we get per channel each time we receive an audio page from the
> > +	 * Chamelium device. */
> > +	channel_len = CAPTURE_SAMPLES;
> > +	channel = malloc(sizeof(double) * channel_len);
> > +
> > +	buf_cap = capture_channels * channel_len;
> > +	buf = malloc(sizeof(int32_t) * buf_cap);
> > +	buf_len = 0;
> > +
> > +	recv = NULL;
> > +	recv_len = 0;
> > +
> > +	streak = 0;
> > +	msec = 0;
> > +	i = 0;
> > +	while (streak < MIN_STREAK && msec < AUDIO_DURATION) {
> > +		ok = chamelium_stream_receive_realtime_audio(stream,
> > +							     &page_count,
> > +							     &recv, &recv_len);
> > +		igt_assert(ok);
> > +
> > +		memcpy(&buf[buf_len], recv, recv_len * sizeof(int32_t));
> > +		buf_len += recv_len;
> > +
> > +		if (buf_len < buf_cap)
> > +			continue;
> > +		igt_assert(buf_len == buf_cap);
> > +
> > +		if (dump_fd >= 0) {
> > +			buf_size = buf_len * sizeof(int32_t);
> > +			igt_assert(write(dump_fd, buf, buf_size) == buf_size);
> > +		}
> > +
> > +		/* TODO: check other channels too, not just the first one */
> > +		audio_extract_channel_s32_le(channel, channel_len, buf, buf_len,
> > +					     capture_channels, 0);
> > +
> > +		msec = i * channel_len / (double) capture_rate * 1000;
> > +		igt_debug("Detecting audio signal, t=%d msec\n", msec);
> > +
> > +		if (audio_signal_detect(signal, capture_rate, channel,
> > +					channel_len))
> > +			streak++;
> > +		else
> > +			streak = 0;
> > +
> > +		buf_len = 0;
> > +		i++;
> > +	}
> > +
> > +	if (dump_fd >= 0) {
> > +		close(dump_fd);
> > +		if (streak == MIN_STREAK) {
> > +			/* Test succeeded, no need to keep the captured data */
> > +			unlink(dump_path);
> > +		} else
> > +			igt_debug("Saved captured audio data to %s\n", dump_path);
> > +		free(dump_path);
> > +	}
> > +
> > +	free(recv);
> > +	free(buf);
> > +	free(channel);
> > +
> > +	ok = chamelium_stream_stop_realtime_audio(stream);
> > +	igt_assert(ok);
> > +
> > +	audio_file = chamelium_stop_capturing_audio(data->chamelium,
> > +						    port);
> > +	if (audio_file) {
> > +		igt_debug("Audio file saved on the Chamelium in %s\n",
> > +			  audio_file->path);
> > +		chamelium_destroy_audio_file(audio_file);
> > +	}
> 
> I would suggest to only dump this file on failure, not when having a
> success.

1. We can't decide this after-the-fact: we can only decide whether we
   dump or not before starting the capture.
2. There are two kinds of audio dumps: local (on the DUT, see dump_fd) 
   and remote (on the Chamelium, see the last param of 
   chamelium_start_capturing_audio). If the file has been dumped on the
   Chamelium, chamelium_stop_capturing_audio will return the audio file
   details. It's sometimes useful to enable Chamelium dumps for
   debugging purposes.

> > +
> > +	audio_signal_clean(signal);
> > +	free(signal);
> > +
> > +	chamelium_stream_deinit(stream);
> > +
> > +	igt_assert(streak == MIN_STREAK);
> > +	return true;
> > +}
> > +
> > +static void
> > +test_display_audio(data_t *data, struct chamelium_port *port,
> > +		   const char *audio_device)
> > +{
> > +	bool run = false;
> > +	struct alsa *alsa;
> > +	int ret;
> > +	igt_output_t *output;
> > +	igt_plane_t *primary;
> > +	struct igt_fb fb;
> > +	drmModeModeInfo *mode;
> > +	drmModeConnector *connector;
> > +	int fb_id, i;
> > +
> > +	igt_require(alsa_has_exclusive_access());
> > +
> > +	alsa = alsa_init();
> > +	igt_assert(alsa);
> > +
> > +	reset_state(data, port);
> > +
> > +	/* Use the default Chamelium EDID for this test, as the base IGT EDID
> > +	 * doesn't advertise audio support (see drm_detect_monitor_audio in
> > +	 * the kernel tree). */
> > +	output = prepare_output(data, port, false);
> > +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> > +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > +	igt_assert(primary);
> > +
> > +	igt_assert(connector->count_modes > 0);
> > +	mode = &connector->modes[0];
> > +
> > +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
> > +					    mode->hdisplay, mode->vdisplay,
> > +					    DRM_FORMAT_XRGB8888,
> > +					    LOCAL_DRM_FORMAT_MOD_NONE,
> > +					    0, 0, 0, &fb);
> > +	igt_assert(fb_id > 0);
> > +
> > +	/* Enable the output because the receiver won't try to receive audio if
> > +	 * it doesn't receive video. */
> Maybe move the above comment to above igt_assert(connector->count_modes > 0); ?

Ack

> > +	enable_output(data, port, output, mode, &fb);
> > +
> > +	for (i = 0; i < sampling_rates_count; i++) {
> > +		ret = alsa_open_output(alsa, audio_device);
> > +		igt_assert(ret >= 0);
> > +
> > +		/* TODO: playback on all 8 available channels */
> > +		run |= do_test_display_audio(data, port, alsa,
> > +					     PLAYBACK_CHANNELS,
> > +					     sampling_rates[i]);
> > +
> > +		alsa_close_output(alsa);
> > +	}
> > +
> > +	/* Make sure we tested at least one frequency. */
> > +	igt_assert(run);
> > +
> > +	igt_remove_fb(data->drm_fd, &fb);
> > +
> > +	drmModeFreeConnector(connector);
> > +
> > +	free(alsa);
> > +}
> > +
> > +
> >  static void select_tiled_modifier(igt_plane_t *plane, uint32_t width,
> >  				  uint32_t height, uint32_t format,
> >  				  uint64_t *modifier)
> > @@ -1037,7 +1298,7 @@ static void test_display_planes_random(data_t *data,
> >  	reset_state(data, port);
> >  
> >  	/* Find the connector and pipe. */
> > -	output = prepare_output(data, port);
> > +	output = prepare_output(data, port, true);
> >  
> >  	mode = igt_output_get_mode(output);
> >  
> > @@ -1308,6 +1569,9 @@ igt_main
> >  
> >  		connector_subtest("dp-frame-dump", DisplayPort)
> >  			test_display_frame_dump(&data, port);
> > +
> > +		connector_subtest("dp-audio", DisplayPort)
> > +			test_display_audio(&data, port, "HDMI");
> >  	}
> >  
> >  	igt_subtest_group {
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 5167a6cc..5d3eed82 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -238,20 +238,13 @@ if libdrm_nouveau.found()
> >  	test_deps += libdrm_nouveau
> >  endif
> >  
> > -if _build_chamelium and chamelium.found()
> > +if chamelium_found
> >  	test_progs += [
> >  		'kms_chamelium',
> >  	]
> >  	test_deps += chamelium
> >  endif
> >  
> > -if _build_audio and alsa.found() and gsl.found()
> > -	test_progs += [
> > -		'audio',
> > -	]
> > -	test_deps += alsa
> > -endif
> > -
> >  test_executables = []
> >  test_list = []
> >  
> > 
> 
> Well... that was a mouthful! For the next patches, please split stuff
> more aggressively, even when a patch would only be introducing dead code.

Yes, I promise not to do this again. :S

Thanks for the review!

> With these changed:
> 
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
> 
> Martin


More information about the igt-dev mailing list