[libcamera-devel] [PATCH v3 1/1] gstreamer: Provide mulitple colorimetry support

[Rishikesh Donadkar]

> > +     if (gst_structure_has_field(s, "colorimetry")) {
> > +             const gchar *colorimetry_str = gst_structure_get_string(s, "colorimetry");
> > +             GstVideoColorimetry colorimetry;
> > +
> > +             if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))
> > +                     g_critical("Invalid colorimetry %s", colorimetry_str);
>
>
> If the parsing failed, the colorimetry structure will be left uninitialized.
> Perhaps you should just early return if the caps string is invalid ?
>
Noted.
>
> > -     /* Configure colorimetry */
> > -     if (gst_structure_has_field(s, "colorimetry")) {
> > -             const gchar *colorimetry_str = gst_structure_get_string(s, "colorimetry");
> > -             GstVideoColorimetry colorimetry;
> > -
> > -             if (!gst_video_colorimetry_from_string(&colorimetry, colorimetry_str))
> > -                     g_critical("Invalid colorimetry %s", colorimetry_str);
> > -
> > -             stream_cfg.colorSpace = colorspace_from_colorimetry(colorimetry);
> > -             /* Check if colorimetry had any identifiers which did not map */
> > -             if (colorimetry.primaries != GST_VIDEO_COLOR_PRIMARIES_UNKNOWN &&
> > -                 stream_cfg.colorSpace == ColorSpace::Raw) {
> > -                     GST_ERROR("One or more identifiers could not be mapped for %s colorimetry",
> > -                               colorimetry_str);
> > -                     stream_cfg.colorSpace = std::nullopt;
> > +     /* Create new caps, copy the structure with best resolutions
> > +      * and normalize the caps.
> > +      */
> > +     GstCaps *ncaps = gst_caps_copy_nth(caps, colorimetry_index);
> > +     ncaps = gst_caps_normalize(ncaps);
>
> I'm worried that original caps may have width/height ranges which will also get
> expended, leading to a really large number of caps. Any reason not to enumerate
> the colorimetry field instead instead ?

 Yes, expanding height/width range would create a lot of structures.
The first idea I thought of was to enumerate the colorimetry list
itself, but I don't know how ;-;
This is the reason I had to go with the approach of creating duplicate
caps and normalizing it.
If there is a way to enumerate the colorimetry list in your knowledge
please let me know, that would be helpful.

Regards,

Rishikesh Donadkar