Re: [igt-dev] [PATCH i-g-t v6 2/7] tests/kms_chamelium: capture audio data in real-time

Martin Peres
2019-04-17T12:57:21Z

On 17/04/2019 15:43, Simon Ser wrote:
> Before this patch, the audio test first sends an audio signal for 2s, and then
> checks whether the captured signal matches.
>
> This patch makes it so we send and check the signal in parallel. Thus we can
> stop the test as soon as we receive the correct signal. This saves ~2s per
> audio test.

Given that run_audio_thread is the one calling the callback, we should
not have coherency issues.

This patch is:

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
> lib/igt_alsa.c | 15 +++++++-------
> tests/kms_chamelium.c | 47 +++++++++++++++++++++++++++++++++----------
> 2 files changed, 43 insertions(+), 19 deletions(-)
>
> diff --git a/lib/igt_alsa.c b/lib/igt_alsa.c
> index 22089881..fc6d336b 100644
> --- a/lib/igt_alsa.c
> +++ b/lib/igt_alsa.c
> @@ -497,7 +497,8 @@ void alsa_register_input_callback(struct alsa *alsa,
> /**
> * alsa_run:
> * @alsa: The target alsa structure
> - * @duration_ms: The maximum duration of the run in milliseconds
> + * @duration_ms: The maximum duration of the run in milliseconds, or -1 for an
> + * infinite duration.
> *
> * Run ALSA playback and capture on the input and output devices for at
> * most @duration_ms milliseconds, calling the registered callbacks when needed.
> @@ -545,7 +546,7 @@ int alsa_run(struct alsa *alsa, int duration_ms)
> do {
> reached = true;
>
> - if (output_total < output_limit) {
> + if (output_limit < 0 || output_total < output_limit) {
> reached = false;
>
> if (!output_ready) {
> @@ -607,7 +608,8 @@ int alsa_run(struct alsa *alsa, int duration_ms)
>
> }
>
> - if (alsa->input_callback && input_total < input_limit) {
> + if (alsa->input_callback &&
> + (input_limit < 0 || input_total < input_limit)) {
> reached = false;
>
> if (input_count == input_trigger) {
> @@ -660,11 +662,8 @@ int alsa_run(struct alsa *alsa, int duration_ms)
> ret = 0;
>
> complete:
> - if (output_buffer)
> - free(output_buffer);
> -
> - if (input_buffer)
> - free(input_buffer);
> + free(output_buffer);
> + free(input_buffer);
>
> return ret;
> }
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index a712250a..014a22b3 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -29,7 +29,9 @@
> #include "igt_vc4.h"
>
> #include <fcntl.h>
> +#include <pthread.h>
> #include <string.h>
> +#include <stdatomic.h>
>
> typedef struct {
> struct chamelium *chamelium;
> @@ -719,7 +721,7 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> /* Capture paremeters control the audio signal we receive */
> #define CAPTURE_SAMPLES 2048
>
> -#define AUDIO_DURATION 2000 /* ms */
> +#define AUDIO_TIMEOUT 2000 /* ms */
> /* A streak of 3 gives confidence that the signal is good. */
> #define MIN_STREAK 3
>
> @@ -746,14 +748,28 @@ static int test_frequencies[] = {
>
> static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
>
> +struct audio_state {
> + struct audio_signal *signal;
> + atomic_bool run;
> +};
> +
> static int
> -output_callback(void *data, short *buffer, int frames)
> +audio_output_callback(void *data, short *buffer, int frames)
> {
> - struct audio_signal *signal = (struct audio_signal *) data;
> + struct audio_state *state = data;
> +
> + audio_signal_fill(state->signal, buffer, frames);
>
> - audio_signal_fill(signal, buffer, frames);
> + return state->run ? 0 : -1;
> +}
> +
> +static void *
> +run_audio_thread(void *data)
> +{
> + struct alsa *alsa = data;
>
> - return 0;
> + alsa_run(alsa, -1);
> + return NULL;
> }
>
> static bool
> @@ -774,6 +790,8 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> char dump_suffix[64];
> char *dump_path = NULL;
> int dump_fd = -1;
> + pthread_t thread;
> + struct audio_state state = {};
>
> if (!alsa_test_output_configuration(alsa, playback_channels,
> playback_rate))
> @@ -811,15 +829,15 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> audio_signal_add_frequency(signal, test_frequencies[i]);
> audio_signal_synthesize(signal);
>
> - alsa_register_output_callback(alsa, output_callback, signal,
> + state.signal = signal;
> + state.run = true;
> + alsa_register_output_callback(alsa, audio_output_callback, &state,
> PLAYBACK_SAMPLES);
>
> - /* TODO: detect signal in real-time */
> - ret = alsa_run(alsa, AUDIO_DURATION);
> + /* Start playing audio */
> + ret = pthread_create(&thread, NULL, run_audio_thread, alsa);
> igt_assert(ret == 0);
>
> - alsa_close_output(alsa);
> -
> /* Needs to be a multiple of 128, because that's the number of samples
> * we get per channel each time we receive an audio page from the
> * Chamelium device. */
> @@ -836,7 +854,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> streak = 0;
> msec = 0;
> i = 0;
> - while (streak < MIN_STREAK && msec < AUDIO_DURATION) {
> + while (streak < MIN_STREAK && msec < AUDIO_TIMEOUT) {
> ok = chamelium_stream_receive_realtime_audio(stream,
> &page_count,
> &recv, &recv_len);
> @@ -871,6 +889,13 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
> i++;
> }
>
> + igt_debug("Stopping audio playback\n");
> + state.run = false;
> + ret = pthread_join(thread, NULL);
> + igt_assert(ret == 0);
> +
> + alsa_close_output(alsa);
> +
> if (dump_fd >= 0) {
> close(dump_fd);
> if (streak == MIN_STREAK) {