lib/igt_alsa: support all alsa formats

This allows us to test whether HW supports a PCM format, and only run the test if it's the case.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_alsa.c        | 18 +++++++++++++-----
 lib/igt_alsa.h        |  4 ++--
 tests/kms_chamelium.c | 31 ++++++++++++++++++++-----------
 3 files changed, 35 insertions(+), 18 deletions(-)

lib/igt_alsa.c +13 -5

index 2dfb8ccd3ad9..5b5980a94888 100644
@@ -266,8 +266,8 @@ void alsa_close_output(struct alsa *alsa)
 	alsa->output_callback = NULL;
 }
 
-static bool alsa_test_configuration(snd_pcm_t *handle, int channels,
-			     int sampling_rate)
+static bool alsa_test_configuration(snd_pcm_t *handle, snd_pcm_format_t fmt,
+				    int channels, int sampling_rate)
 {
 	snd_pcm_hw_params_t *params;
 	int ret;

@@ -281,6 +281,13 @@ static bool alsa_test_configuration(snd_pcm_t *handle, int channels,
 	if (ret < 0)
 		return false;
 
+	ret = snd_pcm_hw_params_test_format(handle, params, fmt);
+	if (ret < 0) {
+		igt_debug("Output device doesn't support the format %s\n",
+			  snd_pcm_format_name(fmt));
+		return false;
+	}
+
 	ret = snd_pcm_hw_params_test_rate(handle, params, sampling_rate, 0);
 	if (ret < 0) {
 		snd_pcm_hw_params_get_rate_min(params, &min_rate, &min_rate_dir);

@@ -307,6 +314,7 @@ static bool alsa_test_configuration(snd_pcm_t *handle, int channels,
 /**
  * alsa_test_output_configuration:
  * @alsa: The target alsa structure
+ * @fmt: The format to test
  * @channels: The number of channels to test
  * @sampling_rate: The sampling rate to test
  *

@@ -315,8 +323,8 @@ static bool alsa_test_configuration(snd_pcm_t *handle, int channels,
  *
  * Returns: A boolean indicating whether the test succeeded
  */
-bool alsa_test_output_configuration(struct alsa *alsa, int channels,
-				    int sampling_rate)
+bool alsa_test_output_configuration(struct alsa *alsa, snd_pcm_format_t fmt,
+				    int channels, int sampling_rate)
 {
 	snd_pcm_t *handle;
 	bool ret;

@@ -325,7 +333,7 @@ bool alsa_test_output_configuration(struct alsa *alsa, int channels,
 	for (i = 0; i < alsa->output_handles_count; i++) {
 		handle = alsa->output_handles[i];
 
-		ret = alsa_test_configuration(handle, channels, sampling_rate);
+		ret = alsa_test_configuration(handle, fmt, channels, sampling_rate);
 		if (!ret)
 			return false;
 	}

lib/igt_alsa.h +2 -2

index 46b3568d26fd..1ece9f5255d8 100644
@@ -38,8 +38,8 @@ bool alsa_has_exclusive_access(void);
 struct alsa *alsa_init(void);
 int alsa_open_output(struct alsa *alsa, const char *device_name);
 void alsa_close_output(struct alsa *alsa);
-bool alsa_test_output_configuration(struct alsa *alsa, int channels,
-				    int sampling_rate);
+bool alsa_test_output_configuration(struct alsa *alsa, snd_pcm_format_t dmt,
+				    int channels, int sampling_rate);
 void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
 			   int channels, int sampling_rate);
 void alsa_register_output_callback(struct alsa *alsa,

tests/kms_chamelium.c +18 -9

index 2b465565418d..af9f54d1f4c7 100644
@@ -821,8 +821,8 @@ run_audio_thread(void *data)
 
 static bool
 do_test_display_audio(data_t *data, struct chamelium_port *port,
-		      struct alsa *alsa, int playback_channels,
-		      int playback_rate)
+		      struct alsa *alsa, snd_pcm_format_t playback_format,
+		      int playback_channels, int playback_rate)
 {
 	int ret, capture_rate, capture_channels, msec, freq, step;
 	struct chamelium_audio_file *audio_file;

@@ -841,9 +841,11 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	struct audio_state state = {};
 	int channel_mapping[8], capture_chan;
 
-	igt_debug("Testing with playback sampling rate %d Hz and %d channels\n",
+	igt_debug("Testing with playback format %s, sampling rate %d Hz and "
+		  "%d channels\n",
+		  snd_pcm_format_name(playback_format),
 		  playback_rate, playback_channels);
-	alsa_configure_output(alsa, SND_PCM_FORMAT_S16_LE,
+	alsa_configure_output(alsa, playback_format,
 			      playback_channels, playback_rate);
 
 	chamelium_start_capturing_audio(data->chamelium, port, false);

@@ -913,7 +915,9 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	}
 
 	if (igt_frame_dump_is_enabled()) {
-		snprintf(dump_suffix, sizeof(dump_suffix), "capture-%dch-%d",
+		snprintf(dump_suffix, sizeof(dump_suffix),
+			 "capture-%s-%dch-%dHz",
+			 snd_pcm_format_name(playback_format),
 			 playback_channels, playback_rate);
 
 		dump_fd = audio_create_wav_file_s32_le(dump_suffix,

@@ -1037,6 +1041,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	drmModeConnector *connector;
 	int fb_id, i;
 	int channels, sampling_rate;
+	snd_pcm_format_t format;
 
 	igt_require(alsa_has_exclusive_access());
 

@@ -1074,23 +1079,27 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 		ret = alsa_open_output(alsa, audio_device);
 		igt_assert(ret >= 0);
 
+		/* TODO: playback with different formats */
 		/* TODO: playback on all 8 available channels */
+		format = SND_PCM_FORMAT_S16_LE;
 		channels = PLAYBACK_CHANNELS;
 		sampling_rate = test_sampling_rates[i];
 
-		if (!alsa_test_output_configuration(alsa, channels,
+		if (!alsa_test_output_configuration(alsa, format, channels,
 						    sampling_rate)) {
-			igt_debug("Skipping test with sample rate %d Hz and %d channels "
-				  "because at least one of the selected output devices "
-				  "doesn't support this configuration\n",
+			igt_debug("Skipping test with format %s, sample rate "
+				  "%d Hz and %d channels because at least one "
+				  "of the selected output devices doesn't "
+				  "support this configuration\n",
+				  snd_pcm_format_name(format),
 				  channels, sampling_rate);
 			continue;
 		}

This adds two new helpers to generate S24_LE and S32_LE signals.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_audio.c | 30 ++++++++++++++++++++++++++++++
 lib/igt_audio.h |  4 ++++
 2 files changed, 34 insertions(+)

lib/igt_audio.c +0 -0

index 90d16fe4bd11..876084a994c3 100644
@@ -262,6 +262,36 @@ void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
 	free(tmp);
 }

lib/igt_audio.c +5 -0

+	for (i = 0; i < signal->channels * samples; ++i)
+		buffer[i] = 0xFFFFFF * tmp[i];
+
+	free(tmp);
+}

lib/igt_audio.c +6 -0

+	for (i = 0; i < signal->channels * samples; ++i)
+		buffer[i] = UINT32_MAX * tmp[i];
+
+	free(tmp);
+}
+
 /**
  * Checks that frequencies specified in signal, and only those, are included
  * in the input data.

lib/igt_audio.h +0 -0

index f915d55d63fc..c8de70871faa 100644

Quoted 1 line · Simon Ser

This adds two new helpers to generate S24_LE and S32_LE signals.

I got a little taken aback by the multiplication... until I remembered that the doubles were between [0, 1].

I think this patch could add a small note (see below), but regardless of what you chose to do:

Reviewed-by Martin Peres <[email protected]>

Quoted 13 lines

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_audio.c | 30 ++++++++++++++++++++++++++++++
 lib/igt_audio.h |  4 ++++
 2 files changed, 34 insertions(+)

lib/igt_audio.c +0 -0

index 90d16fe4bd11..876084a994c3 100644
@@ -262,6 +262,36 @@ void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
 	free(tmp);
 }

lib/igt_audio.c void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer, · line 273

Quoted 10 lines

lib/igt_audio.c +9 -0

 
+void audio_signal_fill_s24_le(struct audio_signal *signal, int32_t *buffer,
+			      size_t samples)
+{
+	double *tmp;
+	size_t i;
+
+	tmp = malloc(sizeof(double) * signal->channels * samples);
+	audio_signal_fill(signal, tmp, samples);
+

/* NOTE: audio_signal_fill() returns samples between [0, 1] */

Quoted 5 lines

lib/igt_audio.c +5 -0

+	for (i = 0; i < signal->channels * samples; ++i)
+		buffer[i] = 0xFFFFFF * tmp[i];
+
+	free(tmp);
+}

lib/igt_audio.c void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer, · line 288

Quoted 10 lines

lib/igt_audio.c +10 -0

+
+void audio_signal_fill_s32_le(struct audio_signal *signal, int32_t *buffer,
+			      size_t samples)
+{
+	double *tmp;
+	size_t i;
+
+	tmp = malloc(sizeof(double) * signal->channels * samples);
+	audio_signal_fill(signal, tmp, samples);
+

/* NOTE: audio_signal_fill() returns samples between [0, 1] */

Quoted 17 lines

lib/igt_audio.c +6 -0

+	for (i = 0; i < signal->channels * samples; ++i)
+		buffer[i] = UINT32_MAX * tmp[i];
+
+	free(tmp);
+}
+
 /**
  * Checks that frequencies specified in signal, and only those, are included
  * in the input data.

lib/igt_audio.h +0 -0

index f915d55d63fc..c8de70871faa 100644
@@ -44,6 +44,10 @@ void audio_signal_fill(struct audio_signal *signal, double *buffer,
 		       size_t buffer_len);
 void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
 			      size_t buffer_len);

lib/igt_audio.h void audio_signal_fill(struct audio_signal *signal, double *buffer, · line 53

Quoted 8 lines

lib/igt_audio.h +4 -0

+void audio_signal_fill_s24_le(struct audio_signal *signal, int32_t *buffer,
+			      size_t buffer_len);
+void audio_signal_fill_s32_le(struct audio_signal *signal, int32_t *buffer,
+			      size_t buffer_len);
 bool audio_signal_detect(struct audio_signal *signal, int sampling_rate,
 			 int channel, const double *samples, size_t samples_len);
 size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,

This adds a new test_formats array, which is a list of PCM formats we want to run the tests with. We try to run tests for all combinations of sampling rates and formats.

The ALSA callback now needs to fill the playback buffer differently depending on the format.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 26 deletions(-)

tests/kms_chamelium.c +40 -16

index af9f54d1f4c7..88356dd232b6 100644
@@ -795,8 +795,17 @@ static int test_frequencies[] = {
 
 static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
 
+static const snd_pcm_format_t test_formats[] = {
+	SND_PCM_FORMAT_S16_LE,
+	SND_PCM_FORMAT_S24_LE,
+	SND_PCM_FORMAT_S32_LE,
+};
+
+static const size_t test_formats_count = sizeof(test_formats) / sizeof(test_formats[0]);
+
 struct audio_state {
 	struct audio_signal *signal;
+	snd_pcm_format_t format;
 	atomic_bool run;
 };
 

@@ -805,7 +814,19 @@ audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, samples);
+	switch (state->format) {
+	case SND_PCM_FORMAT_S16_LE:
+		audio_signal_fill_s16_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S24_LE:
+		audio_signal_fill_s24_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S32_LE:
+		audio_signal_fill_s32_le(state->signal, buffer, samples);
+		break;
+	default:
+		assert(false); /* unreachable */
+	}
 
 	return state->run ? 0 : -1;
 }

@@ -881,6 +902,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	audio_signal_synthesize(signal);
 
 	state.signal = signal;
+	state.format = playback_format;
 	state.run = true;
 	alsa_register_output_callback(alsa, audio_output_callback, &state,
 				      PLAYBACK_SAMPLES);

@@ -1027,6 +1049,22 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	return success;
 }
 
+static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
+				     int channels, int sampling_rate)
+{
+	if (!alsa_test_output_configuration(alsa, format, channels,
+					    sampling_rate)) {
+		igt_debug("Skipping test with format %s, sampling rate %d Hz "
+			  "and %d channels because at least one of the "
+			  "selected output devices doesn't support this "
+			  "configuration\n",
+			  snd_pcm_format_name(format),
+			  sampling_rate, channels);
+		return false;
+	}
+	return true;
+}
+
 static void
 test_display_audio(data_t *data, struct chamelium_port *port,
 		   const char *audio_device, enum test_edid edid)

@@ -1039,7 +1077,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	struct igt_fb fb;
 	drmModeModeInfo *mode;
 	drmModeConnector *connector;
-	int fb_id, i;
+	int fb_id, i, j;
 	int channels, sampling_rate;
 	snd_pcm_format_t format;
 

@@ -1076,35 +1114,31 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	run = false;
 	success = true;
 	for (i = 0; i < test_sampling_rates_count; i++) {
-		ret = alsa_open_output(alsa, audio_device);
-		igt_assert(ret >= 0);
-
-		/* TODO: playback with different formats */
-		/* TODO: playback on all 8 available channels */
-		format = SND_PCM_FORMAT_S16_LE;
-		channels = PLAYBACK_CHANNELS;
-		sampling_rate = test_sampling_rates[i];
-
-		if (!alsa_test_output_configuration(alsa, format, channels,
-						    sampling_rate)) {
-			igt_debug("Skipping test with format %s, sample rate "
-				  "%d Hz and %d channels because at least one "
-				  "of the selected output devices doesn't "

tests/kms_chamelium.c +9 -4

+			/* TODO: playback on all 8 available channels */
+			format = test_formats[j];
+			channels = PLAYBACK_CHANNELS;
+			sampling_rate = test_sampling_rates[i];
 
-		run = true;
+			if (!test_audio_configuration(alsa, format, channels,
+						      sampling_rate))
+				continue;
 
-		success &= do_test_display_audio(data, port, alsa, format,
-						 channels, sampling_rate);
+			run = true;
 
-		alsa_close_output(alsa);
+			success &= do_test_display_audio(data, port, alsa,

Quoted 117 lines · Simon

This adds a new test_formats array, which is a list of PCM formats we want to run the tests with. We try to run tests for all combinations of sampling rates and formats.

The ALSA callback now needs to fill the playback buffer differently depending on the format.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 26 deletions(-)

tests/kms_chamelium.c +40 -20

index af9f54d1f4c7..88356dd232b6 100644
@@ -795,8 +795,17 @@ static int test_frequencies[] = {
 
 static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
 
+static const snd_pcm_format_t test_formats[] = {
+	SND_PCM_FORMAT_S16_LE,
+	SND_PCM_FORMAT_S24_LE,
+	SND_PCM_FORMAT_S32_LE,
+};
+
+static const size_t test_formats_count = sizeof(test_formats) / sizeof(test_formats[0]);
+
 struct audio_state {
 	struct audio_signal *signal;
+	snd_pcm_format_t format;
 	atomic_bool run;
 };
 

@@ -805,7 +814,19 @@ audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, samples);
+	switch (state->format) {
+	case SND_PCM_FORMAT_S16_LE:
+		audio_signal_fill_s16_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S24_LE:
+		audio_signal_fill_s24_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S32_LE:
+		audio_signal_fill_s32_le(state->signal, buffer, samples);
+		break;
+	default:
+		assert(false); /* unreachable */
+	}
 
 	return state->run ? 0 : -1;
 }

@@ -881,6 +902,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	audio_signal_synthesize(signal);
 
 	state.signal = signal;
+	state.format = playback_format;
 	state.run = true;
 	alsa_register_output_callback(alsa, audio_output_callback, &state,
 				      PLAYBACK_SAMPLES);

@@ -1027,6 +1049,22 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	return success;
 }
 
+static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
+				     int channels, int sampling_rate)
+{
+	if (!alsa_test_output_configuration(alsa, format, channels,
+					    sampling_rate)) {
+		igt_debug("Skipping test with format %s, sampling rate %d Hz "
+			  "and %d channels because at least one of the "
+			  "selected output devices doesn't support this "
+			  "configuration\n",
+			  snd_pcm_format_name(format),
+			  sampling_rate, channels);
+		return false;
+	}
+	return true;
+}
+
 static void
 test_display_audio(data_t *data, struct chamelium_port *port,
 		   const char *audio_device, enum test_edid edid)

@@ -1039,7 +1077,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	struct igt_fb fb;
 	drmModeModeInfo *mode;
 	drmModeConnector *connector;
-	int fb_id, i;
+	int fb_id, i, j;
 	int channels, sampling_rate;
 	snd_pcm_format_t format;
 

@@ -1076,35 +1114,31 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	run = false;
 	success = true;
 	for (i = 0; i < test_sampling_rates_count; i++) {
-		ret = alsa_open_output(alsa, audio_device);
-		igt_assert(ret >= 0);
-
-		/* TODO: playback with different formats */
-		/* TODO: playback on all 8 available channels */
-		format = SND_PCM_FORMAT_S16_LE;
-		channels = PLAYBACK_CHANNELS;
-		sampling_rate = test_sampling_rates[i];
-
-		if (!alsa_test_output_configuration(alsa, format, channels,
-						    sampling_rate)) {
-			igt_debug("Skipping test with format %s, sample rate "
-				  "%d Hz and %d channels because at least one "
-				  "of the selected output devices doesn't "
-				  "support this configuration\n",
-				  snd_pcm_format_name(format),
-				  channels, sampling_rate);
-			continue;

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1121

Quoted 6 lines

tests/kms_chamelium.c +5 -1

-		}
+		for (j = 0; j < test_formats_count; j++) {
+			ret = alsa_open_output(alsa, audio_device);
+			igt_assert(ret >= 0);
+
+			/* TODO: playback with different formats */

You forgot to remove the TODO.

Quoted 16 lines

tests/kms_chamelium.c +9 -4

+			/* TODO: playback on all 8 available channels */
+			format = test_formats[j];
+			channels = PLAYBACK_CHANNELS;
+			sampling_rate = test_sampling_rates[i];
 
-		run = true;
+			if (!test_audio_configuration(alsa, format, channels,
+						      sampling_rate))
+				continue;
 
-		success &= do_test_display_audio(data, port, alsa, format,
-						 channels, sampling_rate);
+			run = true;
 
-		alsa_close_output(alsa);
+			success &= do_test_display_audio(data, port, alsa,

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1142

Quoted 10 lines

tests/kms_chamelium.c +6 -1

+							 format, channels,
+							 sampling_rate);
+
+			alsa_close_output(alsa);
+		}
 	}
 
-	/* Make sure we tested at least one frequency. */
+	/* Make sure we tested at least one frequency and format. */
 	igt_assert(run);

Do we really want to fail if alsa did not export any format?

I mean, maybe we should just skip in this condition and have another test with EDID that would test that the audio driver parses the EDIDs correctly and exposes the right formats.

Regardless, this patch looks good with the above TODO removed:

Reviewed-by Martin Peres <[email protected]>

Quoted 3 lines

tests/kms_chamelium.c +0 -0

 	/* Make sure all runs were successful. */
 	igt_assert(success);

Quoted 118 lines · Martin

Quoted 117 lines · Simon

This adds a new test_formats array, which is a list of PCM formats we want to run the tests with. We try to run tests for all combinations of sampling rates and formats.

The ALSA callback now needs to fill the playback buffer differently depending on the format.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
 1 file changed, 60 insertions(+), 26 deletions(-)

tests/kms_chamelium.c +40 -20

index af9f54d1f4c7..88356dd232b6 100644
@@ -795,8 +795,17 @@ static int test_frequencies[] = {
 
 static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
 
+static const snd_pcm_format_t test_formats[] = {
+	SND_PCM_FORMAT_S16_LE,
+	SND_PCM_FORMAT_S24_LE,
+	SND_PCM_FORMAT_S32_LE,
+};
+
+static const size_t test_formats_count = sizeof(test_formats) / sizeof(test_formats[0]);
+
 struct audio_state {
 	struct audio_signal *signal;
+	snd_pcm_format_t format;
 	atomic_bool run;
 };
 

@@ -805,7 +814,19 @@ audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, samples);
+	switch (state->format) {
+	case SND_PCM_FORMAT_S16_LE:
+		audio_signal_fill_s16_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S24_LE:
+		audio_signal_fill_s24_le(state->signal, buffer, samples);
+		break;
+	case SND_PCM_FORMAT_S32_LE:
+		audio_signal_fill_s32_le(state->signal, buffer, samples);
+		break;
+	default:
+		assert(false); /* unreachable */
+	}
 
 	return state->run ? 0 : -1;
 }

@@ -881,6 +902,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	audio_signal_synthesize(signal);
 
 	state.signal = signal;
+	state.format = playback_format;
 	state.run = true;
 	alsa_register_output_callback(alsa, audio_output_callback, &state,
 				      PLAYBACK_SAMPLES);

@@ -1027,6 +1049,22 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	return success;
 }
 
+static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
+				     int channels, int sampling_rate)
+{
+	if (!alsa_test_output_configuration(alsa, format, channels,
+					    sampling_rate)) {
+		igt_debug("Skipping test with format %s, sampling rate %d Hz "
+			  "and %d channels because at least one of the "
+			  "selected output devices doesn't support this "
+			  "configuration\n",
+			  snd_pcm_format_name(format),
+			  sampling_rate, channels);
+		return false;
+	}
+	return true;
+}
+
 static void
 test_display_audio(data_t *data, struct chamelium_port *port,
 		   const char *audio_device, enum test_edid edid)

@@ -1039,7 +1077,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	struct igt_fb fb;
 	drmModeModeInfo *mode;
 	drmModeConnector *connector;
-	int fb_id, i;
+	int fb_id, i, j;
 	int channels, sampling_rate;
 	snd_pcm_format_t format;
 

@@ -1076,35 +1114,31 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	run = false;
 	success = true;
 	for (i = 0; i < test_sampling_rates_count; i++) {
-		ret = alsa_open_output(alsa, audio_device);
-		igt_assert(ret >= 0);
-
-		/* TODO: playback with different formats */
-		/* TODO: playback on all 8 available channels */
-		format = SND_PCM_FORMAT_S16_LE;
-		channels = PLAYBACK_CHANNELS;
-		sampling_rate = test_sampling_rates[i];
-
-		if (!alsa_test_output_configuration(alsa, format, channels,
-						    sampling_rate)) {
-			igt_debug("Skipping test with format %s, sample rate "
-				  "%d Hz and %d channels because at least one "
-				  "of the selected output devices doesn't "
-				  "support this configuration\n",
-				  snd_pcm_format_name(format),
-				  channels, sampling_rate);
-			continue;

Quoted 8 lines

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1121

Quoted 6 lines

tests/kms_chamelium.c +5 -1

-		}
+		for (j = 0; j < test_formats_count; j++) {
+			ret = alsa_open_output(alsa, audio_device);
+			igt_assert(ret >= 0);
+
+			/* TODO: playback with different formats */

You forgot to remove the TODO.

Derp

Quoted 29 lines

Quoted 16 lines

tests/kms_chamelium.c +9 -4

+			/* TODO: playback on all 8 available channels */
+			format = test_formats[j];
+			channels = PLAYBACK_CHANNELS;
+			sampling_rate = test_sampling_rates[i];
 
-		run = true;
+			if (!test_audio_configuration(alsa, format, channels,
+						      sampling_rate))
+				continue;
 
-		success &= do_test_display_audio(data, port, alsa, format,
-						 channels, sampling_rate);
+			run = true;
 
-		alsa_close_output(alsa);
+			success &= do_test_display_audio(data, port, alsa,

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1142

Quoted 10 lines

tests/kms_chamelium.c +6 -1

+							 format, channels,
+							 sampling_rate);
+
+			alsa_close_output(alsa);
+		}
 	}
 
-	/* Make sure we tested at least one frequency. */
+	/* Make sure we tested at least one frequency and format. */
 	igt_assert(run);

Do we really want to fail if alsa did not export any format?

Quoted 3 lines

I mean, maybe we should just skip in this condition and have another test with EDID that would test that the audio driver parses the EDIDs correctly and exposes the right formats.

Hmm. I'd rather keep it until we have this test.

However checking whether ALSA exposes the correct formats isn't going to work: the Chamelium device only exposes 16 and 24-bit formats but ALSA reports a 32-bit format…

However /proc/asound parses and reports correct values. Maybe we could have a test that uses this instead.

Quoted 10 lines

Regardless, this patch looks good with the above TODO removed:

Reviewed-by Martin Peres <[email protected]>

Quoted 3 lines

tests/kms_chamelium.c +0 -0

 	/* Make sure all runs were successful. */
 	igt_assert(success);

In these configurations the Chamelium device sends malformed data:

- More than 2 channels
- 2 channels, more than 16-bit samples and more than 32KHz sampling rate

In this cases, skip the test.

See crbug.com/950917.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tests/kms_chamelium.c +4 -0

index 88356dd232b6..e888a642c24a 100644
@@ -1062,6 +1062,17 @@ static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
 			  sampling_rate, channels);
 		return false;
 	}
+	/* TODO: the Chamelium device sends a malformed signal for some audio
+	 * configurations. See crbug.com/950917 */
+	if ((format != SND_PCM_FORMAT_S16_LE && sampling_rate >= 44100) ||
+			channels > 2) {

Quoted 5 lines · Simon

In these configurations the Chamelium device sends malformed data:

- More than 2 channels
- 2 channels, more than 16-bit samples and more than 32KHz sampling rate

Quoted 3 lines

In this cases, skip the test.

See crbug.com/950917.

Reviewed-by Martin Peres <[email protected]>

Thanks for the series, and the nice split between patches :)

Quoted 20 lines

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

tests/kms_chamelium.c +7 -0

index 88356dd232b6..e888a642c24a 100644
@@ -1062,6 +1062,17 @@ static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
 			  sampling_rate, channels);
 		return false;
 	}
