[igt-dev] [PATCH i-g-t v6 5/7] tests/kms_chamelium: run audio test with multiple sampling rates
Ser, Simon
simon.ser at intel.com
Tue Apr 23 12:59:29 UTC 2019
On Tue, 2019-04-23 at 10:58 +0300, Martin Peres wrote:
> 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 at 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.
Nack. This has just been moved around, see line 815 above. This is
necessary because of the new chamelium_get_audio_format call.
> > /* 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.
Ack. Moved to patch 3 "tests/kms_chamelium: test we receive a signal
from both audio channels" which introduced this variable.
> Otherwise, the code looks OK:
>
> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
More information about the igt-dev
mailing list