[libcamera-devel] [PATCH v1] gstreamer: Simplify camera config generation call

[Barnabás Pőcze]

Hi


2023. május 2., kedd 23:13 keltezéssel, Umang Jain <umang.jain at ideasonboard.com> írta:

> [...]
> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > index 6eb0a0eb..4f325ad4 100644
> > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > @@ -128,11 +128,9 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
> >   {
> >   	g_autoptr(GstCaps) caps = gst_caps_new_empty();
> >   	const gchar *name = camera->id().c_str();
> > -	StreamRoles roles;
> >
> > -	roles.push_back(StreamRole::VideoRecording);
> > -	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
> > -	if (!config || config->size() != roles.size()) {
> 
> I would leave it as it is, as it's easier to add new roles (just a
> .push_back() call) and the adjoining pieces automatically makes it fall
> into place..

That is a fair point, I did not really explain the motivation. I would like to
post a second version of https://patchwork.libcamera.org/patch/18285/ that would
also remove the `StreamRoles` alias altogether.

If this current version is undesirable, then I can incorporate this change into
the second version of the aforementioned patch series as follows:

diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
index 6eb0a0eb..332837e4 100644
--- a/src/gstreamer/gstlibcameraprovider.cpp
+++ b/src/gstreamer/gstlibcameraprovider.cpp
@@ -126,13 +126,12 @@ gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)
 static GstDevice *
 gst_libcamera_device_new(const std::shared_ptr<Camera> &camera)
 {
+	static const StreamRole roles[] = { StreamRole::VideoRecording };
 	g_autoptr(GstCaps) caps = gst_caps_new_empty();
 	const gchar *name = camera->id().c_str();
-	StreamRoles roles;
 
-	roles.push_back(StreamRole::VideoRecording);
 	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles);
-	if (!config || config->size() != roles.size()) {
+	if (!config || config->size() != std::size(roles)) {
 		GST_ERROR("Failed to generate a default configuration for %s", name);
 		return nullptr;
 	}

would this be then acceptable?


> > +	std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration({ StreamRole::VideoRecording });
> > +	if (!config || config->size() != 1) {
> >   		GST_ERROR("Failed to generate a default configuration for %s", name);
> >   		return nullptr;
> >   	}
> [...]


Regards,
Barnabás Pőcze