[libcamera-devel] [PATCH 2/2] test: gstreamer: Check availability of cameras before running

[Laurent Pinchart]

Hello,

On Thu, Jul 21, 2022 at 04:23:42PM +0200, Jacopo Mondi via libcamera-devel wrote:
> On Thu, Jul 21, 2022 at 07:15:31PM +0530, Umang Jain via libcamera-devel wrote:
> > Move the logic for checking the availability of cameras from
> > multi_stream_test to gstreamer test base class. Since
> > single_stream_class always assumes that a camera is available on the
> > system (which is not always the case for e.g. RPi in CI/CD environments)
> > it makes sense to have the availability check in the base class.
> > If no cameras are available, the behaviour should be to skip instead
> > of a failure.
> >
> > We currently have 2 tests for gstreamer differing based on number
> > of streams supported by the camera. Hence, the camera availability
> > is checked in conjunction with the number of the streams required by
> > the derived class.
> >
> > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> >  .../gstreamer/gstreamer_multi_stream_test.cpp | 20 +----------
> >  test/gstreamer/gstreamer_test.cpp             | 33 ++++++++++++++++++-
> >  test/gstreamer/gstreamer_test.h               |  4 ++-
> >  3 files changed, 36 insertions(+), 21 deletions(-)
> >
> > diff --git a/test/gstreamer/gstreamer_multi_stream_test.cpp b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > index 112f1dee..b8387c10 100644
> > --- a/test/gstreamer/gstreamer_multi_stream_test.cpp
> > +++ b/test/gstreamer/gstreamer_multi_stream_test.cpp
> > @@ -29,7 +29,7 @@ class GstreamerMultiStreamTest : public GstreamerTest, public Test
> >  {
> >  public:
> >  	GstreamerMultiStreamTest()
> > -		: GstreamerTest()
> > +		: GstreamerTest(2)
> 
> As this is a test, I think it's ok even if it's a tad rough
> 
> >  	{
> >  	}
> >
> > @@ -39,24 +39,6 @@ protected:
> >  		if (status_ != TestPass)
> >  			return status_;
> >
> > -		/* 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;
> > -		}
> > -
> >  		const gchar *streamDescription = "queue ! fakesink";
> >  		g_autoptr(GError) error = NULL;
> >
> > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> > index cfb8afc6..4668aa00 100644
> > --- a/test/gstreamer/gstreamer_test.cpp
> > +++ b/test/gstreamer/gstreamer_test.cpp
> > @@ -5,6 +5,7 @@
> >   * libcamera Gstreamer element API tests
> >   */
> >
> > +#include <libcamera/libcamera.h>
> 
> Empty line ?
> 
> >  #include <libcamera/base/utils.h>
> >
> >  #include "gstreamer_test.h"
> > @@ -25,9 +26,10 @@ const char *__asan_default_options()
> >  }
> >  }
> >
> > -GstreamerTest::GstreamerTest()
> > +GstreamerTest::GstreamerTest(unsigned int numStreams)
> >  	: pipeline_(nullptr), libcameraSrc_(nullptr)
> >  {
> > +
> 
> No empty line :)
> 
> 
> >  	/*
> >  	* GStreamer by default spawns a process to run the
> >  	* gst-plugin-scanner helper. If libcamera is compiled with ASan
> > @@ -67,9 +69,38 @@ GstreamerTest::GstreamerTest()
> >  		return;
> >  	}
> >
> > +	/*
> > +	 * Atleast one camera should be available with numStreams streams
> > +	 * otherwise, skip the test entirely.

	 * Atleast one camera should be available with numStreams streams,
	 * otherwise skip the test entirely.

> > +	 */
> > +	if (!satisfyCameraStreams(numStreams)) {
> > +		status_ = TestSkip;
> > +		return;
> > +	}
> > +
> >  	status_ = TestPass;
> >  }
> >
> > +bool GstreamerTest::satisfyCameraStreams(unsigned int numStreams)
> 
> Bikeshedding on the function name apart...
> And with Nicolas comment clarified
> 
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> > +{
> > +	libcamera::CameraManager cm;
> > +	bool cameraFound = false;
> > +
> > +	cm.start();
> > +
> > +	for (auto &camera : cm.cameras()) {
> > +		if (camera->streams().size() < numStreams)
> > +			continue;
> > +
> > +		cameraFound = true;
> > +		break;
> > +	}
> > +
> > +	cm.stop();
> > +
> > +	return cameraFound;
> > +}
> > +
> >  GstreamerTest::~GstreamerTest()
> >  {
> >  	g_clear_object(&pipeline_);
> > diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h
> > index 35adab0e..fa41721f 100644
> > --- a/test/gstreamer/gstreamer_test.h
> > +++ b/test/gstreamer/gstreamer_test.h
> > @@ -15,7 +15,7 @@
> >  class GstreamerTest
> >  {
> >  public:
> > -	GstreamerTest();
> > +	GstreamerTest(unsigned int numStreams = 1);
> >  	virtual ~GstreamerTest();
> >
> >  protected:
> > @@ -27,4 +27,6 @@ protected:
> >  	GstElement *pipeline_;
> >  	GstElement *libcameraSrc_;
> >  	int status_;

Blank line.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> > +private:
> > +	bool satisfyCameraStreams(unsigned int numStreams);
> >  };

-- 
Regards,

Laurent Pinchart