[libcamera-devel] [PATCH v2] Add enable_auto_focus option to the GStreamer plugin
Nicolas Dufresne
nicolas at ndufresne.ca
Mon Jun 5 19:54:28 CEST 2023
Hi,
Le lundi 05 juin 2023 à 19:01 +0300, Laurent Pinchart a écrit :
> Hi Nicolas,
>
> On Mon, Jun 05, 2023 at 11:47:51AM -0400, Nicolas Dufresne via libcamera-devel wrote:
> > Le lundi 05 juin 2023 à 16:44 +0200, Cedric Nugteren via libcamera-devel a écrit :
> > > Cameras such as the PiCam 3 support auto-focus, but the GStreamer plugin
> > > for libcamera does not enable auto-focus. With this patch auto-focus can
> > > be enabled for cameras that support it. By default it is disabled, which
> > > means default behaviour remains unchanged. For cameras that do not
> > > support auto-focus, an error message shows up if it auto-focus is
> > > enabled.
> > >
> > > This was tested on cameras that do not support auto-focus (e.g. PiCam2)
> > > and was tested on a camera that does support auto-focus (PiCam3) by
> > > enabling it, observing auto-focus, and by disabling it or by not setting
> > > the option, both resulting in auto-focus being disabled.
> > >
> > > This should close https://bugs.libcamera.org/show_bug.cgi?id=188.
> > >
> > > Signed-off-by: Cedric Nugteren <web at cedricnugteren.nl>
> > > ---
> > > Changes since v1:
> > > - Now re-using the GParamSpec variables instead of re-creating.
> > > - Use bool instead of gboolean for enable_auto_focus.
> > > - Remove changes from the reconfigure_element functions.
> > > ---
> > > src/gstreamer/gstlibcameraprovider.cpp | 12 +++++++++++
> > > src/gstreamer/gstlibcamerasrc.cpp | 29 +++++++++++++++++++++++++-
> > > 2 files changed, 40 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp
> > > index 6eb0a0eb..579cb8c0 100644
> > > --- a/src/gstreamer/gstlibcameraprovider.cpp
> > > +++ b/src/gstreamer/gstlibcameraprovider.cpp
> > > @@ -31,6 +31,7 @@ GST_DEBUG_CATEGORY_STATIC(provider_debug);
> > >
> > > enum {
> > > PROP_DEVICE_NAME = 1,
> > > + PROP_ENABLE_AUTO_FOCUS = 2,
> > > };
> > >
> > > #define GST_TYPE_LIBCAMERA_DEVICE gst_libcamera_device_get_type()
> > > @@ -40,6 +41,7 @@ G_DECLARE_FINAL_TYPE(GstLibcameraDevice, gst_libcamera_device,
> > > struct _GstLibcameraDevice {
> > > GstDevice parent;
> > > gchar *name;
> > > + bool enable_auto_focus = false;
> > > };
> > >
> > > G_DEFINE_TYPE(GstLibcameraDevice, gst_libcamera_device, GST_TYPE_DEVICE)
> > > @@ -56,6 +58,7 @@ gst_libcamera_device_create_element(GstDevice *device, const gchar *name)
> > > g_assert(source);
> > >
> > > g_object_set(source, "camera-name", GST_LIBCAMERA_DEVICE(device)->name, nullptr);
> > > + g_object_set(source, "enable-auto-focus", GST_LIBCAMERA_DEVICE(device)->enable_auto_focus, nullptr);
> > >
> > > return source;
> > > }
> > > @@ -82,6 +85,9 @@ gst_libcamera_device_set_property(GObject *object, guint prop_id,
> > > case PROP_DEVICE_NAME:
> > > device->name = g_value_dup_string(value);
> > > break;
> > > + case PROP_ENABLE_AUTO_FOCUS:
> > > + device->enable_auto_focus = g_value_get_boolean(value);
> > > + break;
> > > default:
> > > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > > break;
> > > @@ -121,6 +127,12 @@ gst_libcamera_device_class_init(GstLibcameraDeviceClass *klass)
> > > (GParamFlags)(G_PARAM_STATIC_STRINGS | G_PARAM_WRITABLE |
> > > G_PARAM_CONSTRUCT_ONLY));
> > > g_object_class_install_property(object_class, PROP_DEVICE_NAME, pspec);
> > > + pspec = g_param_spec_boolean("enable-auto-focus",
> > > + "Enable auto-focus",
> > > + "Enable auto-focus if set to true, "
> > > + "disable it if set to false",
> > > + FALSE, G_PARAM_WRITABLE);
> > > + g_object_class_install_property(object_class, PROP_ENABLE_AUTO_FOCUS, pspec);
> > > }
> > >
> > > static GstDevice *
> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > > index a10cbd4f..b2d8456b 100644
> > > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > > @@ -146,6 +146,7 @@ struct _GstLibcameraSrc {
> > > GstTask *task;
> > >
> > > gchar *camera_name;
> > > + bool enable_auto_focus = false;
> > >
> > > GstLibcameraSrcState *state;
> > > GstLibcameraAllocator *allocator;
> > > @@ -154,7 +155,8 @@ struct _GstLibcameraSrc {
> > >
> > > enum {
> > > PROP_0,
> > > - PROP_CAMERA_NAME
> > > + PROP_CAMERA_NAME,
> > > + PROP_ENABLE_AUTO_FOCUS,
> > > };
> > >
> > > G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src, GST_TYPE_ELEMENT,
> > > @@ -577,6 +579,18 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
> > > gst_flow_combiner_add_pad(self->flow_combiner, srcpad);
> > > }
> > >
> > > + if (self->enable_auto_focus) {
> > > + const ControlInfoMap &infoMap = state->cam_->controls();
> > > + if (infoMap.find(&controls::AfMode) != infoMap.end()) {
> > > + state->initControls_.set(controls::AfMode, controls::AfModeContinuous);
> >
> > I don't really like the idea of mapping modes to a boolean here. I think this
> > should be an GEnum type of property. Some mode requires some actions to actually
> > work, best to just skip these for now.
>
> In the long run, we need to map all libcamera controls to GStreamer
> properties. Given the amount of controls, I'm hoping we will be able to
> either do so in a dynamic way (similar to what v4l2src does when mapping
> V4L2 controls), or with code generation. That's why I'm not sure if we
> need to spend more time now on mapping better this particular control
> manually (as long as we don't need to preserve backward compatibility).
> I trust your judgement on this topic.
Properties are part of the API, so renaming them changing their type will break
applications using it. I'd strongly suggest to make this an enum (this is easy)
with the two supporting values. Adding more enums values later is not a break.
>
> > > + } else {
> > > + GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> > > + ("Failed to enable auto focus"),
> > > + ("AfMode not found in camera controls, "
> > > + "please retry with 'enable-auto-focus=false'"));
> > > + }
> > > + }
> > > +
> > > ret = state->cam_->start(&state->initControls_);
> > > if (ret) {
> > > GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> > > @@ -659,6 +673,9 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,
> > > g_free(self->camera_name);
> > > self->camera_name = g_value_dup_string(value);
> > > break;
> > > + case PROP_ENABLE_AUTO_FOCUS:
> > > + self->enable_auto_focus = g_value_get_boolean(value);
> > > + break;
> > > default:
> > > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > > break;
> > > @@ -676,6 +693,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_ENABLE_AUTO_FOCUS:
> > > + g_value_set_boolean(value, self->enable_auto_focus);
> > > + break;
> > > default:
> > > G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > > break;
> > > @@ -844,4 +864,11 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> > > | G_PARAM_READWRITE
> > > | G_PARAM_STATIC_STRINGS));
> > > g_object_class_install_property(object_class, PROP_CAMERA_NAME, spec);
> > > + spec = g_param_spec_boolean("enable-auto-focus",
> > > + "Enable auto-focus",
> > > + "Enable auto-focus if set to true, "
> > > + "disable it if set to false",
> > > + FALSE, G_PARAM_WRITABLE);
> > > + g_object_class_install_property(object_class, PROP_ENABLE_AUTO_FOCUS, spec);
> > > +
> > > }
>
More information about the libcamera-devel
mailing list