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

[Jacopo Mondi]

Hi Umang,

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.
> +	 */
> +	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>

Thanks
   j
> +{
> +	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_;
> +private:
> +	bool satisfyCameraStreams(unsigned int numStreams);
>  };
> --
> 2.31.1
>