<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>