[libcamera-devel] [PATCH] gstreamer: Add 'list-cameras' property

[Nicolas Dufresne]

Le mardi 04 juillet 2023 à 08:31 +0200, Umang Jain a écrit :
> Hi Nicolas
> 
> On 7/3/23 10:22 PM, Nicolas Dufresne wrote:
> > Hi Ujain,
> > 
> > Le lundi 03 juillet 2023 à 21:57 +0200, Umang Jain a écrit :
> > > Add a new property called 'list-cameras' on libcamerasrc to enumerate
> > > and expose the cameras present on the system. The enumeration will help
> > > appplications using libcamerasrc to know and set the 'camera-name'
> > > property in order to use that camera with libcamerasrc.
> > > 
> > > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > I'm sorry, but I have to reject this. This is the job of the device provider. If
> > some information is missing in the provider, just add it there.
> 
> Makes sense. I'll try writing a test to enumerate cameras on the device 
> provider and using a pre-configured libcamerasrc.
> 
> For the missing information, does the camera-supported controls be a 
> good candidate to get added to device-provider as well?  I see we are 
> already enumerating streams in there but I understand those are part of 
> the capabilities.

As discussed on IRC, I'd aim maybe for a set of supported features. We can
probably simply use G_TYPE_FLAGS property (shared on both the provided and the
libcamerasrc). Each flag would state which features are to be expected, and the
doc will say for each flags which controls are going to exist, or which "mode"
if a feature (like auto-focus) can support multiple mode which imply different
set of controls.

The idea is that libcamera controls don't match 1:1 to features, but some
controls works in tandem, e.g. if you didn't select a specific "mode" a specific
control may have no effect.

