[libcamera-devel] [PATCH v5 1/2] gstreamer: Configure the camera before exposing the caps.

[Umang Jain]

Hi Rishi,

Thank you for the patch.

On 9/15/22 5:17 PM, Rishikesh Donadkar wrote:
> Configure the camera before exposing the caps so that the stream configuration
> and valid controls values (and bounds) are available, required to expose the new caps.
> controls::FrameDurationLimits is one such example.
>
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>

I would repharse the commit message to expand a bit on why we need to do 
it before caps are exposed.

Other than, code wise looks good to me and I've also tested for no 
regressions so,

Tested-by: Umang Jain <umang.jain at ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>

> ---
>   src/gstreamer/gstlibcamerasrc.cpp | 19 ++++++++++---------
>   1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 16d70fea..60032236 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -515,6 +515,16 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
>   		goto done;
>   	}
>   
> +	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);
> +		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.
> @@ -535,15 +545,6 @@ gst_libcamera_src_task_enter(GstTask *task, [[maybe_unused]] GThread *thread,
>   		gst_pad_push_event(srcpad, gst_event_new_segment(&segment));
>   	}
>   
> -	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;
> -	}
> -
>   	self->allocator = gst_libcamera_allocator_new(state->cam_, state->config_.get());
>   	if (!self->allocator) {
>   		GST_ELEMENT_ERROR(self, RESOURCE, NO_SPACE_LEFT,