[igt-dev] [PATCH i-g-t v6 5/7] tests/kms_chamelium: run audio test with multiple sampling rates
Martin Peres
martin.peres at linux.intel.com
Thu Apr 25 09:58:35 UTC 2019
On 23/04/2019 15:59, Ser, Simon wrote:
> 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.
OK, fair enough!
>
>>> /* 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.
OK! Please apply my R-b!
>
>> Otherwise, the code looks OK:
>>
>> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
More information about the igt-dev
mailing list