Re: [igt-dev] [PATCH i-g-t v6 5/7] tests/kms_chamelium: run audio test with multiple sampling rates

Martin Peres
2019-04-23T07:58:41Z

On 17/04/2019 15:43, Simon Ser wrote:
> The audio test is now run multiple times with a variety of playback sampling
> rates.
>
> We now query the capture audio format from the Chamelium XML-RPC API instead of
> hardcoding it.
>
> One limitation is that we need to start sendting an audio signal before being

Typo on sending.

> able to query the capture audio format. However we need the capture sample rate
> to decide which frequencies we generate. For now we use the playback rate and
> check that it's the same as the capture rate.
>
> Another limitation is that the DP receiver reports an unknown sampling rate
> during the 41.1KHz test. In this case we assume the capture rate is the same as
> the playback rate. We'll fail later anyway if this assumption is incorrect
> since we check the signal we receive.
>
> Chameleon bug: https://crbug.com/950913
>
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> lib/igt_chamelium.c | 80 +++++++++++++++++++++++++-------------
> lib/igt_chamelium.h | 3 ++
> lib/igt_chamelium_stream.c | 17 --------
> lib/igt_chamelium_stream.h | 2 -
> tests/kms_chamelium.c | 56 +++++++++++++++-----------
> 5 files changed, 90 insertions(+), 68 deletions(-)
>
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 6ac7b722..ffc68f35 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -964,31 +964,6 @@ void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
> xmlrpc_DECREF(res);
> }
>
> -/**
> - * chamelium_start_capturing_audio:
> - * @chamelium: the Chamelium instance
> - * @port: the port to capture audio from (it must support audio)
> - * @save_to_file: whether the captured audio data should be saved to a file on
> - * the Chamelium device
> - *
> - * Starts capturing audio from a Chamelium port. To stop the capture, use
> - * #chamelium_stop_capturing_audio. To retrieve the audio data, either use the
> - * stream server or enable @save_to_file (the latter is mainly useful for
> - * debugging purposes).
> - *
> - * It isn't possible to capture audio from multiple ports at the same time.
> - */
> -void chamelium_start_capturing_audio(struct chamelium *chamelium,
> - struct chamelium_port *port,
> - bool save_to_file)
> -{
> - xmlrpc_value *res;
> -
> - res = chamelium_rpc(chamelium, port, "StartCapturingAudio", "(ib)",
> - port->id, save_to_file);
> - xmlrpc_DECREF(res);
> -}
> -
> static void audio_format_from_xml(struct chamelium *chamelium,
> xmlrpc_value *res, int *rate, int *channels)
> {
> @@ -1008,8 +983,10 @@ static void audio_format_from_xml(struct chamelium *chamelium,
> igt_assert(strcmp(sample_format, "S32_LE") == 0);
> free(sample_format);
>
> - xmlrpc_read_int(&chamelium->env, res_rate, rate);
> - xmlrpc_read_int(&chamelium->env, res_channel, channels);
> + if (rate)
> + xmlrpc_read_int(&chamelium->env, res_rate, rate);
> + if (channels)
> + xmlrpc_read_int(&chamelium->env, res_channel, channels);
>
> xmlrpc_DECREF(res_channel);
> xmlrpc_DECREF(res_sample_format);
> @@ -1017,6 +994,55 @@ static void audio_format_from_xml(struct chamelium *chamelium,
> xmlrpc_DECREF(res_type);
> }
>
> +/**
> + * chamelium_get_audio_format:
> + * @chamelium: the Chamelium instance
> + * @port: the audio port
> + * @rate: if non-NULL, will be set to the sample rate in Hz
> + * @channels: if non-NULL, will be set to the number of channels
> + *
> + * Obtains the audio format of the captured data. Users should start sending an
> + * audio signal to the Chamelium device prior to calling this function.
> + *
> + * The captured data is guaranteed to be in the S32_LE format.
> + */
> +void chamelium_get_audio_format(struct chamelium *chamelium,
> + struct chamelium_port *port,
> + int *rate, int *channels)
> +{
> + xmlrpc_value *res;
> +
> + res = chamelium_rpc(chamelium, port, "GetAudioFormat", "(i)",
> + port->id);
> + audio_format_from_xml(chamelium, res, rate, channels);
> + xmlrpc_DECREF(res);
> +}
> +
> +/**
> + * chamelium_start_capturing_audio:
> + * @chamelium: the Chamelium instance
> + * @port: the port to capture audio from (it must support audio)
> + * @save_to_file: whether the captured audio data should be saved to a file on
> + * the Chamelium device
> + *
> + * Starts capturing audio from a Chamelium port. To stop the capture, use
> + * #chamelium_stop_capturing_audio. To retrieve the audio data, either use the
> + * stream server or enable @save_to_file (the latter is mainly useful for
> + * debugging purposes).
> + *
> + * It isn't possible to capture audio from multiple ports at the same time.
> + */
> +void chamelium_start_capturing_audio(struct chamelium *chamelium,
> + struct chamelium_port *port,
> + bool save_to_file)
> +{
> + xmlrpc_value *res;
> +
> + res = chamelium_rpc(chamelium, port, "StartCapturingAudio", "(ib)",
> + port->id, save_to_file);
> + xmlrpc_DECREF(res);
> +}
> +
> /**
> * chamelium_stop_capturing_audio:
> * @chamelium: the Chamelium instance
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index 728d16ea..f47b84cb 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -109,6 +109,9 @@ void chamelium_capture(struct chamelium *chamelium, struct chamelium_port *port,
> void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
> struct chamelium_port *port,
> int mapping[static 8]);
> +void chamelium_get_audio_format(struct chamelium *chamelium,
> + struct chamelium_port *port,
> + int *rate, int *channels);
> void chamelium_start_capturing_audio(struct chamelium *chamelium,
> struct chamelium_port *port, bool save_to_file);
> struct chamelium_audio_file *chamelium_stop_capturing_audio(struct chamelium *chamelium,
> diff --git a/lib/igt_chamelium_stream.c b/lib/igt_chamelium_stream.c
> index 68ddb217..a8cd19e5 100644
> --- a/lib/igt_chamelium_stream.c
> +++ b/lib/igt_chamelium_stream.c
> @@ -537,23 +537,6 @@ bool chamelium_stream_stop_realtime_audio(struct chamelium_stream *client)
> return true;
> }
>
> -/**
> - * chamelium_stream_audio_format:
> - *
> - * Gets the format used for audio pages.
> - *
> - * Data will always be captured in raw pages of S32_LE elements. This function
> - * exposes the sampling rate and the number of channels.
> - */
> -void chamelium_stream_audio_format(struct chamelium_stream *stream,
> - int *rate, int *channels)
> -{
> - /* TODO: the Chamelium streaming server doesn't expose those yet.
> - * Just hardcode the values for now. */
> - *rate = 48000;
> - *channels = 8;
> -}
> -
> /**
> * chamelium_stream_init:
> *
> diff --git a/lib/igt_chamelium_stream.h b/lib/igt_chamelium_stream.h
> index de4e9931..3e1c5d14 100644
> --- a/lib/igt_chamelium_stream.h
> +++ b/lib/igt_chamelium_stream.h
> @@ -42,8 +42,6 @@ struct chamelium_stream *chamelium_stream_init(void);
> void chamelium_stream_deinit(struct chamelium_stream *client);
> bool chamelium_stream_dump_realtime_audio(struct chamelium_stream *client,
> enum chamelium_stream_realtime_mode mode);
> -void chamelium_stream_audio_format(struct chamelium_stream *stream,
> - int *rate, int *channels);
> bool chamelium_stream_receive_realtime_audio(struct chamelium_stream *client,
> size_t *page_count,
> int32_t **buf, size_t *buf_len);
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 8d6cc092..f535ca52 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -725,15 +725,14 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> /* A streak of 3 gives confidence that the signal is good. */
> #define MIN_STREAK 3
>
> -/* TODO: Chamelium only supports 48KHz for now */
> static int sampling_rates[] = {
> -/* 32000, */
> -/* 44100, */
> + 32000,
> + 44100,
> 48000,
> -/* 88200, */
> -/* 96000, */
> -/* 176400, */
> -/* 192000, */
> + 88200,
> + 96000,
> + 176400,
> + 192000,
> };
>
> static int sampling_rates_count = sizeof(sampling_rates) / sizeof(int);
> @@ -815,19 +814,6 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> ok = chamelium_stream_dump_realtime_audio(stream, stream_mode);
> igt_assert(ok);
>
> - chamelium_stream_audio_format(stream, &capture_rate, &capture_channels);
> -
> - if (igt_frame_dump_is_enabled()) {
> - snprintf(dump_suffix, sizeof(dump_suffix), "capture-%dch-%d",
> - playback_channels, playback_rate);
> -
> - dump_fd = audio_create_wav_file_s32_le(dump_suffix,
> - capture_rate,
> - capture_channels,
> - &dump_path);
> - igt_assert(dump_fd >= 0);
> - }
> -
> signal = audio_signal_init(playback_channels, playback_rate);
> igt_assert(signal);
>
> @@ -835,8 +821,12 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> * independent from each other. To do so, we'll add a different offset
> * to the base frequencies for each channel. We need to choose a big
> * enough offset so that we're sure to detect mixed up channels.
> + *
> + * Note that we assume capture_rate == playback_rate. We'll assert this
> + * later on. We cannot retrieve the capture rate before starting
> + * playing audio, so we don't really have the choice.
> */
> - step = 2 * capture_rate / CAPTURE_SAMPLES;
> + step = 2 * playback_rate / CAPTURE_SAMPLES;
> for (i = 0; i < test_frequencies_count; i++) {
> for (j = 0; j < playback_channels; j++) {
> freq = test_frequencies[i] + j * step;
> @@ -854,6 +844,17 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> ret = pthread_create(&thread, NULL, run_audio_thread, alsa);
> igt_assert(ret == 0);
>
> + /* Only after we've started playing audio, we can retrieve the capture
> + * format used by the Chamelium device. */
> + chamelium_get_audio_format(data->chamelium, port,
> + &capture_rate, &capture_channels);
> + if (capture_rate == 0) {
> + igt_debug("Audio receiver doesn't indicate the capture "
> + "sampling rate, assuming it's %d Hz\n", playback_rate);
> + capture_rate = playback_rate;
> + } else
> + igt_assert(capture_rate == playback_rate);
> +
> chamelium_get_audio_channel_mapping(data->chamelium, port,
> channel_mapping);
> /* Make sure we can capture all channels we send. */
> @@ -868,6 +869,17 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> igt_assert(ok);
> }
>
> + if (igt_frame_dump_is_enabled()) {
> + snprintf(dump_suffix, sizeof(dump_suffix), "capture-%dch-%d",
> + playback_channels, playback_rate);
> +
> + dump_fd = audio_create_wav_file_s32_le(dump_suffix,
> + capture_rate,
> + capture_channels,
> + &dump_path);
> + igt_assert(dump_fd >= 0);
> + }
> +

This belongs to another commit.

> /* Needs to be a multiple of 128, because that's the number of samples
> * we get per channel each time we receive an audio page from the
> * Chamelium device. */
> @@ -939,7 +951,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
>
> if (dump_fd >= 0) {
> close(dump_fd);
> - if (streak == MIN_STREAK) {
> + if (success) {
> /* Test succeeded, no need to keep the captured data */
> unlink(dump_path);
> } else
>

Dito.

Otherwise, the code looks OK:

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>