[libcamera-devel] [PATCH v2 0/3] gstreamer: Implement renegotiation

[Nicolas Dufresne]

Hi Jaslo,

Le jeudi 30 novembre 2023 à 16:43 +0100, Jaslo Ziska via libcamera-devel a
écrit :
> Hi,
> 
> in this revision I mostly implemented Nicolas' suggestions namely
> writing a comment on the locking behaviour for
> gst_libcamera_src_negotiate and using the correct types and putting
> the addition of the clearRequests function into a seperate commit.
> Also the gst_libcamera_src_negotiate function now clears the
> reconfigure flag and thus not all pads need to be iterated when
> checking for this flag in the running task.
> 
> Another thing I changed is that in processRequest in the
> GST_FLOW_NOT_NEGOTIATED case it is now checked whether any pad
> actually requested a renegotiation and if not an error is returned.

Very nice work. For the entire series:

Tested-by: Nicolas Dufresne <nicolas.dufresne at collabora.com>

The test I have conducted is fairly minimal. I have tested the simple pipeline
with UVC camera, and a single pad for now. For this purpose, I have patched
GStreamer v4l2src test software:


diff --git a/subprojects/gst-plugins-good/tests/examples/v4l2/v4l2src-
renegotiate.c b/subprojects/gst-plugins-good/tests/examples/v4l2/v4l2src-
renegotiate.c
index cae28d2d66..d31fe08a1f 100644
--- a/subprojects/gst-plugins-good/tests/examples/v4l2/v4l2src-renegotiate.c
+++ b/subprojects/gst-plugins-good/tests/examples/v4l2/v4l2src-renegotiate.c
@@ -136,8 +136,8 @@ main (gint argc, gchar ** argv)
 
   loop = g_main_loop_new (NULL, FALSE);
 
-  desc = g_strdup_printf ("v4l2src name=src device=\"%s\" io-mode=\"%s\" "
-      "! capsfilter name=cf ! %s", device, io_mode, videosink);
+  desc = g_strdup_printf ("libcamerasrc name=src "
+      "! capsfilter name=cf ! %s", videosink);
   pipeline = gst_parse_launch (desc, &error);
   g_free (desc);
   if (!pipeline) {


With good visual output and serial showing:

$> v4l2src-renegotiate -s glimagesink
Setting resolution to '320x240'
[122:15:30.282196360] [691281]  INFO IPAManager ipa_manager.cpp:143 libcamera is
not installed. Adding '/home/nicolas/Sources/libcamera/build/src/ipa' to the IPA
search path
[122:15:30.282921702] [691281]  INFO Camera camera_manager.cpp:284 libcamera
v0.1.0+130-be11e882-dirty (2023-12-04T16:20:46-05:00)
[122:15:30.803772071] [691293]  INFO Camera camera.cpp:1183 configuring streams:
(0) 320x240-YUYV
Setting resolution to '1280x720'
[122:15:34.232874025] [691293]  INFO Camera camera.cpp:1183 configuring streams:
(0) 1280x720-YUYV
Setting resolution to '640x480'
[122:15:37.238145227] [691293]  INFO Camera camera.cpp:1183 configuring streams:
(0) 640x480-YUYV
[122:15:37.920471727] [691292]  INFO V4L2 v4l2_videodevice.cpp:1820
/dev/video2[20:cap]: Zero sequence expected for first frame (got 2)


We probably want a similar tool or test in libcamera itself though. It just a
little difficult, since it so tied to the capabilities of the underneath
hardware. If anyone have suggestions, this would be welcome, I would probably
not block this meanwhile. If C is not a problem to folks here, I'd be happy to
just copy over that modified example. Or I suppose its simple enough that it
could be ported to C++.

regards,
Nicolas

> 
> Regards,
> 
> Jaslo
> 
> Jaslo Ziska (3):
>   gstreamer: Move negotiation logic to separate function
>   gstreamer: Add GstLibcameraSrcState::clearRequests method
>   gstreamer: Implement renegotiation
> 
>  src/gstreamer/gstlibcamerasrc.cpp | 263 ++++++++++++++++++------------
>  1 file changed, 158 insertions(+), 105 deletions(-)
> 
> --
> 2.43.0