[igt-dev] [PATCH i-g-t v4 1/5] tests/kms_chamelium: add dp-audio test
Martin Peres
martin.peres at linux.intel.com
Wed Apr 17 12:17:12 UTC 2019
On 17/04/2019 11:40, Ser, Simon wrote:
> 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.
Just write it in the commit message.
>
>>> }
>>>
>>> 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
Then keep amplitude, that's ok!
>
>>> 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.
Yeah, Arek agrees :)
>
>>>
>>> - 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.
agreed. Don't worry about it.
>
>>>
>>> - 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.
> */
Looks good!
>
>> 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.
ok!
>
>>> +{
>>> + 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.
ok!
>
>>> +
>>> + 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.
Of course! Sorry for the confusion! Where are we dumping the generated
and received WAVs when the test is failing then? Is that a TODO?
>
>>> +
>>> + 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!
You're welcome!
Feel free to land this patch with the modifications you proposed /
agreed to change.
>
>> With these changed:
>>
>> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
>>
>> Martin
More information about the igt-dev
mailing list