[PATCH 1/3] gstreamer: Remove auto-focus-mode property

[Jaslo Ziska]

Hi Nicolas and Laurent,

Nicolas Dufresne <nicolas at ndufresne.ca> writes:
> Hi Laurent,
>
> Le mercredi 07 août 2024 à 23:13 +0300, Laurent Pinchart a 
> écrit :
>> Hi Jaslo,
>> 
>> Thank you for the patch.
>> 
>> On Mon, Aug 05, 2024 at 11:28:36AM +0200, Jaslo Ziska wrote:
>> > In preparation of the next commit remove the auto-focus-mode 
>> > property
>> > from the libcamera element.
>> 
>> I would write
>> 
>>     In preparation for generic support of all libcaemra 
>>     controls, remove the
>>     manual handling of the auto-focus-mode property from the 
>>     libcamerasrc
>>     element.
>> 
>> Could you explain here why you're not also removing the manual
>> auto-focus handling from src/gstreamer/gstlibcameraprovider.cpp 
>> ?

Oops, I didn't even know that was there.

> That escaped my review, and should have never been introduce in 
> the first place.
> Can be removed independently from this patchset. There should be 
> no property
> proxying between the provider and the generated elements.

If you want to I can also add a patch removing this to the set (or 
add it to the existing patch removing auto-focus-mode support).

Best regards,

Jaslo

>
> Nicolas
>
>> 
>> > Signed-off-by: Jaslo Ziska <jaslo at ziska.de>
>> > ---
>> >  src/gstreamer/gstlibcamerasrc.cpp | 28 
>> >  ----------------------------
>> >  1 file changed, 28 deletions(-)
>> > 
>> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp 
>> > b/src/gstreamer/gstlibcamerasrc.cpp
>> > index e1bb6b4c..5a3e2989 100644
>> > --- a/src/gstreamer/gstlibcamerasrc.cpp
>> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
>> > @@ -142,7 +142,6 @@ struct _GstLibcameraSrc {
>> >  	GstTask *task;
>> >  
>> >  	gchar *camera_name;
>> > -	controls::AfModeEnum auto_focus_mode = 
>> > controls::AfModeManual;
>> >  
>> >  	std::atomic<GstEvent *> pending_eos;
>> >  
>> > @@ -154,7 +153,6 @@ struct _GstLibcameraSrc {
>> >  enum {
>> >  	PROP_0,
>> >  	PROP_CAMERA_NAME,
>> > -	PROP_AUTO_FOCUS_MODE,
>> >  };
>> >  
>> >  static void gst_libcamera_src_child_proxy_init(gpointer 
>> >  g_iface,
>> > @@ -663,18 +661,6 @@ gst_libcamera_src_task_enter(GstTask 
>> > *task, [[maybe_unused]] GThread *thread,
>> >  		gst_pad_push_event(srcpad, 
>> >  gst_event_new_segment(&segment));
>> >  	}
>> >  
>> > -	if (self->auto_focus_mode != controls::AfModeManual) {
>> > -		const ControlInfoMap &infoMap = 
>> > state->cam_->controls();
>> > -		if (infoMap.find(&controls::AfMode) != infoMap.end()) 
>> > {
>> > -			state->initControls_.set(controls::AfMode, 
>> > self->auto_focus_mode);
>> > -		} else {
>> > -			GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
>> > -					  ("Failed to enable auto focus"),
>> > -					  ("AfMode not supported by this camera, "
>> > -					   "please retry with 
>> > 'auto-focus-mode=AfModeManual'"));
>> > -		}
>> > -	}
>> > -
>> >  	ret = state->cam_->start(&state->initControls_);
>> >  	if (ret) {
>> >  		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
>> > @@ -742,9 +728,6 @@ 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_AUTO_FOCUS_MODE:
>> > -		self->auto_focus_mode = 
>> > static_cast<controls::AfModeEnum>(g_value_get_enum(value));
>> > -		break;
>> >  	default:
>> >  		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, 
>> >  pspec);
>> >  		break;
>> > @@ -762,9 +745,6 @@ 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_AUTO_FOCUS_MODE:
>> > -		g_value_set_enum(value, 
>> > static_cast<gint>(self->auto_focus_mode));
>> > -		break;
>> >  	default:
>> >  		G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, 
>> >  pspec);
>> >  		break;
>> > @@ -967,14 +947,6 @@ 
>> > gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
>> >  							     | G_PARAM_STATIC_STRINGS));
>> >  	g_object_class_install_property(object_class, 
>> >  PROP_CAMERA_NAME, spec);
>> >  
>> > -	spec = g_param_spec_enum("auto-focus-mode",
>> > -				 "Set auto-focus mode",
>> > -				 "Available options: AfModeManual, "
>> > -				 "AfModeAuto or AfModeContinuous.",
>> > -				 gst_libcamera_auto_focus_get_type(),
>> > -				 static_cast<gint>(controls::AfModeManual),
>> > -				 G_PARAM_WRITABLE);
>> > -	g_object_class_install_property(object_class, 
>> > PROP_AUTO_FOCUS_MODE, spec);
>> >  }
>> >  
>> >  /* GstChildProxy implementation */
>>