<div dir="ltr"><div>I tested this on the PiCamV3 and the auto-focus option still seems to work indeed.</div><div><br></div><div>Cedric<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jun 22, 2023 at 1:17 PM Umang Jain <<a href="mailto:umang.jain@ideasonboard.com">umang.jain@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">As more and more libcamera controls get plumbed, more member variables<br>
get introduced in struct GstLibcameraSrc. Instead of doing that, now<br>
maintain a single ControlList which is more appropriate to keep track<br>
of controls that get sets on libcamerasrc.<br>
<br>
This also brings easy validation of controls set on libcamerasrc. If<br>
the controls are not supported by camera, abort early and report what is<br>
not supported.<br>
<br>
The current patch migrates previously introduced control,<br>
auto-focus-mode, to be set directly in control list.<br>
<br>
Signed-off-by: Umang Jain <<a href="mailto:umang.jain@ideasonboard.com" target="_blank">umang.jain@ideasonboard.com</a>><br>
---<br>
Cedric, Can you please help test this?<br>
<br>
(I'm travelling so have tested this on my RPi and inferring from<br>
libcamera/gstreamer logs, it seems to have not broken anything)<br>
<br>
An additional test from your side would be appreciated.<br>
---<br>
 src/gstreamer/gstlibcamerasrc.cpp | 45 ++++++++++++++++++++++---------<br>
 src/gstreamer/gstlibcamerasrc.h   |  6 ++---<br>
 2 files changed, 36 insertions(+), 15 deletions(-)<br>
<br>
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp<br>
index 47d8ff43..a2266310 100644<br>
--- a/src/gstreamer/gstlibcamerasrc.cpp<br>
+++ b/src/gstreamer/gstlibcamerasrc.cpp<br>
@@ -146,7 +146,7 @@ struct _GstLibcameraSrc {<br>
        GstTask *task;<br>
<br>
        gchar *camera_name;<br>
-       controls::AfModeEnum auto_focus_mode = controls::AfModeManual;<br>
+       ControlList *controls;<br>
<br>
        GstLibcameraSrcState *state;<br>
        GstLibcameraAllocator *allocator;<br>
@@ -462,7 +462,9 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,<br>
        GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);<br>
        GLibRecLocker lock(&self->stream_lock);<br>
        GstLibcameraSrcState *state = self->state;<br>
+       const ControlInfoMap &info_map = state->cam_->controls();<br>
        GstFlowReturn flow_ret = GST_FLOW_OK;<br>
+       gboolean supported_ctrl = true;<br>
        gint ret;<br>
<br>
        g_autoptr(GstStructure) element_caps = gst_structure_new_empty("caps");<br>
@@ -579,18 +581,31 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,<br>
                gst_flow_combiner_add_pad(self->flow_combiner, srcpad);<br>
        }<br>
<br>
-       if (self->auto_focus_mode != controls::AfModeManual) {<br>
-               const ControlInfoMap &infoMap = state->cam_->controls();<br>
-               if (infoMap.find(&controls::AfMode) != infoMap.end()) {<br>
-                       state->initControls_.set(controls::AfMode, self->auto_focus_mode);<br>
-               } else {<br>
+       /*<br>
+        * Check the controls set on libcamerasrc is supported by the camera.<br>
+        * This can be easily done by CameraControlValidator but it is an internal<br>
+        * libcamera class. Hence avoid adding a internal header even though<br>
+        * gstreamer links with libcamera-private.<br>
+        */<br>
+       for (auto iter = self->controls->begin(); iter != self->controls->end(); iter++) {<br>
+               if (info_map.find(iter->first) == info_map.end()) {<br>
+                       const ControlIdMap *id_map = self->controls->idMap();<br>
                        GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,<br>
-                                         ("Failed to enable auto focus"),<br>
-                                         ("AfMode not supported by this camera, "<br>
-                                          "please retry with 'auto-focus-mode=AfModeManual'"));<br>
+                                         ("Failed to set %s", id_map->at(iter->first)->name().c_str()),<br>
+                                         ("%s not supported by this camera, ", id_map->at(iter->first)->name().c_str()));<br>
+                       supported_ctrl = false;<br>
+                       break;<br>
                }<br>
        }<br>