+	/* TODO: the Chamelium device sends a malformed signal for some audio
+	 * configurations. See crbug.com/950917 */
+	if ((format != SND_PCM_FORMAT_S16_LE && sampling_rate >= 44100) ||
+			channels > 2) {
+		igt_debug("Skipping test with format %s, sampling rate %d Hz "
+			  "and %d channels because the Chamelium device "
+			  "doesn't support this configuration\n",

tests/kms_chamelium.c static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format, · line 1078

Quoted 8 lines

tests/kms_chamelium.c +4 -0

+			  snd_pcm_format_name(format),
+			  sampling_rate, channels);
+		return false;
+	}
 	return true;
 }
 

This commit adds a new format argument to alsa_configure_output. This allows the caller to decide the format used for the audio signal.

This removes the assumption that samples are S16_LE items in igt_alsa. The alsa function snd_pcm_format_physical_width is used instead of sizeof(short). This also removes the theorically incorrect assumption that sizeof(short) == sizeof(int16_t).

Right now S16_LE is hardcoded in the test. Tests with other formats will be added in future commits.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_alsa.c        | 23 +++++++++++++----------
 lib/igt_alsa.h        |  7 ++++---
 tests/kms_chamelium.c |  7 ++++---
 3 files changed, 21 insertions(+), 16 deletions(-)

