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

[Martin Peres <martin.peres@linux.intel.com>]

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) {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev