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

[Umang Jain]

Hi Kieran,

On 8/31/22 6:44 PM, Kieran Bingham wrote:
> 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.

I've asked Rishi to test the patch on RPi.

Rishikesh, do you have any update? As far as I can remember the 
conversation was around:

https://bugs.libcamera.org/show_bug.cgi?id=75#c8

With this patch, one doesn't need to manually specify 'interlace-mode' 
to make the pipeline work
>
> 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