[libcamera-devel] [PATCH] gstreamer: Provide interlace-mode as fixated caps

[Kieran Bingham]

Quoting Laurent Pinchart via libcamera-devel (2022-08-30 02:01:46)
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Mon, Aug 29, 2022 at 03:32:51PM +0530, Umang Jain via libcamera-devel wrote:
> > The 'interlace-mode' for libcamerasrc will always be 'progressive'.
> 
> I'm *really* crossing my fingers, hoping that you're right :-)
> 
> > Provide it via fixated caps mechanism [1]
> > 
> > [1] https://gstreamer.freedesktop.org/documentation/plugin-development/advanced/negotiation.html?gi-language=c#fixed-negotiation
> > 
> > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> > Rishi, Can you please check this patch as well? I think it will closely
> > co-relate with the framerate being captured in caps, as fixate
> > negotitation mechanism.
> > ---
> >  src/gstreamer/gstlibcamerasrc.cpp | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> > index 16d70fea..24a2e33e 100644
> > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > @@ -800,11 +800,37 @@ gst_libcamera_src_release_pad(GstElement *element, GstPad *pad)
> >       gst_element_remove_pad(element, pad);
> >  }
> >  
> > +static GstCaps *gst_libcamera_src_src_fixate([[maybe_unused]] GstBaseSrc *bsrc,
> > +                                          GstCaps *caps)
> > +{
> > +     GstStructure *structure;
> 
> This could be a local variable within the loop, up to you.
> 
> > +     GstCaps *fixated_caps;
> > +
> > +     fixated_caps = gst_caps_make_writable(caps);
> > +
> > +     for (guint i = 0; i < gst_caps_get_size(fixated_caps); ++i) {
> > +             structure = gst_caps_get_structure(fixated_caps, i);
> > +             if (gst_structure_has_field(structure, "interlace-mode"))
> > +                     gst_structure_fixate_field_string(structure, "interlace-mode",
> > +                                                       "progressive");
> > +             else
> > +                     gst_structure_set(structure, "interlace-mode", G_TYPE_STRING,
> > +                                       "progressive", NULL);
> > +     }
> > +
> > +     fixated_caps = gst_caps_fixate(fixated_caps);
> > +
> > +     return fixated_caps;
> 
> Just
> 
>         return gst_caps_fixate(fixated_caps);
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> 
> But I'm no expert here.

Aha - I'm excited to see more gstreamer improvements here.

I believe this was reported when trying to connect the v4l2h264enc
testing on RPi.

Perhaps it would be nice to have a before and after test case, but I'm
not currently sure of the whole pipeline that caused issues with
interlace-mode.

This sounds and looks good though.

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

> 
> > +}
> > +
> >  static void
> >  gst_libcamera_src_class_init(GstLibcameraSrcClass *klass)
> >  {
> >       GstElementClass *element_class = GST_ELEMENT_CLASS(klass);
> >       GObjectClass *object_class = G_OBJECT_CLASS(klass);
> > +     GstBaseSrcClass *gstbasesrc_class = (GstBaseSrcClass *)klass;
> > +
> > +     gstbasesrc_class->fixate = gst_libcamera_src_src_fixate;
> >  
> >       object_class->set_property = gst_libcamera_src_set_property;
> >       object_class->get_property = gst_libcamera_src_get_property;
> 
> -- 
> Regards,
> 
> Laurent Pinchart