> > 
> > NAK
> > 
> > > ---
> > >   src/gstreamer/gstlibcamerasrc.cpp | 46 +++++++++++++++++++++++++++++++
> > >   test/gstreamer/gstreamer_test.cpp | 30 ++++++++++++++++++++
> > >   test/gstreamer/gstreamer_test.h   |  2 ++
> > >   3 files changed, 78 insertions(+)
> > > 
> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > > index 1f10136b..f337728a 100644
> > > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > > @@ -152,6 +152,7 @@ struct _GstLibcameraSrc {
> > >   enum {
> > >   	PROP_0,
> > >   	PROP_CAMERA_NAME,
> > > +	PROP_LIST_CAMERAS,
> > >   	PROP_AUTO_FOCUS_MODE,
> > >   };
> > >   
> > > @@ -666,6 +667,39 @@ gst_libcamera_src_close(GstLibcameraSrc *self)
> > >   	state->cm_.reset();
> > >   }
> > >   
> > > +static gboolean
> > > +gst_libcamera_src_set_cameras_list(GstLibcameraSrc *self, GValue *value)
> > > +{
> > > +	std::shared_ptr<CameraManager> cm;
> > > +	GValue val = G_VALUE_INIT;
> > > +	gint ret;
> > > +
> > > +	g_value_init(&val, G_TYPE_STRING);
> > > +	g_value_reset(value);
> > > +
> > > +	cm = gst_libcamera_get_camera_manager(ret);
> > > +	if (ret) {
> > > +		GST_ELEMENT_ERROR(self, LIBRARY, INIT,
> > > +				  ("Failed listing cameras."),
> > > +				  ("libcamera::CameraMananger::start() failed: %s", g_strerror(-ret)));
> > > +		return false;
> > > +	}
> > > +
> > > +	if (cm->cameras().empty()) {
> > > +		GST_ELEMENT_ERROR(self, RESOURCE, NOT_FOUND,
> > > +				  ("Could not find any supported camera on this system."),
> > > +				  ("libcamera::CameraMananger::cameras() is empty"));
> > > +		return false;
> > > +	}
> > > +
> > > +	for (const std::shared_ptr<Camera> &cam : cm->cameras()) {
> > > +		g_value_set_string(&val, cam->id().c_str());
> > > +		gst_value_array_append_value(value, &val);
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > >   static void
> > >   gst_libcamera_src_set_property(GObject *object, guint prop_id,
> > >   			       const GValue *value, GParamSpec *pspec)
> > > @@ -699,6 +733,9 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,
> > >   	case PROP_CAMERA_NAME:
> > >   		g_value_set_string(value, self->camera_name);
> > >   		break;
> > > +	case PROP_LIST_CAMERAS:
> > > +		gst_libcamera_src_set_cameras_list(self, value);
> > > +		break;
> > >   	case PROP_AUTO_FOCUS_MODE: {
> > >   		auto auto_focus_mode = self->controls->get(controls::AfMode).value_or(controls::AfModeManual);
> > >   		g_value_set_enum(value, auto_focus_mode);
> > > @@ -879,6 +916,15 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> > >   							     | G_PARAM_STATIC_STRINGS));
> > >   	g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> > >   
> > > +	spec = gst_param_spec_array("list-cameras", "Cameras list",
> > > +				    "Retrieve list of all cameras",
> > > +				    g_param_spec_string("camera-name", "Camera name",
> > > +							"Name of the camera", nullptr,
> > > +							(GParamFlags)(G_PARAM_READABLE
> > > +								      | G_PARAM_STATIC_STRINGS)),
> > > +				    (GParamFlags)(G_PARAM_READABLE | G_PARAM_STATIC_STRINGS));
> > > +	g_object_class_install_property(object_class, PROP_LIST_CAMERAS, spec);
> > > +
> > >   	spec = g_param_spec_enum("auto-focus-mode",
> > >   				 "Set auto-focus mode",
> > >   				 "Available options: AfModeManual, "
> > > diff --git a/test/gstreamer/gstreamer_test.cpp b/test/gstreamer/gstreamer_test.cpp
> > > index 6ad0c15c..7f3ccd2b 100644
> > > --- a/test/gstreamer/gstreamer_test.cpp
> > > +++ b/test/gstreamer/gstreamer_test.cpp
> > > @@ -97,6 +97,9 @@ bool GstreamerTest::checkMinCameraStreamsAndSetCameraName(unsigned int numStream
> > >   		break;
> > >   	}
> > >   
> > > +	for (auto &camera : cm.cameras())
> > > +		cameraNames_.push_back(strdup(camera->id().c_str()));
> > > +
> > >   	cm.stop();
> > >   
> > >   	return cameraFound;
> > > @@ -112,6 +115,9 @@ GstreamerTest::~GstreamerTest()
> > >   
> > >   int GstreamerTest::createPipeline()
> > >   {
> > > +	GValue cameras_list_array = G_VALUE_INIT;
> > > +	guint index, i;
> > > +
> > >   	libcameraSrc_ = gst_element_factory_make("libcamerasrc", "libcamera");
> > >   	pipeline_ = gst_pipeline_new("test-pipeline");
> > >   
> > > @@ -125,6 +131,30 @@ int GstreamerTest::createPipeline()
> > >   	g_object_set(libcameraSrc_, "camera-name", cameraName_.c_str(), NULL);
> > >   	g_object_ref_sink(libcameraSrc_);
> > >   
> > > +	g_value_init(&cameras_list_array, GST_TYPE_ARRAY);
> > > +	g_object_get_property(G_OBJECT(libcameraSrc_), "list-cameras", &cameras_list_array);
> > > +	if (gst_value_array_get_size(&cameras_list_array) != cameraNames_.size()) {
> > > +		g_printerr("libcamerasrc 'list-cameras' properties count does not match.\n");
> > > +		return TestFail;
> > > +	}
> > > +
> > > +	for (index = 0; index < cameraNames_.size(); index++) {
> > > +		bool matched = false;
> > > +
> > > +		for (i = 0; i < gst_value_array_get_size(&cameras_list_array); i++) {
> > > +			const char *cam = g_value_get_string(gst_value_array_get_value(&cameras_list_array, i));
> > > +			if (strcmp(cam, cameraNames_[index]) == 0) {
> > > +				matched = true;
> > > +				break;
> > > +			}
> > > +		}
> > > +
> > > +		if (!matched) {
> > > +			g_printerr("Camera %s not found in 'list-cameras' property.\n", cameraNames_[index]);
> > > +			return TestFail;
> > > +		}
> > > +	}
> > > +
> > >   	return TestPass;
> > >   }
> > >   
> > > diff --git a/test/gstreamer/gstreamer_test.h b/test/gstreamer/gstreamer_test.h
> > > index aa2261e2..5d61bc2a 100644
> > > --- a/test/gstreamer/gstreamer_test.h
> > > +++ b/test/gstreamer/gstreamer_test.h
> > > @@ -9,6 +9,7 @@
> > >   
> > >   #include <iostream>
> > >   #include <unistd.h>
> > > +#include <vector>
> > >   
> > >   #include <gst/gst.h>
> > >   
> > > @@ -29,6 +30,7 @@ protected:
> > >   	GstElement *libcameraSrc_;
> > >   	int status_;
> > >   
> > > +	std::vector<const char *> cameraNames_;
> > >   private:
> > >   	bool checkMinCameraStreamsAndSetCameraName(unsigned int numStreams);
> > >   };
>