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

[Umang Jain]

On 7/21/22 17:09, Jacopo Mondi wrote:
> Hi Umang
>
> On Thu, Jul 21, 2022 at 05:04:24PM +0530, Umang Jain wrote:
>> Hi Jacopo,
>>
>> On 7/21/22 16:16, Jacopo Mondi wrote:
>>> Hi Umang,
>>>
>>> On Thu, Jul 21, 2022 at 04:00:20PM +0530, Umang Jain via libcamera-devel wrote:
>>>> 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>
>>> Is this to fix the recurring failing test on rpi ?
>>
>> Yes,  I tested on Rpi that no cameras will fail the test in the same way as
>> https://buildbot.libcamera.org/#/builders/16/builds/168
>>
>> Having a camera passes the test, so the test assumes that a camera is
>> present on the system.
>>
>>> Nit: could you add a Fixes tag ? I also used Reported-by: with a link
>>> to a failing build from buildbot. Not that we use it now, but it's
>>> nice for statistics ?
>>>
>>>> ---
>>>>    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();
>>>> +
>>> The multistream test has a very similar:
>>>
>>> 		/* Check if platform supports multistream capture */
>>> 		libcamera::CameraManager cm;
>>> 		cm.start();
>>> 		bool cameraFound = false;
>>> 		for (auto &camera : cm.cameras()) {
>>> 			if (camera->streams().size() > 1) {
>>> 				cameraName_ = camera->id();
>>> 				cameraFound = true;
>>> 				cm.stop();
>>> 				break;
>>> 			}
>>> 		}
>>>
>>> 		if (!cameraFound) {
>>> 			cm.stop();
>>> 			return TestSkip;
>>> 		}
>>>
>>> Is this something that should go in the base class ?
>>
>> I did contemplate putting in the base class itself. That approach might end
>> up with multiple cameraManager's start()/stop() cycles (which  I think is
>> Okay?).
>>
>> For e.g. in the multistream test, the base class would start() -> find
>> cameras ->stop() and the multistream class would start() -> finding cameras
>> with multistream support -> stop(). Then the actual test starts with
>> libcamerasrc element. So there's additional one cycle in this case.
>>
>> whereas single stream test wouldn't need to have any start() -> stop()
>> sequence as base class would handle it.
>>
> Isn't it just a numStream parameter to the base class init() function ?


Not sure what you mean, grepping numStreams denotes:

[uajain at fedora libcamera]$ git grep -ni "numStream"
src/libcamera/pipeline/simple/simple.cpp:198: unsigned int numStreams,
src/libcamera/pipeline/simple/simple.cpp:350: unsigned int numStreams,
src/libcamera/pipeline/simple/simple.cpp:352:   : Camera::Private(pipe), 
streams_(numStreams)
src/libcamera/pipeline/simple/simple.cpp:1266:  unsigned int numStreams = 1;
src/libcamera/pipeline/simple/simple.cpp:1284: numStreams = streams;
src/libcamera/pipeline/simple/simple.cpp:1307: 
std::make_unique<SimpleCameraData>(this, numStreams, sensor);

>
>> Probably it's worth to do it in the base class ... Let me see and get back
>> to you
>>
>>>>    		const gchar *streamDescription = "videoconvert ! fakesink";
>>>>    		g_autoptr(GError) error0 = NULL;
>>>>    		stream0_ = gst_parse_bin_from_description_full(streamDescription, TRUE,
>>>> --
>>>> 2.31.1
>>>>