Re: [igt-dev] [PATCH i-g-t v4 1/5] tests/kms_chamelium: add dp-audio test

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

On 17/04/2019 15:57, Ser, Simon wrote:
> On Wed, 2019-04-17 at 15:17 +0300, Martin Peres wrote:
>>>>> +	if (dump_fd >= 0) {
>>>>> +		close(dump_fd);
>>>>> +		if (streak == MIN_STREAK) {
>>>>> +			/* Test succeeded, no need to keep the captured data */
>>>>> +			unlink(dump_path);
>>>>> +		} else
>>>>> +			igt_debug("Saved captured audio data to %s\n", dump_path);
>>>>> +		free(dump_path);
>>>>> +	}
>>>>> +
>>>>> +	free(recv);
>>>>> +	free(buf);
>>>>> +	free(channel);
>>>>> +
>>>>> +	ok = chamelium_stream_stop_realtime_audio(stream);
>>>>> +	igt_assert(ok);
>>>>> +
>>>>> +	audio_file = chamelium_stop_capturing_audio(data->chamelium,
>>>>> +						    port);
>>>>> +	if (audio_file) {
>>>>> +		igt_debug("Audio file saved on the Chamelium in %s\n",
>>>>> +			  audio_file->path);
>>>>> +		chamelium_destroy_audio_file(audio_file);
>>>>> +	}
>>>>
>>>> I would suggest to only dump this file on failure, not when having a
>>>> success.
>>>
>>> 1. We can't decide this after-the-fact: we can only decide whether we
>>>    dump or not before starting the capture.
>>> 2. There are two kinds of audio dumps: local (on the DUT, see dump_fd) 
>>>    and remote (on the Chamelium, see the last param of 
>>>    chamelium_start_capturing_audio). If the file has been dumped on the
>>>    Chamelium, chamelium_stop_capturing_audio will return the audio file
>>>    details. It's sometimes useful to enable Chamelium dumps for
>>>    debugging purposes.
>>
>> Of course! Sorry for the confusion! Where are we dumping the generated
>> and received WAVs when the test is failing then? Is that a TODO?
> 
> So, this is done a little earlier (see the quoted code above). We do
> also dump audio data for tests that succeed, but unlink the file in
> that case. This allows us not to keep all of the captured data in
> memory (dumps are generally worth a couple of MiB) and to keep the code
> simple (no dynamic memory allocation). But honestly I'm not feeling
> strongly about this and I'm open to change it.
> 

OK, so we dump the received the WAV file and we also dump which
frequencies we expected. That should be sufficient!

When IGT learns to store external files in the .json, we'll have to
revisit the test to embed these new files.

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev