[igt-dev] [PATCH i-g-t 4/5] tests/kms_chamelium: add S24_LE and S32_LE audio tests
Martin Peres
martin.peres at linux.intel.com
Mon May 20 13:43:01 UTC 2019
On 20/05/2019 16:10, Ser, Simon wrote:
> On Mon, 2019-05-20 at 13:20 +0100, Peres, Martin wrote:
>> On 17/05/2019 19:03, Ser, Simon wrote:
>>> This adds a new test_formats array, which is a list of PCM formats we want to
>>> run the tests with. We try to run tests for all combinations of sampling rates
>>> and formats.
>>>
>>> The ALSA callback now needs to fill the playback buffer differently depending
>>> on the format.
>>>
>>> Signed-off-by: Simon Ser <simon.ser at intel.com>
>>> ---
>>> tests/kms_chamelium.c | 86 ++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 60 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>> index af9f54d1f4c7..88356dd232b6 100644
>>> --- a/tests/kms_chamelium.c
>>> +++ b/tests/kms_chamelium.c
>>> @@ -795,8 +795,17 @@ static int test_frequencies[] = {
>>>
>>> static int test_frequencies_count = sizeof(test_frequencies) / sizeof(int);
>>>
>>> +static const snd_pcm_format_t test_formats[] = {
>>> + SND_PCM_FORMAT_S16_LE,
>>> + SND_PCM_FORMAT_S24_LE,
>>> + SND_PCM_FORMAT_S32_LE,
>>> +};
>>> +
>>> +static const size_t test_formats_count = sizeof(test_formats) / sizeof(test_formats[0]);
>>> +
>>> struct audio_state {
>>> struct audio_signal *signal;
>>> + snd_pcm_format_t format;
>>> atomic_bool run;
>>> };
>>>
>>> @@ -805,7 +814,19 @@ audio_output_callback(void *data, void *buffer, int samples)
>>> {
>>> struct audio_state *state = data;
>>>
>>> - audio_signal_fill_s16_le(state->signal, buffer, samples);
>>> + switch (state->format) {
>>> + case SND_PCM_FORMAT_S16_LE:
>>> + audio_signal_fill_s16_le(state->signal, buffer, samples);
>>> + break;
>>> + case SND_PCM_FORMAT_S24_LE:
>>> + audio_signal_fill_s24_le(state->signal, buffer, samples);
>>> + break;
>>> + case SND_PCM_FORMAT_S32_LE:
>>> + audio_signal_fill_s32_le(state->signal, buffer, samples);
>>> + break;
>>> + default:
>>> + assert(false); /* unreachable */
>>> + }
>>>
>>> return state->run ? 0 : -1;
>>> }
>>> @@ -881,6 +902,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
>>> audio_signal_synthesize(signal);
>>>
>>> state.signal = signal;
>>> + state.format = playback_format;
>>> state.run = true;
>>> alsa_register_output_callback(alsa, audio_output_callback, &state,
>>> PLAYBACK_SAMPLES);
>>> @@ -1027,6 +1049,22 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
>>> return success;
>>> }
>>>
>>> +static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
>>> + int channels, int sampling_rate)
>>> +{
>>> + if (!alsa_test_output_configuration(alsa, format, channels,
>>> + sampling_rate)) {
>>> + igt_debug("Skipping test with format %s, sampling rate %d Hz "
>>> + "and %d channels because at least one of the "
>>> + "selected output devices doesn't support this "
>>> + "configuration\n",
>>> + snd_pcm_format_name(format),
>>> + sampling_rate, channels);
>>> + return false;
>>> + }
>>> + return true;
>>> +}
>>> +
>>> static void
>>> test_display_audio(data_t *data, struct chamelium_port *port,
>>> const char *audio_device, enum test_edid edid)
>>> @@ -1039,7 +1077,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>>> struct igt_fb fb;
>>> drmModeModeInfo *mode;
>>> drmModeConnector *connector;
>>> - int fb_id, i;
>>> + int fb_id, i, j;
>>> int channels, sampling_rate;
>>> snd_pcm_format_t format;
>>>
>>> @@ -1076,35 +1114,31 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>>> run = false;
>>> success = true;
>>> for (i = 0; i < test_sampling_rates_count; i++) {
>>> - ret = alsa_open_output(alsa, audio_device);
>>> - igt_assert(ret >= 0);
>>> -
>>> - /* TODO: playback with different formats */
>>> - /* TODO: playback on all 8 available channels */
>>> - format = SND_PCM_FORMAT_S16_LE;
>>> - channels = PLAYBACK_CHANNELS;
>>> - sampling_rate = test_sampling_rates[i];
>>> -
>>> - if (!alsa_test_output_configuration(alsa, format, channels,
>>> - sampling_rate)) {
>>> - igt_debug("Skipping test with format %s, sample rate "
>>> - "%d Hz and %d channels because at least one "
>>> - "of the selected output devices doesn't "
>>> - "support this configuration\n",
>>> - snd_pcm_format_name(format),
>>> - channels, sampling_rate);
>>> - continue;
>>> - }
>>> + for (j = 0; j < test_formats_count; j++) {
>>> + ret = alsa_open_output(alsa, audio_device);
>>> + igt_assert(ret >= 0);
>>> +
>>> + /* TODO: playback with different formats */
>>
>> You forgot to remove the TODO.
>
> Derp
>
>>> + /* TODO: playback on all 8 available channels */
>>> + format = test_formats[j];
>>> + channels = PLAYBACK_CHANNELS;
>>> + sampling_rate = test_sampling_rates[i];
>>>
>>> - run = true;
>>> + if (!test_audio_configuration(alsa, format, channels,
>>> + sampling_rate))
>>> + continue;
>>>
>>> - success &= do_test_display_audio(data, port, alsa, format,
>>> - channels, sampling_rate);
>>> + run = true;
>>>
>>> - alsa_close_output(alsa);
>>> + success &= do_test_display_audio(data, port, alsa,
>>> + format, channels,
>>> + sampling_rate);
>>> +
>>> + alsa_close_output(alsa);
>>> + }
>>> }
>>>
>>> - /* Make sure we tested at least one frequency. */
>>> + /* Make sure we tested at least one frequency and format. */
>>> igt_assert(run);
>>
>> Do we really want to fail if alsa did not export any format?
>>
>> I mean, maybe we should just skip in this condition and have another
>> test with EDID that would test that the audio driver parses the EDIDs
>> correctly and exposes the right formats.
>
> Hmm. I'd rather keep it until we have this test.
Fair-enough :)
>
> However checking whether ALSA exposes the correct formats isn't going
> to work: the Chamelium device only exposes 16 and 24-bit formats but
> ALSA reports a 32-bit format…
>
> However /proc/asound parses and reports correct values. Maybe we could
> have a test that uses this instead.
Sounds great! So we'll want to either use the inject test for this, or
use a chamelium test for this... or both since chamelium is conceptually
better :)
Martin
>
>> Regardless, this patch looks good with the above TODO removed:
>> Reviewed-by: Martin Peres <martin.peres at linux.intel.com>
>>
>>
>>> /* Make sure all runs were successful. */
>>> igt_assert(success);
>>>
>>
>>
More information about the igt-dev
mailing list