<br>
+       if (!supported_ctrl) {<br>
+               gst_task_stop(task);<br>
+               flow_ret = GST_FLOW_NOT_NEGOTIATED;<br>
+               goto done;<br>
+       }<br>
+<br>
+       state->initControls_.merge(*self->controls);<br>
+<br>
        ret = state->cam_->start(&state->initControls_);<br>
        if (ret) {<br>
                GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,<br>
@@ -674,7 +689,8 @@ gst_libcamera_src_set_property(GObject *object, guint prop_id,<br>
                self->camera_name = g_value_dup_string(value);<br>
                break;<br>
        case PROP_AUTO_FOCUS_MODE:<br>
-               self->auto_focus_mode = static_cast<controls::AfModeEnum>(g_value_get_enum(value));<br>
+               self->controls->set(controls::AfMode,<br>
+                                   static_cast<controls::AfModeEnum>(g_value_get_enum(value)));<br>
                break;<br>
        default:<br>
                G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);<br>
@@ -693,9 +709,11 @@ gst_libcamera_src_get_property(GObject *object, guint prop_id, GValue *value,<br>
        case PROP_CAMERA_NAME:<br>
                g_value_set_string(value, self->camera_name);<br>
                break;<br>
-       case PROP_AUTO_FOCUS_MODE:<br>
-               g_value_set_enum(value, static_cast<gint>(self->auto_focus_mode));<br>
+       case PROP_AUTO_FOCUS_MODE: {<br>
+               auto auto_focus_mode = self->controls->get(controls::AfMode).value_or(controls::AfModeManual);<br>
+               g_value_set_enum(value, auto_focus_mode);<br>
                break;<br>
+       }<br>
        default:<br>
                G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);<br>
                break;<br>
@@ -760,6 +778,7 @@ gst_libcamera_src_finalize(GObject *object)<br>
        g_rec_mutex_clear(&self->stream_lock);<br>
        g_clear_object(&self->task);<br>
        g_free(self->camera_name);<br>
+       delete self->controls;<br>
        delete self->state;<br>
<br>
        return klass->finalize(object);<br>
@@ -783,6 +802,8 @@ gst_libcamera_src_init(GstLibcameraSrc *self)<br>
        /* C-style friend. */<br>
        state->src_ = self;<br>
        self->state = state;<br>
+<br>
+       self->controls = new ControlList(controls::controls, nullptr);<br>
 }<br>
<br>
 static GstPad *<br>
diff --git a/src/gstreamer/gstlibcamerasrc.h b/src/gstreamer/gstlibcamerasrc.h<br>
index 0a88ba02..2e6d74cb 100644<br>
--- a/src/gstreamer/gstlibcamerasrc.h<br>
+++ b/src/gstreamer/gstlibcamerasrc.h<br>
@@ -26,17 +26,17 @@ gst_libcamera_auto_focus_get_type()<br>
        static GType type = 0;<br>
        static const GEnumValue values[] = {<br>
                {<br>
-                       static_cast<gint>(libcamera::controls::AfModeManual),<br>
+                       libcamera::controls::AfModeManual,<br>
                        "AfModeManual",<br>
                        "manual-focus",<br>
                },<br>
                {<br>
-                       static_cast<gint>(libcamera::controls::AfModeAuto),<br>
+                       libcamera::controls::AfModeAuto,<br>
                        "AfModeAuto",<br>
                        "automatic-auto-focus",<br>
                },<br>
                {<br>
-                       static_cast<gint>(libcamera::controls::AfModeContinuous),<br>
+                       libcamera::controls::AfModeContinuous,<br>
                        "AfModeContinuous",<br>
                        "continuous-auto-focus",<br>
                },<br>
-- <br>
2.39.1<br>
<br>
</blockquote></div>