[libcamera-devel] [PATCH v2] Add enable_auto_focus option to the GStreamer plugin
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Mon Jun 5 17:33:31 CEST 2023
Hi Cedric,
(CC'ing Nicolas)
Thank you for the patch.
On Mon, Jun 05, 2023 at 04:44:05PM +0200, Cedric Nugteren via libcamera-devel wrote:
> 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
I think you meant "its auto-focus" instead of "it auto-focus" ?
> 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.
This should be written
Bug: https://bugs.libcamera.org/show_bug.cgi?id=188.
which I realize isn't documented anywhere :-S I wonder where to best
mention this. Is there any particular place in the documentation that
would have caught your eyes ?
> 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.
Perfect :-)
> ---
> 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);
Nicolas, is there any particular rule when it comes to naming properties
for GStreamer elements ?
>
> 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);
Incorrect indentation (you should use tabs instead of spaces, and the
last line is indented by one space too much.
We have a tool, in utils/checkstyle.py, that checks for common coding
style violations and can in many cases propose fixes (see
https://libcamera.org/coding-style.html#tools). I recommend installing
it as a git post-commit hook, how to do so is also documented in the
previous URL.
Please take the output of the tool with a pinch of salt though. For code
formatting, the tool is based on clang-format, and while it does a
fairly good job, not all suggestions are perfect. You can ignore some of
the warnings if you think that's appropriate (mostly based on the coding
style of the existing code in the file).
> + 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);
> + } 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'"));
> + }
> + }
We should be able to enable and disable auto-focus at runtime as well. I
don't want to ask for too much yak-shaving, so I'm fine with this patch
as an initial solution. Please note, however, that once we enable
support for controls more generically in libcamerasrc, the name of the
property may change.
> +
> 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);
> +
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list