lib/igt_alsa.c +7 -6

index 28a866e0ee74..2dfb8ccd3ad9 100644
@@ -27,7 +27,6 @@
 #include "config.h"
 
 #include <limits.h>
-#include <alsa/asoundlib.h>
 
 #include "igt_alsa.h"
 #include "igt_aux.h"

@@ -47,10 +46,11 @@
 struct alsa {
 	snd_pcm_t *output_handles[HANDLES_MAX];
 	int output_handles_count;
+	snd_pcm_format_t output_format;
 	int output_sampling_rate;
 	int output_channels;
 
-	int (*output_callback)(void *data, short *buffer, int samples);
+	int (*output_callback)(void *data, void *buffer, int samples);
 	void *output_callback_data;
 	int output_samples_trigger;
 };

@@ -342,8 +342,8 @@ bool alsa_test_output_configuration(struct alsa *alsa, int channels,
  * Configure the output devices with the configuration specified by @channels
  * and @sampling_rate.
  */
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate)
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate)
 {
 	snd_pcm_t *handle;
 	int ret;

@@ -354,13 +354,14 @@ void alsa_configure_output(struct alsa *alsa, int channels,
 	for (i = 0; i < alsa->output_handles_count; i++) {
 		handle = alsa->output_handles[i];
 
-		ret = snd_pcm_set_params(handle, SND_PCM_FORMAT_S16_LE,
+		ret = snd_pcm_set_params(handle, fmt,
 					 SND_PCM_ACCESS_RW_INTERLEAVED,
 					 channels, sampling_rate,
 					 soft_resample, latency);
 		igt_assert(ret >= 0);
 	}
 
+	alsa->output_format = fmt;
 	alsa->output_channels = channels;
 	alsa->output_sampling_rate = sampling_rate;
 }

@@ -379,7 +380,7 @@ void alsa_configure_output(struct alsa *alsa, int channels,
  * for failure.
  */
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger)
 {
 	alsa->output_callback = callback;

lib/igt_alsa.c +5 -3

 	int output_limit;
 	int output_total = 0;
 	int output_counts[alsa->output_handles_count];
 	bool output_ready = false;
 	int output_channels;
+	int bytes_per_sample;
 	int output_trigger;
 	bool reached;
 	int index;

@@ -418,9 +420,10 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 
 	output_limit = alsa->output_sampling_rate * duration_ms / 1000;
 	output_channels = alsa->output_channels;
+	bytes_per_sample = snd_pcm_format_physical_width(alsa->output_format) / 8;
 	output_trigger = alsa->output_samples_trigger;
-	output_buffer = malloc(sizeof(short) * output_channels *
-			       output_trigger);
+	output_buffer = malloc(output_channels * output_trigger *
+			       bytes_per_sample);
 
 	do {
 		reached = true;

@@ -454,7 +457,7 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 					count = avail < count ? avail : count;
 
 					ret = snd_pcm_writei(handle,
-							     &output_buffer[index],
+							     &output_buffer[index * bytes_per_sample],
 							     count);
 					if (ret < 0) {
 						ret = snd_pcm_recover(handle,

lib/igt_alsa.h +4 -3

index a10985ff777f..46b3568d26fd 100644
@@ -29,6 +29,7 @@
 
 #include "config.h"
 
+#include <alsa/asoundlib.h>
 #include <stdbool.h>
 
 struct alsa;

@@ -39,10 +40,10 @@ int alsa_open_output(struct alsa *alsa, const char *device_name);
 void alsa_close_output(struct alsa *alsa);
 bool alsa_test_output_configuration(struct alsa *alsa, int channels,
 				    int sampling_rate);
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate);
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate);
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger);
 int alsa_run(struct alsa *alsa, int duration_ms);
 

tests/kms_chamelium.c +2 -2

index c8b6b22d7b4a..2b465565418d 100644
@@ -801,11 +801,11 @@ struct audio_state {
 };
 
 static int
-audio_output_callback(void *data, short *buffer, int frames)
+audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, frames);
+	audio_signal_fill_s16_le(state->signal, buffer, samples);
 
 	return state->run ? 0 : -1;
 }

Quoted 79 lines · Simon Ser

This commit adds a new format argument to alsa_configure_output. This allows the caller to decide the format used for the audio signal.

This removes the assumption that samples are S16_LE items in igt_alsa. The alsa function snd_pcm_format_physical_width is used instead of sizeof(short). This also removes the theorically incorrect assumption that sizeof(short) == sizeof(int16_t).

Right now S16_LE is hardcoded in the test. Tests with other formats will be added in future commits.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_alsa.c        | 23 +++++++++++++----------
 lib/igt_alsa.h        |  7 ++++---
 tests/kms_chamelium.c |  7 ++++---
 3 files changed, 21 insertions(+), 16 deletions(-)

lib/igt_alsa.c +7 -6

index 28a866e0ee74..2dfb8ccd3ad9 100644
@@ -27,7 +27,6 @@
 #include "config.h"
 
 #include <limits.h>
-#include <alsa/asoundlib.h>
 
 #include "igt_alsa.h"
 #include "igt_aux.h"

@@ -47,10 +46,11 @@
 struct alsa {
 	snd_pcm_t *output_handles[HANDLES_MAX];
 	int output_handles_count;
+	snd_pcm_format_t output_format;
 	int output_sampling_rate;
 	int output_channels;
 
-	int (*output_callback)(void *data, short *buffer, int samples);
+	int (*output_callback)(void *data, void *buffer, int samples);
 	void *output_callback_data;
 	int output_samples_trigger;
 };

@@ -342,8 +342,8 @@ bool alsa_test_output_configuration(struct alsa *alsa, int channels,
  * Configure the output devices with the configuration specified by @channels
  * and @sampling_rate.
  */
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate)
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate)
 {
 	snd_pcm_t *handle;
 	int ret;

@@ -354,13 +354,14 @@ void alsa_configure_output(struct alsa *alsa, int channels,
 	for (i = 0; i < alsa->output_handles_count; i++) {
 		handle = alsa->output_handles[i];
 
-		ret = snd_pcm_set_params(handle, SND_PCM_FORMAT_S16_LE,
+		ret = snd_pcm_set_params(handle, fmt,
 					 SND_PCM_ACCESS_RW_INTERLEAVED,
 					 channels, sampling_rate,
 					 soft_resample, latency);
 		igt_assert(ret >= 0);
 	}
 
+	alsa->output_format = fmt;
 	alsa->output_channels = channels;
 	alsa->output_sampling_rate = sampling_rate;
 }

@@ -379,7 +380,7 @@ void alsa_configure_output(struct alsa *alsa, int channels,
  * for failure.
  */
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger)
 {
 	alsa->output_callback = callback;

lib/igt_alsa.c void alsa_register_output_callback(struct alsa *alsa, · line 406

Quoted 6 lines

lib/igt_alsa.c +1 -1

@@ -402,12 +403,13 @@ void alsa_register_output_callback(struct alsa *alsa,
 int alsa_run(struct alsa *alsa, int duration_ms)
 {
 	snd_pcm_t *handle;
-	short *output_buffer = NULL;
+	char *output_buffer = NULL;

Why not void*?

Quoted 83 lines

lib/igt_alsa.c +5 -3

 	int output_limit;
 	int output_total = 0;
 	int output_counts[alsa->output_handles_count];
 	bool output_ready = false;
 	int output_channels;
+	int bytes_per_sample;
 	int output_trigger;
 	bool reached;
 	int index;

@@ -418,9 +420,10 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 
 	output_limit = alsa->output_sampling_rate * duration_ms / 1000;
 	output_channels = alsa->output_channels;
+	bytes_per_sample = snd_pcm_format_physical_width(alsa->output_format) / 8;
 	output_trigger = alsa->output_samples_trigger;
-	output_buffer = malloc(sizeof(short) * output_channels *
-			       output_trigger);
+	output_buffer = malloc(output_channels * output_trigger *
+			       bytes_per_sample);
 
 	do {
 		reached = true;

@@ -454,7 +457,7 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 					count = avail < count ? avail : count;
 
 					ret = snd_pcm_writei(handle,
-							     &output_buffer[index],
+							     &output_buffer[index * bytes_per_sample],
 							     count);
 					if (ret < 0) {
 						ret = snd_pcm_recover(handle,

lib/igt_alsa.h +4 -3

index a10985ff777f..46b3568d26fd 100644
@@ -29,6 +29,7 @@
 
 #include "config.h"
 
+#include <alsa/asoundlib.h>
 #include <stdbool.h>
 
 struct alsa;

@@ -39,10 +40,10 @@ int alsa_open_output(struct alsa *alsa, const char *device_name);
 void alsa_close_output(struct alsa *alsa);
 bool alsa_test_output_configuration(struct alsa *alsa, int channels,
 				    int sampling_rate);
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate);
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate);
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger);
 int alsa_run(struct alsa *alsa, int duration_ms);
 

tests/kms_chamelium.c +4 -3

index c8b6b22d7b4a..2b465565418d 100644
@@ -801,11 +801,11 @@ struct audio_state {
 };
 
 static int
-audio_output_callback(void *data, short *buffer, int frames)
+audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, frames);
+	audio_signal_fill_s16_le(state->signal, buffer, samples);
 
 	return state->run ? 0 : -1;
 }

@@ -843,7 +843,8 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 
 	igt_debug("Testing with playback sampling rate %d Hz and %d channels\n",
 		  playback_rate, playback_channels);
-	alsa_configure_output(alsa, playback_channels, playback_rate);
+	alsa_configure_output(alsa, SND_PCM_FORMAT_S16_LE,
+			      playback_channels, playback_rate);
 

tests/kms_chamelium.c do_test_display_audio(data_t *data, struct chamelium_port *port, · line 850

Quoted 3 lines

tests/kms_chamelium.c +0 -0

 	chamelium_start_capturing_audio(data->chamelium, port, false);
 

Quoted 80 lines · Martin Peres

Quoted 79 lines · Simon Ser

This commit adds a new format argument to alsa_configure_output. This allows the caller to decide the format used for the audio signal.

This removes the assumption that samples are S16_LE items in igt_alsa. The alsa function snd_pcm_format_physical_width is used instead of sizeof(short). This also removes the theorically incorrect assumption that sizeof(short) == sizeof(int16_t).

Right now S16_LE is hardcoded in the test. Tests with other formats will be added in future commits.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_alsa.c        | 23 +++++++++++++----------
 lib/igt_alsa.h        |  7 ++++---
 tests/kms_chamelium.c |  7 ++++---
 3 files changed, 21 insertions(+), 16 deletions(-)

lib/igt_alsa.c +7 -6

index 28a866e0ee74..2dfb8ccd3ad9 100644
@@ -27,7 +27,6 @@
 #include "config.h"
 
 #include <limits.h>
-#include <alsa/asoundlib.h>
 
 #include "igt_alsa.h"
 #include "igt_aux.h"

@@ -47,10 +46,11 @@
 struct alsa {
 	snd_pcm_t *output_handles[HANDLES_MAX];
 	int output_handles_count;
+	snd_pcm_format_t output_format;
 	int output_sampling_rate;
 	int output_channels;
 
-	int (*output_callback)(void *data, short *buffer, int samples);
+	int (*output_callback)(void *data, void *buffer, int samples);
 	void *output_callback_data;
 	int output_samples_trigger;
 };

@@ -342,8 +342,8 @@ bool alsa_test_output_configuration(struct alsa *alsa, int channels,
  * Configure the output devices with the configuration specified by @channels
  * and @sampling_rate.
  */
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate)
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate)
 {
 	snd_pcm_t *handle;
 	int ret;

@@ -354,13 +354,14 @@ void alsa_configure_output(struct alsa *alsa, int channels,
 	for (i = 0; i < alsa->output_handles_count; i++) {
 		handle = alsa->output_handles[i];
 
-		ret = snd_pcm_set_params(handle, SND_PCM_FORMAT_S16_LE,
+		ret = snd_pcm_set_params(handle, fmt,
 					 SND_PCM_ACCESS_RW_INTERLEAVED,
 					 channels, sampling_rate,
 					 soft_resample, latency);
 		igt_assert(ret >= 0);
 	}
 
+	alsa->output_format = fmt;
 	alsa->output_channels = channels;
 	alsa->output_sampling_rate = sampling_rate;
 }

@@ -379,7 +380,7 @@ void alsa_configure_output(struct alsa *alsa, int channels,
  * for failure.
  */
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger)
 {
 	alsa->output_callback = callback;

Quoted 8 lines

lib/igt_alsa.c void alsa_register_output_callback(struct alsa *alsa, · line 406

Quoted 6 lines

lib/igt_alsa.c +1 -1

@@ -402,12 +403,13 @@ void alsa_register_output_callback(struct alsa *alsa,
 int alsa_run(struct alsa *alsa, int duration_ms)
 {
 	snd_pcm_t *handle;
-	short *output_buffer = NULL;
+	char *output_buffer = NULL;

Why not void*?

Below in the snd_pcm_writei call, we need to add an offset to this address. Since it's not possible to use the array syntax on void * I used char *. That way output_buffer[i] is the i-th byte of output_buffer.

(Note that doing pointer arithmetic on void * is undefined behaviour.)

Quoted 84 lines

Quoted 84 lines

lib/igt_alsa.c +5 -3

 	int output_limit;
 	int output_total = 0;
 	int output_counts[alsa->output_handles_count];
 	bool output_ready = false;
 	int output_channels;
+	int bytes_per_sample;
 	int output_trigger;
 	bool reached;
 	int index;

@@ -418,9 +420,10 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 
 	output_limit = alsa->output_sampling_rate * duration_ms / 1000;
 	output_channels = alsa->output_channels;
+	bytes_per_sample = snd_pcm_format_physical_width(alsa->output_format) / 8;
 	output_trigger = alsa->output_samples_trigger;
-	output_buffer = malloc(sizeof(short) * output_channels *
-			       output_trigger);
+	output_buffer = malloc(output_channels * output_trigger *
+			       bytes_per_sample);
 
 	do {
 		reached = true;

@@ -454,7 +457,7 @@ int alsa_run(struct alsa *alsa, int duration_ms)
 					count = avail < count ? avail : count;
 
 					ret = snd_pcm_writei(handle,
-							     &output_buffer[index],
+							     &output_buffer[index * bytes_per_sample],
 							     count);
 					if (ret < 0) {
 						ret = snd_pcm_recover(handle,

lib/igt_alsa.h +4 -3

index a10985ff777f..46b3568d26fd 100644
@@ -29,6 +29,7 @@
 
 #include "config.h"
 
+#include <alsa/asoundlib.h>
 #include <stdbool.h>
 
 struct alsa;

@@ -39,10 +40,10 @@ int alsa_open_output(struct alsa *alsa, const char *device_name);
 void alsa_close_output(struct alsa *alsa);
 bool alsa_test_output_configuration(struct alsa *alsa, int channels,
 				    int sampling_rate);
-void alsa_configure_output(struct alsa *alsa, int channels,
-			   int sampling_rate);
+void alsa_configure_output(struct alsa *alsa, snd_pcm_format_t fmt,
+			   int channels, int sampling_rate);
 void alsa_register_output_callback(struct alsa *alsa,
-				   int (*callback)(void *data, short *buffer, int samples),
+				   int (*callback)(void *data, void *buffer, int samples),
 				   void *callback_data, int samples_trigger);
 int alsa_run(struct alsa *alsa, int duration_ms);
 

tests/kms_chamelium.c +4 -3

index c8b6b22d7b4a..2b465565418d 100644
@@ -801,11 +801,11 @@ struct audio_state {
 };
 
 static int
-audio_output_callback(void *data, short *buffer, int frames)
+audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	audio_signal_fill_s16_le(state->signal, buffer, frames);
+	audio_signal_fill_s16_le(state->signal, buffer, samples);
 
 	return state->run ? 0 : -1;
 }

@@ -843,7 +843,8 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 
 	igt_debug("Testing with playback sampling rate %d Hz and %d channels\n",
 		  playback_rate, playback_channels);
-	alsa_configure_output(alsa, playback_channels, playback_rate);
+	alsa_configure_output(alsa, SND_PCM_FORMAT_S16_LE,
+			      playback_channels, playback_rate);
 

Quoted 3 lines

tests/kms_chamelium.c do_test_display_audio(data_t *data, struct chamelium_port *port, · line 850

Quoted 3 lines

tests/kms_chamelium.c +0 -0

 	chamelium_start_capturing_audio(data->chamelium, port, false);