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

[Rishikesh Donadkar]

Hello Laurent,

On Mon, Sep 19, 2022 at 10:49 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> 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.
>
Got it.
>
> 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 ?

This happens as a result of the bt601 colorimetry. As stated by Umang
in the initial patch email's reply, bt601 doesn't have 1:1 mapping to
the kernel colorspace. I now realise that the colorimetry bt601 should
not have been a part of the tests. bt709 would have been chosen if
bt601 had not been on the list.
>
>
> > > >
> > > > 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,

Rishikesh Donadkar