[libcamera-devel] [PATCH] test: gstreamer_single_stream: Check for cameras before running

[Umang Jain]

Hi Nicolas,

I just posted a updated version for this under "[PATCH 2/2] test: 
gstreamer: Check availability of cameras before running"

On 7/21/22 19:19, Nicolas Dufresne wrote:
> Hi Umang,
>
> thanks for the patch ...
>
> Le jeudi 21 juillet 2022 à 16:00 +0530, Umang Jain via libcamera-devel a écrit :
>> Before running or setting up the pipeline, check for cameras availablity
>> first. If no cameras are available, skip the test.
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> ---
>>   test/gstreamer/gstreamer_single_stream_test.cpp | 13 +++++++++++++
>>   1 file changed, 13 insertions(+)
>>
>> diff --git a/test/gstreamer/gstreamer_single_stream_test.cpp b/test/gstreamer/gstreamer_single_stream_test.cpp
>> index a0dd12cf..1e0801e8 100644
>> --- a/test/gstreamer/gstreamer_single_stream_test.cpp
>> +++ b/test/gstreamer/gstreamer_single_stream_test.cpp
>> @@ -8,6 +8,8 @@
>>   #include <iostream>
>>   #include <unistd.h>
>>   
>> +#include <libcamera/libcamera.h>
>> +
>>   #include <gst/gst.h>
>>   
>>   #include "gstreamer_test.h"
>> @@ -29,6 +31,17 @@ protected:
>>   		if (status_ != TestPass)
>>   			return status_;
>>   
>> +		libcamera::CameraManager cm;
>> +		cm.start();
>> +
>> +		bool cameraFound = cm.cameras().size() > 1 ? true : false;
>> +		if (!cameraFound) {
>> +			cm.stop();
>> +			return TestSkip;
>> +		}
>> +
>> +		cm.stop();
>> +
> I think starting a CM here might be problematic (even racy), specially that it
> is not destroyed before GStreamer starts using it. But perhaps things have
> improved ...


Right, I did overlook that part.

In the new version, [PATCH 2/2] test: gstreamer: Check availability of 
cameras before running

the CameraManager instance is limited to a function (local instance), so 
probably this is solved with the new patch...

>
> At the same time, I feel like this is avoiding some obvious testing. Instead of
> that, perhaps you should let if fail and handle the error code ? Or even better,

The generated error is:

                     g_printerr("Unable to set the pipeline to the 
playing state.\n");

I haven't looked deep, but handling the error code directly from the 
pipeline i.e. GST_STATE_CHANGE_FAILURE
might mean a lot of things, including unavailability of the camera. Not 
sure how we can bifurcate and handle it specifically

> perhaps you should use the device provider instead of custom libcamera code ?


I looked into that earlier, but didn't get much far with device 
provider. I guess I need to circle back.

>
> regards,
> Nicolas
>
>>   		const gchar *streamDescription = "videoconvert ! fakesink";
>>   		g_autoptr(GError) error0 = NULL;
>>   		stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,