[libcamera-devel] [PATCH v3 15/27] gst: libcamerasrc: Implement minimal caps negotiation

[Laurent Pinchart]

Hi Nicolas,

Thank you for the patch.

On Fri, Mar 06, 2020 at 03:26:25PM -0500, Nicolas Dufresne wrote:
> From: Nicolas Dufresne <nicolas.dufresne at collabora.com>
> 
> This is not expected to work in every possible cases, but should be sufficient as
> an initial implementation. What it does is that it turns the StreamFormats into
> caps and queries downstream caps with that as a filter.
> 
> The result is the subset of caps that can be used. We then keep the first
> structure in that result and fixate using the default values found in
> StreamConfiguration as a default in case a range is available.
> 
> We then validate this configuration and turn the potentially modified
> configuration into caps that we push downstream. Note that we trust the order
> in StreamFormats as being sorted best first, but this is not currently in
> libcamera. A todo has been added in the head of this file as a reminder to fix
> that in the core.
> 
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  src/gstreamer/gstlibcamerasrc.cpp | 81 +++++++++++++++++++++++++++++++
>  1 file changed, 81 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 1680314..95af141 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -6,6 +6,12 @@
>   * gstlibcamerasrc.cpp - GStreamer Capture Element
>   */
>  
> +/**
> + * \todo libcamera UVC drivers picks the lowest possible resolution first, this
> + * should be fixed so that we get a decent resolution and framerate for the
> + * role by default.
> + */
> +
>  #include "gstlibcamerasrc.h"
>  
>  #include <vector>
> @@ -25,6 +31,7 @@ GST_DEBUG_CATEGORY_STATIC(source_debug);
>  struct GstLibcameraSrcState {
>  	std::unique_ptr<CameraManager> cm_;
>  	std::shared_ptr<Camera> cam_;
> +	std::unique_ptr<CameraConfiguration> config_;
>  	std::vector<GstPad *> srcpads_;
>  };
>  
> @@ -133,16 +140,90 @@ gst_libcamera_src_task_enter(GstTask *task, GThread *thread, gpointer user_data)
>  	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
>  	GLibRecLocker lock(&self->stream_lock);
>  	GstLibcameraSrcState *state = self->state;
> +	GstFlowReturn flow_ret = GST_FLOW_OK;
> +	gint ret;
>  
>  	GST_DEBUG_OBJECT(self, "Streaming thread has started");
>  
>  	guint group_id = gst_util_group_id_next();
> +	StreamRoles roles;
>  	for (GstPad *srcpad : state->srcpads_) {
>  		/* Create stream-id and push stream-start. */
>  		g_autofree gchar *stream_id = gst_pad_create_stream_id(srcpad, GST_ELEMENT(self), nullptr);
>  		GstEvent *event = gst_event_new_stream_start(stream_id);
>  		gst_event_set_group_id(event, group_id);
>  		gst_pad_push_event(srcpad, event);
> +
> +		/* Collect the streams roles for the next iteration. */
> +		roles.push_back(gst_libcamera_pad_get_role(srcpad));
> +	}
> +
> +	/* Generate the stream configurations, there should be one per pad. */
> +	state->config_ = state->cam_->generateConfiguration(roles);
> +	/*
> +	 * \todo Check if camera may increase or decrease the number of streams
> +	 * regardless of the number of roles.
> +	 */
> +	g_assert(state->config_->size() == state->srcpads_.size());
> +
> +	for (gsize i = 0; i < state->srcpads_.size(); i++) {
> +		GstPad *srcpad = state->srcpads_[i];
> +		StreamConfiguration &stream_cfg = state->config_->at(i);
> +
> +		/* Retrieve the supported caps. */
> +		g_autoptr(GstCaps) filter = gst_libcamera_stream_formats_to_caps(stream_cfg.formats());
> +		g_autoptr(GstCaps) caps = gst_pad_peer_query_caps(srcpad, filter);
> +		if (gst_caps_is_empty(caps)) {
> +			flow_ret = GST_FLOW_NOT_NEGOTIATED;
> +			break;
> +		}
> +
> +		/* Fixate caps and configure the stream. */
> +		caps = gst_caps_make_writable(caps);
> +		gst_libcamera_configure_stream_from_caps(stream_cfg, caps);
> +	}
> +
> +	if (flow_ret != GST_FLOW_OK)
> +		goto done;
> +
> +	/* Validate the configuration. */
> +	if (state->config_->validate() == CameraConfiguration::Invalid) {
> +		flow_ret = GST_FLOW_NOT_NEGOTIATED;
> +		goto done;
> +	}
> +
> +	/*
> +	 * Regardless if it has been modified, create clean caps and push the
> +	 * caps event. Downstream will decide if the caps are acceptable.
> +	 */
> +	for (gsize i = 0; i < state->srcpads_.size(); i++) {
> +		GstPad *srcpad = state->srcpads_[i];
> +		const StreamConfiguration &stream_cfg = state->config_->at(i);
> +
> +		g_autoptr(GstCaps) caps = gst_libcamera_stream_configuration_to_caps(stream_cfg);
> +		if (!gst_pad_push_event(srcpad, gst_event_new_caps(caps))) {
> +			flow_ret = GST_FLOW_NOT_NEGOTIATED;
> +			break;
> +		}
> +	}
> +
> +	ret = state->cam_->configure(state->config_.get());
> +	if (ret) {
> +		GST_ELEMENT_ERROR(self, RESOURCE, SETTINGS,
> +				  ("Failed to configure camera: %s", g_strerror(-ret)),
> +				  ("Camera::configure() failed with error code %i", ret));
> +		gst_task_stop(task);
> +		return;
> +	}
> +
> +done:
> +	switch (flow_ret) {
> +	case GST_FLOW_NOT_NEGOTIATED:
> +		GST_ELEMENT_FLOW_ERROR(self, flow_ret);
> +		gst_task_stop(task);
> +		break;
> +	default:
> +		break;
>  	}
>  }
>  

-- 
Regards,

Laurent Pinchart