* [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats
@ 2019-05-17 16:02 Simon Ser
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration Simon Ser
` (6 more replies)
0 siblings, 7 replies; 18+ messages in thread
From: Simon Ser @ 2019-05-17 16:02 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
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 <simon.ser@intel.com>
---
lib/igt_alsa.c | 23 +++++++++++++----------
lib/igt_alsa.h | 7 ++++---
tests/kms_chamelium.c | 7 ++++---
3 files changed, 21 insertions(+), 16 deletions(-)
diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
index 28a866e0ee74..2dfb8ccd3ad9 100644
--- a/lib/igt_alsa.c
+++ b/lib/igt_alsa.c
@@ -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;
@@ -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;
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,
diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
index a10985ff777f..46b3568d26fd 100644
--- a/lib/igt_alsa.h
+++ b/lib/igt_alsa.h
@@ -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);
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index c8b6b22d7b4a..2b465565418d 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -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);
chamelium_start_capturing_audio(data->chamelium, port, false);
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
@ 2019-05-17 16:02 ` Simon Ser
2019-05-20 12:08 ` Martin Peres
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation Simon Ser
` (5 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Simon Ser @ 2019-05-17 16:02 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
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 <simon.ser@intel.com>
---
lib/igt_alsa.c | 18 +++++++++++++-----
lib/igt_alsa.h | 4 ++--
tests/kms_chamelium.c | 31 ++++++++++++++++++++-----------
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
index 2dfb8ccd3ad9..5b5980a94888 100644
--- a/lib/igt_alsa.c
+++ b/lib/igt_alsa.c
@@ -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;
}
diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
index 46b3568d26fd..1ece9f5255d8 100644
--- a/lib/igt_alsa.h
+++ b/lib/igt_alsa.h
@@ -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,
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 2b465565418d..af9f54d1f4c7 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -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;
}
run = true;
- success &= do_test_display_audio(data, port, alsa, channels,
- sampling_rate);
+ success &= do_test_display_audio(data, port, alsa, format,
+ channels, sampling_rate);
alsa_close_output(alsa);
}
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration Simon Ser
@ 2019-05-17 16:02 ` Simon Ser
2019-05-20 12:14 ` Martin Peres
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests Simon Ser
` (4 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Simon Ser @ 2019-05-17 16:02 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
This adds two new helpers to generate S24_LE and S32_LE signals.
Signed-off-by: Simon Ser <simon.ser@intel.com>
---
lib/igt_audio.c | 30 ++++++++++++++++++++++++++++++
lib/igt_audio.h | 4 ++++
2 files changed, 34 insertions(+)
diff --git a/lib/igt_audio.c b/lib/igt_audio.c
index 90d16fe4bd11..876084a994c3 100644
--- a/lib/igt_audio.c
+++ b/lib/igt_audio.c
@@ -262,6 +262,36 @@ void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
free(tmp);
}
+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);
+
+ for (i = 0; i < signal->channels * samples; ++i)
+ buffer[i] = 0xFFFFFF * tmp[i];
+
+ free(tmp);
+}
+
+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);
+
+ 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.
diff --git a/lib/igt_audio.h b/lib/igt_audio.h
index f915d55d63fc..c8de70871faa 100644
--- a/lib/igt_audio.h
+++ b/lib/igt_audio.h
@@ -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);
+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,
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration Simon Ser
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation Simon Ser
@ 2019-05-17 16:02 ` Simon Ser
2019-05-20 12:20 ` Peres, Martin
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support Simon Ser
` (3 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Simon Ser @ 2019-05-17 16:02 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
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 <simon.ser@intel.com>
---
tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
1 file changed, 60 insertions(+), 26 deletions(-)
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index af9f54d1f4c7..88356dd232b6 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -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;
- }
+ for (j = 0; j < test_formats_count; j++) {
+ ret = alsa_open_output(alsa, audio_device);
+ igt_assert(ret >= 0);
+
+ /* TODO: playback with different formats */
+ /* 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,
+ 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);
/* Make sure all runs were successful. */
igt_assert(success);
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
` (2 preceding siblings ...)
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests Simon Ser
@ 2019-05-17 16:02 ` Simon Ser
2019-05-20 12:21 ` Peres, Martin
2019-05-17 16:44 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats Patchwork
` (2 subsequent siblings)
6 siblings, 1 reply; 18+ messages in thread
From: Simon Ser @ 2019-05-17 16:02 UTC (permalink / raw)
To: igt-dev; +Cc: martin.peres
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 <simon.ser@intel.com>
---
tests/kms_chamelium.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 88356dd232b6..e888a642c24a 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -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",
+ snd_pcm_format_name(format),
+ sampling_rate, channels);
+ return false;
+ }
return true;
}
--
2.21.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
` (3 preceding siblings ...)
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support Simon Ser
@ 2019-05-17 16:44 ` Patchwork
2019-05-18 4:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-05-20 10:11 ` [igt-dev] [PATCH i-g-t 1/5] " Martin Peres
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2019-05-17 16:44 UTC (permalink / raw)
To: Simon Ser; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats
URL : https://patchwork.freedesktop.org/series/60791/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6096 -> IGTPW_2999
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60791/revisions/1/mbox/
Known issues
------------
Here are the changes found in IGTPW_2999 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_suspend@basic-s3:
- fi-blb-e6850: [PASS][1] -> [INCOMPLETE][2] ([fdo#107718])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
#### Possible fixes ####
* igt@gem_basic@bad-close:
- fi-icl-u2: [INCOMPLETE][3] ([fdo#107713]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-icl-u2/igt@gem_basic@bad-close.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-icl-u2/igt@gem_basic@bad-close.html
* igt@gem_cpu_reloc@basic:
- {fi-icl-y}: [INCOMPLETE][5] ([fdo#107713] / [fdo#110246]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-icl-y/igt@gem_cpu_reloc@basic.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-icl-y/igt@gem_cpu_reloc@basic.html
- {fi-icl-u3}: [INCOMPLETE][7] ([fdo#107713] / [fdo#110246]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-icl-u3/igt@gem_cpu_reloc@basic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-icl-u3/igt@gem_cpu_reloc@basic.html
#### Warnings ####
* igt@i915_selftest@live_hangcheck:
- fi-apl-guc: [INCOMPLETE][9] ([fdo#103927] / [fdo#110624]) -> [DMESG-FAIL][10] ([fdo#110620])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
* igt@runner@aborted:
- fi-apl-guc: [FAIL][11] ([fdo#110624]) -> [FAIL][12] ([fdo#110622])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/fi-apl-guc/igt@runner@aborted.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/fi-apl-guc/igt@runner@aborted.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
[fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246
[fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620
[fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622
[fdo#110624]: https://bugs.freedesktop.org/show_bug.cgi?id=110624
Participating hosts (51 -> 46)
------------------------------
Additional (2): fi-skl-lmem fi-cml-u
Missing (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* IGT: IGT_4996 -> IGTPW_2999
CI_DRM_6096: beb32d3348a566a6aafa292c65e2d60a610479c4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2999: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/
IGT_4996: 6fe5d254ec1b9b47d61408e1b49a7339876bf1e7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* [igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
` (4 preceding siblings ...)
2019-05-17 16:44 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats Patchwork
@ 2019-05-18 4:26 ` Patchwork
2019-05-20 10:11 ` [igt-dev] [PATCH i-g-t 1/5] " Martin Peres
6 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2019-05-18 4:26 UTC (permalink / raw)
To: Simon Ser; +Cc: igt-dev
== Series Details ==
Series: series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats
URL : https://patchwork.freedesktop.org/series/60791/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_6096_full -> IGTPW_2999_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://patchwork.freedesktop.org/api/1.0/series/60791/revisions/1/mbox/
Known issues
------------
Here are the changes found in IGTPW_2999_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_mmap_gtt@forked-big-copy:
- shard-iclb: [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#109100])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb3/igt@gem_mmap_gtt@forked-big-copy.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb1/igt@gem_mmap_gtt@forked-big-copy.html
* igt@gem_mmap_gtt@forked-medium-copy:
- shard-iclb: [PASS][3] -> [INCOMPLETE][4] ([fdo#107713])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb1/igt@gem_mmap_gtt@forked-medium-copy.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb8/igt@gem_mmap_gtt@forked-medium-copy.html
* igt@gem_tiled_swapping@non-threaded:
- shard-apl: [PASS][5] -> [DMESG-WARN][6] ([fdo#108686])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-apl7/igt@gem_tiled_swapping@non-threaded.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-apl6/igt@gem_tiled_swapping@non-threaded.html
* igt@gem_workarounds@suspend-resume:
- shard-apl: [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +5 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-apl1/igt@gem_workarounds@suspend-resume.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-apl7/igt@gem_workarounds@suspend-resume.html
* igt@i915_pm_rpm@i2c:
- shard-iclb: [PASS][9] -> [FAIL][10] ([fdo#104097])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb5/igt@i915_pm_rpm@i2c.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb7/igt@i915_pm_rpm@i2c.html
* igt@kms_flip@flip-vs-expired-vblank:
- shard-glk: [PASS][11] -> [FAIL][12] ([fdo#102887] / [fdo#105363])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-glk2/igt@kms_flip@flip-vs-expired-vblank.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-glk1/igt@kms_flip@flip-vs-expired-vblank.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
- shard-kbl: [PASS][13] -> [FAIL][14] ([fdo#103167])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
- shard-apl: [PASS][15] -> [FAIL][16] ([fdo#103167])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
* igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt:
- shard-iclb: [PASS][17] -> [FAIL][18] ([fdo#103167]) +1 similar issue
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-rgb565-draw-blt.html
* igt@kms_psr@psr2_cursor_plane_onoff:
- shard-iclb: [PASS][19] -> [SKIP][20] ([fdo#109441]) +1 similar issue
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb4/igt@kms_psr@psr2_cursor_plane_onoff.html
#### Possible fixes ####
* igt@gem_mmap_gtt@forked-medium-copy-odd:
- shard-iclb: [INCOMPLETE][21] ([fdo#107713]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb6/igt@gem_mmap_gtt@forked-medium-copy-odd.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb5/igt@gem_mmap_gtt@forked-medium-copy-odd.html
* igt@i915_suspend@fence-restore-untiled:
- shard-apl: [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +1 similar issue
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-apl3/igt@i915_suspend@fence-restore-untiled.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-apl8/igt@i915_suspend@fence-restore-untiled.html
* igt@kms_color@pipe-b-ctm-green-to-red:
- shard-kbl: [FAIL][25] ([fdo#107201]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-kbl2/igt@kms_color@pipe-b-ctm-green-to-red.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-kbl3/igt@kms_color@pipe-b-ctm-green-to-red.html
- shard-apl: [FAIL][27] ([fdo#107201]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-apl3/igt@kms_color@pipe-b-ctm-green-to-red.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-apl8/igt@kms_color@pipe-b-ctm-green-to-red.html
* igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
- shard-glk: [FAIL][29] ([fdo#105363]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-glk8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-glk2/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
* igt@kms_flip@2x-flip-vs-suspend:
- shard-hsw: [INCOMPLETE][31] ([fdo#103540]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-hsw5/igt@kms_flip@2x-flip-vs-suspend.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-hsw2/igt@kms_flip@2x-flip-vs-suspend.html
* igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt:
- shard-iclb: [FAIL][33] ([fdo#103167]) -> [PASS][34] +4 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-shrfb-msflip-blt.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [SKIP][35] ([fdo#109441]) -> [PASS][36] +2 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][37] ([fdo#99912]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-kbl1/igt@kms_setmode@basic.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-kbl3/igt@kms_setmode@basic.html
* igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
- shard-snb: [SKIP][39] ([fdo#109271]) -> [PASS][40] +1 similar issue
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-snb6/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-snb5/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
#### Warnings ####
* igt@gem_mmap_gtt@forked-big-copy-odd:
- shard-iclb: [INCOMPLETE][41] ([fdo#107713] / [fdo#109100]) -> [TIMEOUT][42] ([fdo#109673])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6096/shard-iclb2/igt@gem_mmap_gtt@forked-big-copy-odd.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/shard-iclb4/igt@gem_mmap_gtt@forked-big-copy-odd.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#104097]: https://bugs.freedesktop.org/show_bug.cgi?id=104097
[fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
[fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
[fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
[fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
[fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
[fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
[fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
Participating hosts (10 -> 6)
------------------------------
Missing (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005
Build changes
-------------
* IGT: IGT_4996 -> IGTPW_2999
* Piglit: piglit_4509 -> None
CI_DRM_6096: beb32d3348a566a6aafa292c65e2d60a610479c4 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_2999: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/
IGT_4996: 6fe5d254ec1b9b47d61408e1b49a7339876bf1e7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2999/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
` (5 preceding siblings ...)
2019-05-18 4:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-05-20 10:11 ` Martin Peres
2019-05-20 10:38 ` Ser, Simon
6 siblings, 1 reply; 18+ messages in thread
From: Martin Peres @ 2019-05-20 10:11 UTC (permalink / raw)
To: Simon Ser, igt-dev
On 17/05/2019 19:02, Simon Ser wrote:
> 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 <simon.ser@intel.com>
> ---
> lib/igt_alsa.c | 23 +++++++++++++----------
> lib/igt_alsa.h | 7 ++++---
> tests/kms_chamelium.c | 7 ++++---
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
> index 28a866e0ee74..2dfb8ccd3ad9 100644
> --- a/lib/igt_alsa.c
> +++ b/lib/igt_alsa.c
> @@ -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;
> @@ -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*?
> 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,
> diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
> index a10985ff777f..46b3568d26fd 100644
> --- a/lib/igt_alsa.h
> +++ b/lib/igt_alsa.h
> @@ -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);
>
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index c8b6b22d7b4a..2b465565418d 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -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);
>
> chamelium_start_capturing_audio(data->chamelium, port, false);
>
>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats
2019-05-20 10:11 ` [igt-dev] [PATCH i-g-t 1/5] " Martin Peres
@ 2019-05-20 10:38 ` Ser, Simon
2019-05-20 10:46 ` Martin Peres
0 siblings, 1 reply; 18+ messages in thread
From: Ser, Simon @ 2019-05-20 10:38 UTC (permalink / raw)
To: igt-dev, Peres, Martin
On Mon, 2019-05-20 at 13:11 +0300, Martin Peres wrote:
> On 17/05/2019 19:02, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> > lib/igt_alsa.c | 23 +++++++++++++----------
> > lib/igt_alsa.h | 7 ++++---
> > tests/kms_chamelium.c | 7 ++++---
> > 3 files changed, 21 insertions(+), 16 deletions(-)
> >
> > diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
> > index 28a866e0ee74..2dfb8ccd3ad9 100644
> > --- a/lib/igt_alsa.c
> > +++ b/lib/igt_alsa.c
> > @@ -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;
> > @@ -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.)
> > 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,
> > diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
> > index a10985ff777f..46b3568d26fd 100644
> > --- a/lib/igt_alsa.h
> > +++ b/lib/igt_alsa.h
> > @@ -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);
> >
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index c8b6b22d7b4a..2b465565418d 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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);
> >
> > chamelium_start_capturing_audio(data->chamelium, port, false);
> >
> >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats
2019-05-20 10:38 ` Ser, Simon
@ 2019-05-20 10:46 ` Martin Peres
0 siblings, 0 replies; 18+ messages in thread
From: Martin Peres @ 2019-05-20 10:46 UTC (permalink / raw)
To: Ser, Simon, igt-dev
On 20/05/2019 13:38, Ser, Simon wrote:
> On Mon, 2019-05-20 at 13:11 +0300, Martin Peres wrote:
>> On 17/05/2019 19:02, Simon Ser wrote:
>>> 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 <simon.ser@intel.com>
>>> ---
>>> lib/igt_alsa.c | 23 +++++++++++++----------
>>> lib/igt_alsa.h | 7 ++++---
>>> tests/kms_chamelium.c | 7 ++++---
>>> 3 files changed, 21 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
>>> index 28a866e0ee74..2dfb8ccd3ad9 100644
>>> --- a/lib/igt_alsa.c
>>> +++ b/lib/igt_alsa.c
>>> @@ -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;
>>> @@ -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.)
Fair-enough. Keep char* or use uint8_t*. Both work for me.
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
>>> 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,
>>> diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
>>> index a10985ff777f..46b3568d26fd 100644
>>> --- a/lib/igt_alsa.h
>>> +++ b/lib/igt_alsa.h
>>> @@ -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);
>>>
>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>> index c8b6b22d7b4a..2b465565418d 100644
>>> --- a/tests/kms_chamelium.c
>>> +++ b/tests/kms_chamelium.c
>>> @@ -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);
>>>
>>> chamelium_start_capturing_audio(data->chamelium, port, false);
>>>
>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration Simon Ser
@ 2019-05-20 12:08 ` Martin Peres
0 siblings, 0 replies; 18+ messages in thread
From: Martin Peres @ 2019-05-20 12:08 UTC (permalink / raw)
To: Simon Ser, igt-dev; +Cc: martin.peres
On 17/05/2019 19:02, Simon Ser wrote:
> This allows us to test whether HW supports a PCM format, and only run the test
> if it's the case.
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> lib/igt_alsa.c | 18 +++++++++++++-----
> lib/igt_alsa.h | 4 ++--
> tests/kms_chamelium.c | 31 ++++++++++++++++++++-----------
> 3 files changed, 35 insertions(+), 18 deletions(-)
>
> diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
> index 2dfb8ccd3ad9..5b5980a94888 100644
> --- a/lib/igt_alsa.c
> +++ b/lib/igt_alsa.c
> @@ -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;
> }
> diff --git a/lib/igt_alsa.h b/lib/igt_alsa.h
> index 46b3568d26fd..1ece9f5255d8 100644
> --- a/lib/igt_alsa.h
> +++ b/lib/igt_alsa.h
> @@ -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,
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 2b465565418d..af9f54d1f4c7 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -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;
> }
>
> run = true;
>
> - success &= do_test_display_audio(data, port, alsa, channels,
> - sampling_rate);
> + success &= do_test_display_audio(data, port, alsa, format,
> + channels, sampling_rate);
>
> alsa_close_output(alsa);
> }
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation Simon Ser
@ 2019-05-20 12:14 ` Martin Peres
2019-05-20 13:08 ` Ser, Simon
0 siblings, 1 reply; 18+ messages in thread
From: Martin Peres @ 2019-05-20 12:14 UTC (permalink / raw)
To: Simon Ser, igt-dev; +Cc: martin.peres
On 17/05/2019 19:02, Simon Ser wrote:
> 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 <martin.peres@linux.intel.com>
>
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> lib/igt_audio.c | 30 ++++++++++++++++++++++++++++++
> lib/igt_audio.h | 4 ++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/lib/igt_audio.c b/lib/igt_audio.c
> index 90d16fe4bd11..876084a994c3 100644
> --- a/lib/igt_audio.c
> +++ b/lib/igt_audio.c
> @@ -262,6 +262,36 @@ void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
> free(tmp);
> }
>
> +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] */
> + for (i = 0; i < signal->channels * samples; ++i)
> + buffer[i] = 0xFFFFFF * tmp[i];
> +
> + free(tmp);
> +}
> +
> +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] */
> + 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.
> diff --git a/lib/igt_audio.h b/lib/igt_audio.h
> index f915d55d63fc..c8de70871faa 100644
> --- a/lib/igt_audio.h
> +++ b/lib/igt_audio.h
> @@ -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);
> +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,
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests Simon Ser
@ 2019-05-20 12:20 ` Peres, Martin
2019-05-20 13:10 ` Ser, Simon
0 siblings, 1 reply; 18+ messages in thread
From: Peres, Martin @ 2019-05-20 12:20 UTC (permalink / raw)
To: Ser, Simon, igt-dev
On 17/05/2019 19:03, Ser, Simon wrote:
> 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 <simon.ser@intel.com>
> ---
> tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
> 1 file changed, 60 insertions(+), 26 deletions(-)
>
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index af9f54d1f4c7..88356dd232b6 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -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;
> - }
> + 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.
> + /* 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,
> + 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 <martin.peres@linux.intel.com>
> /* Make sure all runs were successful. */
> igt_assert(success);
>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support Simon Ser
@ 2019-05-20 12:21 ` Peres, Martin
2019-05-20 13:12 ` Ser, Simon
0 siblings, 1 reply; 18+ messages in thread
From: Peres, Martin @ 2019-05-20 12:21 UTC (permalink / raw)
To: Ser, Simon, igt-dev
On 17/05/2019 19:03, Ser, Simon wrote:
> 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.
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
Thanks for the series, and the nice split between patches :)
>
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> tests/kms_chamelium.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 88356dd232b6..e888a642c24a 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -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",
> + snd_pcm_format_name(format),
> + sampling_rate, channels);
> + return false;
> + }
> return true;
> }
>
>
---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki
Business Identity Code: 0357606 - 4
Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation
2019-05-20 12:14 ` Martin Peres
@ 2019-05-20 13:08 ` Ser, Simon
0 siblings, 0 replies; 18+ messages in thread
From: Ser, Simon @ 2019-05-20 13:08 UTC (permalink / raw)
To: igt-dev, martin.peres; +Cc: Peres, Martin
On Mon, 2019-05-20 at 15:14 +0300, Martin Peres wrote:
> On 17/05/2019 19:02, Simon Ser wrote:
> > 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].
That's fair. I added the note to audio_signal_fill instead of
duplicating it in call sites.
> I think this patch could add a small note (see below), but regardless of
> what you chose to do:
>
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> > lib/igt_audio.c | 30 ++++++++++++++++++++++++++++++
> > lib/igt_audio.h | 4 ++++
> > 2 files changed, 34 insertions(+)
> >
> > diff --git a/lib/igt_audio.c b/lib/igt_audio.c
> > index 90d16fe4bd11..876084a994c3 100644
> > --- a/lib/igt_audio.c
> > +++ b/lib/igt_audio.c
> > @@ -262,6 +262,36 @@ void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
> > free(tmp);
> > }
> >
> > +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] */
> > + for (i = 0; i < signal->channels * samples; ++i)
> > + buffer[i] = 0xFFFFFF * tmp[i];
> > +
> > + free(tmp);
> > +}
> > +
> > +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] */
> > + 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.
> > diff --git a/lib/igt_audio.h b/lib/igt_audio.h
> > index f915d55d63fc..c8de70871faa 100644
> > --- a/lib/igt_audio.h
> > +++ b/lib/igt_audio.h
> > @@ -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);
> > +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,
> >
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests
2019-05-20 12:20 ` Peres, Martin
@ 2019-05-20 13:10 ` Ser, Simon
2019-05-20 13:43 ` Martin Peres
0 siblings, 1 reply; 18+ messages in thread
From: Ser, Simon @ 2019-05-20 13:10 UTC (permalink / raw)
To: igt-dev, Peres, Martin
On Mon, 2019-05-20 at 13:20 +0100, Peres, Martin wrote:
> On 17/05/2019 19:03, Ser, Simon wrote:
> > 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 <simon.ser@intel.com>
> > ---
> > tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
> > 1 file changed, 60 insertions(+), 26 deletions(-)
> >
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index af9f54d1f4c7..88356dd232b6 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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;
> > - }
> > + 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
> > + /* 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,
> > + 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.
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.
> Regardless, this patch looks good with the above TODO removed:
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
>
> > /* Make sure all runs were successful. */
> > igt_assert(success);
> >
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support
2019-05-20 12:21 ` Peres, Martin
@ 2019-05-20 13:12 ` Ser, Simon
0 siblings, 0 replies; 18+ messages in thread
From: Ser, Simon @ 2019-05-20 13:12 UTC (permalink / raw)
To: igt-dev, Peres, Martin
On Mon, 2019-05-20 at 13:21 +0100, Peres, Martin wrote:
> On 17/05/2019 19:03, Ser, Simon wrote:
> > 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.
>
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
> Thanks for the series, and the nice split between patches :)
Eheh, thanks for the review!
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> > tests/kms_chamelium.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 88356dd232b6..e888a642c24a 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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",
> > + snd_pcm_format_name(format),
> > + sampling_rate, channels);
> > + return false;
> > + }
> > return true;
> > }
> >
> >
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests
2019-05-20 13:10 ` Ser, Simon
@ 2019-05-20 13:43 ` Martin Peres
0 siblings, 0 replies; 18+ messages in thread
From: Martin Peres @ 2019-05-20 13:43 UTC (permalink / raw)
To: Ser, Simon, igt-dev
On 20/05/2019 16:10, Ser, Simon wrote:
> On Mon, 2019-05-20 at 13:20 +0100, Peres, Martin wrote:
>> On 17/05/2019 19:03, Ser, Simon wrote:
>>> 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 <simon.ser@intel.com>
>>> ---
>>> tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 60 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>> index af9f54d1f4c7..88356dd232b6 100644
>>> --- a/tests/kms_chamelium.c
>>> +++ b/tests/kms_chamelium.c
>>> @@ -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;
>>> - }
>>> + 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
>
>>> + /* 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,
>>> + 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.
>
> Hmm. I'd rather keep it until we have this test.
Fair-enough :)
>
> 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.
Sounds great! So we'll want to either use the inject test for this, or
use a chamelium test for this... or both since chamelium is conceptually
better :)
Martin
>
>> Regardless, this patch looks good with the above TODO removed:
>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>
>>
>>> /* Make sure all runs were successful. */
>>> igt_assert(success);
>>>
>>
>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2019-05-20 13:43 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-17 16:02 [igt-dev] [PATCH i-g-t 1/5] lib/igt_alsa: support all alsa formats Simon Ser
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 2/5] lib/igt_alsa: add format argument to alsa_test_output_configuration Simon Ser
2019-05-20 12:08 ` Martin Peres
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 3/5] lib/igt_audio: add support for S24_LE and S32_LE signal generation Simon Ser
2019-05-20 12:14 ` Martin Peres
2019-05-20 13:08 ` Ser, Simon
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests Simon Ser
2019-05-20 12:20 ` Peres, Martin
2019-05-20 13:10 ` Ser, Simon
2019-05-20 13:43 ` Martin Peres
2019-05-17 16:02 ` [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: skip audio tests Chamelium doesn't support Simon Ser
2019-05-20 12:21 ` Peres, Martin
2019-05-20 13:12 ` Ser, Simon
2019-05-17 16:44 ` [igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/5] lib/igt_alsa: support all alsa formats Patchwork
2019-05-18 4:26 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-05-20 10:11 ` [igt-dev] [PATCH i-g-t 1/5] " Martin Peres
2019-05-20 10:38 ` Ser, Simon
2019-05-20 10:46 ` Martin Peres
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.