[libcamera-devel] [PATCH v1 3/5] gstreamer:Install the colorimetry property

[Nicolas Dufresne]

Le mercredi 06 juillet 2022 à 13:28 +0530, Rishikesh Donadkar a écrit :
> > What would be a use case though for forcing a specific colorimetry ? Do the
> > V4L2
> > stack we have supports this ?
> > 
> > > +    
> > > g_object_class_install_property(object_class,PROP_COLORIMETRY,spec2);
> > >  }
> 
> a use case would be to set a colorspace requested by user with something like
> g_object_set(source, "colorimetry", "bt2020", NULL);
> I have tested that the requested colorimetry gets applied to the
> camera if it passes the validation.

I might have miss-explained, caps fields in GStreamer shall never be replaced by
a property. The caps negotiation process should let you interact with element
(per stream) to select a colorimetry. Something like:

  libcamerasrc ! video/x-raw,colorimetry=bt709 ! ...

Would force bt709 or fail. Something like:

  libcamerasrc ! "video/x-raw,colorimetry={bt709,bt601}" ! ...

Should try bt709 or bt601, or fail. And something like:

  libcamerasrc ! "video/x-raw,colorimetry=bt709;video/x-raw" ! ...

Will ensure bt709 is preffered, but accept anything if that is not possible. So
instead of the properly, the caps negotiation needs to be enhance to try and
match requested colorimetry signalled by downstream. With this method, you can
have different colorimetry per stream/pad if this is supported by the ISP.

Nicolas
> 
> 
> 
> On Mon, Jul 4, 2022 at 11:43 PM Nicolas Dufresne
> <nicolas.dufresne at collabora.com> wrote:
> > 
> > Le dimanche 03 juillet 2022 à 13:03 +0530, Rishikesh Donadkar a écrit :
> > > Add the colorimetry field in the _GstLibcameraSrc. Add switch cases in
> > > gst_libcamera_src_set_property() and gst_libcamera_src_get_property() to
> > > access and modify the property.
> > > 
> > > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> > > ---
> > >  src/gstreamer/gstlibcamerasrc.cpp | 18 +++++++++++++++++-
> > >  1 file changed, 17 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp
> > > b/src/gstreamer/gstlibcamerasrc.cpp
> > > index 46fd02d2..120319b3 100644
> > > --- a/src/gstreamer/gstlibcamerasrc.cpp
> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp
> > > @@ -121,6 +121,7 @@ struct _GstLibcameraSrc {
> > >       GstTask *task;
> > > 
> > >       gchar *camera_name;
> > > +     gchar *colorimetry;
> > > 
> > >       GstLibcameraSrcState *state;
> > >       GstLibcameraAllocator *allocator;
> > > @@ -129,7 +130,8 @@ struct _GstLibcameraSrc {
> > > 
> > >  enum {
> > >       PROP_0,
> > > -     PROP_CAMERA_NAME
> > > +     PROP_CAMERA_NAME,
> > > +     PROP_COLORIMETRY,
> > >  };
> > > 
> > >  G_DEFINE_TYPE_WITH_CODE(GstLibcameraSrc, gst_libcamera_src,
> > > GST_TYPE_ELEMENT,
> > > @@ -532,6 +534,10 @@ gst_libcamera_src_set_property(GObject *object, guint
> > > prop_id,
> > >               g_free(self->camera_name);
> > >               self->camera_name = g_value_dup_string(value);
> > >               break;
> > > +     case PROP_COLORIMETRY:
> > > +             g_free(self->colorimetry);
> > > +             self->colorimetry = g_value_dup_string(value);
> > > +             break;
> > >       default:
> > >               G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > >               break;
> > > @@ -549,6 +555,9 @@ gst_libcamera_src_get_property(GObject *object, guint
> > > prop_id, GValue *value,
> > >       case PROP_CAMERA_NAME:
> > >               g_value_set_string(value, self->camera_name);
> > >               break;
> > > +     case PROP_COLORIMETRY:
> > > +             g_value_set_string(value, self->colorimetry);
> > > +             break;
> > >       default:
> > >               G_OBJECT_WARN_INVALID_PROPERTY_ID(object, prop_id, pspec);
> > >               break;
> > > @@ -717,4 +726,11 @@ gst_libcamera_src_class_init(GstLibcameraSrcClass
> > > *klass)
> > >                                                            |
> > > G_PARAM_READWRITE
> > >                                                            |
> > > G_PARAM_STATIC_STRINGS));
> > >       g_object_class_install_property(object_class, PROP_CAMERA_NAME,
> > > spec);
> > > +
> > > +     GParamSpec *spec2 = g_param_spec_string("colorimetry",
> > > "Coloirmetry",
> > > +                                             "Colorimetry that will be
> > > applied to the StreamConfiguration", nullptr,
> > > +                                           
> > > (GParamFlags)(GST_PARAM_MUTABLE_READY
> > > +                                                          |
> > > G_PARAM_READWRITE
> > > +                                                          |
> > > G_PARAM_STATIC_STRINGS));
> > 
> > As this is a field in caps, the appropriate way to interact in userland is
> > to
> > place such feild in downstream caps filter. Properties are not acceptable.
> > 
> >   libcamerasrc ! video/x-raw,colorimetry=bt709 ! ...
> > 
> > What would be a use case though for forcing a specific colorimetry ? Do the
> > V4L2
> > stack we have supports this ?
> > 
> > > +    
> > > g_object_class_install_property(object_class,PROP_COLORIMETRY,spec2);
> > >  }
> >