Re: [igt-dev] [PATCH i-g-t v2 9/9] tests/kms_chamelium: add audio channel alignment test

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

On 24/05/2019 18:03, Simon Ser wrote:
> The previous pulse test only checked the signal amplitude.

Don't forget to change the name here too.

> 
> This commit adds a new check to the pulse test: channel alignment. This check
> makes sure there is no time shift between each channel.
> 
> This is achieved by first sending a positive signal and then a falling edge.
> Edges in each channel should be aligned.
> 
> The index of each channel's falling edge is stored in number of samples. I
> chose not to implement a per-page test because the edge could be right between
> two pages.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++------
>  1 file changed, 87 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 073feff0d32d..44d04a4d4aad 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -773,7 +773,8 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>  #define MIN_STREAK 3
>  
>  #define PULSE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
> -#define PULSE_ACCURACY 0.001 /* ± 0.1 % of the full amplitude */
> +#define PULSE_AMPLITUDE_ACCURACY 0.001 /* ± 0.1 % of the full amplitude */
> +#define PULSE_ALIGN_ACCURACY 2 /* number of samples */

This value is oddly specific. The chamelium cannot introduce any phase
issue between the channels, so the only place where this could be
introduced is the sound card (and its driver).

Until proven that some soundcard are failing at this, let's just limit
the accuracy to 0 samples.

>  
>  /* TODO: enable >48KHz rates, these are not reliable */
>  static int test_sampling_rates[] = {
> @@ -828,7 +829,7 @@ struct audio_state {
>  	} playback, capture;
>  
>  	const char *name;
> -	struct audio_signal *signal;
> +	struct audio_signal *signal; /* for frequencies test only */
>  	int channel_mapping[8];
>  
>  	size_t recv_pages;
> @@ -839,6 +840,7 @@ struct audio_state {
>  
>  	pthread_t thread;
>  	atomic_bool run;
> +	atomic_bool positive; /* for pulse test only */
>  };
>  
>  static void audio_state_init(struct audio_state *state, data_t *data,
> @@ -1148,16 +1150,16 @@ static int audio_output_pulse_callback(void *data, void *buffer, int samples)
>  	len = samples * state->playback.channels;
>  	tmp = malloc(len * sizeof(double));
>  	for (i = 0; i < len; i++)
> -		tmp[i] = PULSE_AMPLITUDE;
> +		tmp[i] = state->positive ? PULSE_AMPLITUDE : -PULSE_AMPLITUDE;
>  	audio_convert_to(buffer, tmp, len, state->playback.format);
>  	free(tmp);
>  
>  	return state->run ? 0 : -1;
>  }
>  
> -static bool detect_pulse_amplitude(double *buf, size_t buf_len)
> +static bool detect_pulse_amplitude(double *buf, size_t buf_len, bool pos)
>  {
> -	double min, max;
> +	double expected, min, max;
>  	size_t i;
>  	bool ok;
>  
> @@ -1169,34 +1171,64 @@ static bool detect_pulse_amplitude(double *buf, size_t buf_len)
>  			max = buf[i];
>  	}
>  
> -	ok = (min >= PULSE_AMPLITUDE - PULSE_ACCURACY &&
> -	      max <= PULSE_AMPLITUDE + PULSE_ACCURACY);
> +	expected = pos ? PULSE_AMPLITUDE : -PULSE_AMPLITUDE;
> +	ok = (min >= expected - PULSE_AMPLITUDE_ACCURACY &&
> +	      max <= expected + PULSE_AMPLITUDE_ACCURACY);
>  	if (ok)
> -		igt_debug("Pulse detected\n");
> +		igt_debug("Pulse amplitude detected\n");
>  	else
> -		igt_debug("Pulse not detected (min=%f, max=%f)\n",
> +		igt_debug("Pulse amplitude not detected (min=%f, max=%f)\n",
>  			  min, max);
>  	return ok;
>  }
>  
> +static ssize_t detect_falling_edge(double *buf, size_t buf_len)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < buf_len; i++) {
> +		if (buf[i] < 0)
> +			return i;
> +	}
> +
> +	return -1;
> +}
> +
> +/** test_audio_pulse:
> + *
> + * Send pulse signals (one infinite positive pulse followed by a negative one)
> + * and check that:
> + *
> + * - The amplitude of the pulses is correct
> + * - All channels switch from a positive signal to a negative one at the same
> + *   time (ie. all channels are aligned)
> + */
>  static bool test_audio_pulse(struct audio_state *state)
>  {
> -	bool success;
> +	bool success, amp_success, align_success;
>  	int32_t *recv;
>  	size_t recv_len, i, channel_len;
> +	ssize_t j;
>  	int streak, capture_chan;
>  	double *channel;
> +	int falling_edges[8];

How about introducing MAX_CHANNEL?

This looks good otherwise:

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

>  
>  	alsa_register_output_callback(state->alsa,
>  				      audio_output_pulse_callback, state,
>  				      PLAYBACK_SAMPLES);
>  
> +	/* Start by sending a positive signal */
> +	state->positive = true;
> +
>  	audio_state_start(state, "pulse");
>  
> +	for (i = 0; i < state->playback.channels; i++)
> +		falling_edges[i] = -1;
> +
>  	recv = NULL;
>  	recv_len = 0;
> -	success = false;
> -	while (!success && state->msec < AUDIO_TIMEOUT) {
> +	amp_success = false;
> +	while (!amp_success && state->msec < AUDIO_TIMEOUT) {
>  		audio_state_receive(state, &recv, &recv_len);
>  
>  		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
> @@ -1217,17 +1249,58 @@ static bool test_audio_pulse(struct audio_state *state)
>  						     state->capture.channels,
>  						     capture_chan);
>  
> -			if (detect_pulse_amplitude(channel, channel_len))
> +			/* Check whether the amplitude is fine */
> +			if (detect_pulse_amplitude(channel, channel_len,
> +			    state->positive))
>  				streak++;
>  			else
>  				streak = 0;
>  
> +			/* If we're now sending a negative signal, detect the
> +			 * falling edge */
> +			j = detect_falling_edge(channel, channel_len);
> +			if (!state->positive && j >= 0) {
> +				falling_edges[i] = recv_len * state->recv_pages
> +						   + j;
> +			}
> +
>  			free(channel);
>  		}
>  
> -		success = streak == MIN_STREAK * state->playback.channels;
> +		amp_success = streak == MIN_STREAK * state->playback.channels;
> +
> +		if (amp_success && state->positive) {
> +			/* Switch to a negative signal after we've detected the
> +			 * positive one. */
> +			state->positive = false;
> +			amp_success = false;
> +			streak = 0;
> +			igt_debug("Switching to negative pulse\n");
> +		}
> +	}
> +
> +	/* Check alignment between all channels by comparing the index of the
> +	 * falling edge. */
> +	align_success = true;
> +	for (i = 0; i < state->playback.channels; i++) {
> +		if (falling_edges[i] < 0) {
> +			igt_debug("Falling edge not detected for channel %zu\n",
> +				  i);
> +			align_success = false;
> +			continue;
> +		}
> +
> +		if (abs(falling_edges[0] - falling_edges[i]) >
> +		    PULSE_ALIGN_ACCURACY) {
> +			igt_debug("Channel alignment mismatch: "
> +				  "channel 0 has a falling edge at index %d "
> +				  "while channel %zu has index %d\n",
> +				  falling_edges[0], i, falling_edges[i]);
> +			align_success = false;
> +		}
>  	}
>  
> +	success = amp_success && align_success;
>  	audio_state_stop(state, success);
>  
>  	free(recv);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev