[libcamera-devel] [PATCH v6 0/1] Provide multiple colorimetry for libcamerasrc.

[Laurent Pinchart]

Hello Rishikesh,

Because it messes up the order in which people normally read text.
Why is top-posting such a bad thing?
Top-posting.
What is the most annoying thing in e-mail?

(https://en.wikipedia.org/wiki/Posting_style)

Please reply inline, don't copy parts of the e-mail you're replying to
to the top.

On Mon, Sep 19, 2022 at 08:50:37PM +0530, Rishikesh Donadkar wrote:
> Hello Umang,
> > The test number results #2 and #3 have same initial colorimetry members
> > (sRGB, bt601,...) , so should be parsed in the same way, how come one
> > fails at bt601 and other one doesn't at bt601 ?
> 
> bt601  translates to 2:4:5:4 after conversion to colorspace,
> validation and conversion back to colorimetry at the time of exposing
> caps. Gstreamer checks if the colorimetry that we are exposing is
> among the ones present in the colorimetry list, In case#3 Since
> 2:4:5:4 is present in the colorimetry list, negotiation is a success.
> 
> > As mentioned earlier. bt601 doesn't have 1:1 mapping to the kernel
> > colorspace[1], hence we need to look at it separately. I would avoid
> > using that as the same for testing it, until we assess the scope of that
> > work.
> 
> Noted.
> 
> On Mon, Sep 19, 2022 at 11:15 AM Umang Jain wrote:
> >
> > Hi Rishi,
> >
> > Thank you for the test results
> >
> > On 9/19/22 8:27 AM, Rishikesh Donadkar wrote:
> > > GStreamer pipeline supports passing multiple colorimetry as a comma
> > > separated list.
> > >
> > > For Example :
> > >
> > > gst-launch-1.0 libcamerasrc ! "video/x-raw,colorimetry={bt2020,bt709,sRGB}" ! glimagesink
> > >
> > > In this case, if supported by the camera one of bt2020, bt709, sRGB
> > > should be applied.

I understand this as meaning that the first supported colorimetry among
the supplied list will be selected, and negotiation will fail if none of
those colorimetries are supported. Is that correct ? If so ... (see
below)

> > > This series aims to adds multiple colorimetry support to the libcamera
> > > gstreamer element.
> > >
> > > ---
> > > changes from v1 to v2:
> > > - Moved the function colorspace_from_colorimetry() from PATCH 1/2 to
> > >    PATCH 2/2.
> > >
> > > changes from v2 to v3:
> > > - Rebase on top of [v3 PATCH 4/4] gstreamer: Provide colorimetry <> ColorSpace mappings.
> > > - Use the comparator utility function gst_video_colorimetry_is_equal()
> > >    to cut down code.
> > > - Rename dup_stream_cfg to pristine_stream_cfg.
> > > - Log the colorimetry that is selected from the list.
> > > - Change the API for the function gst_libcamera_configure_stream_from_caps()
> > >    to pass pass state->config_ here instead of taking it in a holder
> > >    variable reference.
> > >
> > > changes from v3 to v4:
> > > - Discard the approach form expanding the colorimetry list through caps
> > >    normalization. Instead enumerate the colorimetry list and try out the
> > >    colorimetry.
> > > - Add error checking for invalid colorimetry.
> > >
> > > changes for v4 to v5:
> > > - Use gboolean instead of int. Return true/false.
> > > - Add else if for the mutually exclusive conditions.
> > > - Add final else to trace if anything other than a string or a list is
> > >    passed in the colorimetry field by the user.
> > >
> > > changes from v5 to v6:
> > > - Compare colorspace instead of colorimetry, this will reduce the
> > >    colorimetry<->colorspace conversion.
> > > - Compare the colorspace applied directly with the colorimetry requested
> > >    in the caps.
> > > ---
> > >
> > > ----
> > > Test results(on RPi 4 + OV5647):
> > > gst-launch-1.0 libcamerasrc ! "video/x-raw,colorimetry={bt2020,bt709,sRGB}" ! glimagesink
> > > Selected colorimetry bt709
> > >
> > > gst-launch-1.0 libcamerasrc ! "video/x-raw,colorimetry={sRGB,bt601,bt2020,bt709}" ! glimagesink
> > > Negotiation fails on bt601.

... why does this fail ? The previous test showed that bt709 can work,
shouldn't it be selected here ?

> > >
> > > gst-launch-1.0 libcamerasrc ! "video/x-raw,colorimetry={sRGB,bt601,bt2020,bt709,2:4:5:4}" ! glimagesink
> > > Selected colorimetry bt601.
> >
> > This is confusing, are you sure this isn't a typo?
> >
> > The test number results #2 and #3 have same initial colorimetry members
> > (sRGB, bt601,...) , so should be parsed in the same way, how come one
> > fails at bt601 and other one doesn't at bt601 ?
> >
> > As mentioned earlier. bt601 doesn't have 1:1 mapping to the kernel
> > colorspace[1], hence we need to look at it separately. I would avoid
> > using that as the same for testing it, until we assess the scope of that
> > work.
> >
> > [1]
> > https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/-/merge_requests/724/diffs
> > >
> > > gst-launch-1.0 libcamerasrc ! "video/x-raw,colorimetry={sRGB,bt709,bt2020,bt601}" ! glimagesink
> > > Selected colorimetry bt709
> > > ----
> > >
> > > Rishikesh Donadkar (1):
> > >    gstreamer: Provide multiple colorimetry support for libcamerasrc
> > >
> > >   src/gstreamer/gstlibcamera-utils.cpp | 49 +++++++++++++++++++++++-----
> > >   src/gstreamer/gstlibcamera-utils.h   |  4 ++-
> > >   src/gstreamer/gstlibcamerasrc.cpp    |  2 +-
> > >   3 files changed, 44 insertions(+), 11 deletions(-)
> > >

-- 
Regards,

Laurent Pinchart