[libcamera-devel,v4,7/7] gstreamer: Provide colorimetry <> ColorSpace mappings
diff mbox series

Message ID 20220830074725.1059643-8-umang.jain@ideasonboard.com
State Accepted
Delegated to: Umang Jain
Headers show
Series
  • Colospace adjustment and gstreamer mapping
Related show

Commit Message

Umang Jain Aug. 30, 2022, 7:47 a.m. UTC
From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>

Provide colorimetry <=> libcamera::ColorSpace mappings via:
- GstVideoColorimetry colorimetry_from_colorspace(colorspace);
- ColorSpace colorspace_from_colorimetry(colorimetry);

Read the colorimetry field from caps into the stream configuration.
After stream validation, the sensor supported colorimetry will
be retrieved and the caps will be updated accordingly.

Colorimetry support for gstlibcamera currently undertakes only one
argument. Multiple colorimetry support shall be introduced in
subsequent commits.

Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
---
 src/gstreamer/gstlibcamera-utils.cpp | 169 +++++++++++++++++++++++++++
 1 file changed, 169 insertions(+)

Comments

Nicolas Dufresne via libcamera-devel Sept. 1, 2022, 4:21 a.m. UTC | #1
On Tue, Aug 30, 2022 at 01:17:25PM +0530, Umang Jain via libcamera-devel wrote:
> From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
> 
> Provide colorimetry <=> libcamera::ColorSpace mappings via:
> - GstVideoColorimetry colorimetry_from_colorspace(colorspace);
> - ColorSpace colorspace_from_colorimetry(colorimetry);
> 
> Read the colorimetry field from caps into the stream configuration.
> After stream validation, the sensor supported colorimetry will
> be retrieved and the caps will be updated accordingly.
> 
> Colorimetry support for gstlibcamera currently undertakes only one
> argument. Multiple colorimetry support shall be introduced in
> subsequent commits.
> 
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 169 +++++++++++++++++++++++++++
>  1 file changed, 169 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 5a21a391..c7b44a8e 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -57,6 +57,152 @@ static struct {
>  	/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */
>  };
>  
> +static GstVideoColorimetry
> +colorimetry_from_colorspace(const ColorSpace &colorSpace)
> +{
> +	GstVideoColorimetry colorimetry;
> +
> +	switch (colorSpace.primaries) {
> +	case ColorSpace::Primaries::Raw:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
> +		break;
> +	case ColorSpace::Primaries::Smpte170m:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
> +		break;
> +	case ColorSpace::Primaries::Rec709:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
> +		break;
> +	case ColorSpace::Primaries::Rec2020:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
> +		break;
> +	}
> +
> +	switch (colorSpace.transferFunction) {
> +	case ColorSpace::TransferFunction::Linear:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
> +		break;
> +	case ColorSpace::TransferFunction::Srgb:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
> +		break;
> +	case ColorSpace::TransferFunction::Rec709:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
> +		break;
> +	}
> +
> +	switch (colorSpace.ycbcrEncoding) {
> +	case ColorSpace::YcbcrEncoding::None:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec601:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec709:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec2020:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
> +		break;
> +	}
> +
> +	switch (colorSpace.range) {
> +	case ColorSpace::Range::Full:
> +		colorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;
> +		break;
> +	case ColorSpace::Range::Limited:
> +		colorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;
> +		break;
> +	}
> +
> +	return colorimetry;
> +}
> +
> +static std::optional<ColorSpace>
> +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)
> +{
> +	std::optional<ColorSpace> colorspace;
> +
> +	switch (colorimetry.primaries) {
> +	case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
> +		/* Unknown primaries map to raw colorspace in gstreamer */
> +		return ColorSpace::Raw;
> +	case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
> +		colorspace->primaries = ColorSpace::Primaries::Smpte170m;
> +		break;
> +	case GST_VIDEO_COLOR_PRIMARIES_BT709:
> +		colorspace->primaries = ColorSpace::Primaries::Rec709;
> +		break;
> +	case GST_VIDEO_COLOR_PRIMARIES_BT2020:
> +		colorspace->primaries = ColorSpace::Primaries::Rec2020;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry primaries %d not mapped in gstlibcamera",
> +			    colorimetry.primaries);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.transfer) {
> +	/* Transfer function mappings inspired from v4l2src plugin */
> +	case GST_VIDEO_TRANSFER_GAMMA18:
> +	case GST_VIDEO_TRANSFER_GAMMA20:
> +	case GST_VIDEO_TRANSFER_GAMMA22:
> +	case GST_VIDEO_TRANSFER_GAMMA28:
> +		GST_WARNING("GAMMA 18, 20, 22, 28 transfer functions not supported");
> +	/* fallthrough */
> +	case GST_VIDEO_TRANSFER_GAMMA10:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Linear;
> +		break;
> +	case GST_VIDEO_TRANSFER_SRGB:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Srgb;
> +		break;
> +	case GST_VIDEO_TRANSFER_BT601:
> +	case GST_VIDEO_TRANSFER_BT2020_12:
> +	case GST_VIDEO_TRANSFER_BT2020_10:
> +	case GST_VIDEO_TRANSFER_BT709:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Rec709;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry transfer function %d not mapped in gstlibcamera",
> +			    colorimetry.transfer);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.matrix) {
> +	case GST_VIDEO_COLOR_MATRIX_RGB:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
> +		break;
> +	/* FCC is about the same as BT601 with less digit */
> +	case GST_VIDEO_COLOR_MATRIX_FCC:
> +	case GST_VIDEO_COLOR_MATRIX_BT601:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;
> +		break;
> +	case GST_VIDEO_COLOR_MATRIX_BT709:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;
> +		break;
> +	case GST_VIDEO_COLOR_MATRIX_BT2020:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry matrix %d not mapped in gstlibcamera",
> +			    colorimetry.matrix);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.range) {
> +	case GST_VIDEO_COLOR_RANGE_0_255:
> +		colorspace->range = ColorSpace::Range::Full;
> +		break;
> +	case GST_VIDEO_COLOR_RANGE_16_235:
> +		colorspace->range = ColorSpace::Range::Limited;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry range %d not mapped in gstlibcamera",
> +			    colorimetry.range);
> +		return std::nullopt;
> +	}
> +
> +	return colorspace;
> +}
> +
>  static GstVideoFormat
>  pixel_format_to_gst_format(const PixelFormat &format)
>  {
> @@ -151,6 +297,18 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg
>  			  "width", G_TYPE_INT, stream_cfg.size.width,
>  			  "height", G_TYPE_INT, stream_cfg.size.height,
>  			  nullptr);
> +
> +	if (stream_cfg.colorSpace) {
> +		GstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());
> +		gchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> +
> +		if (colorimetry_str)
> +			gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
> +		else
> +			g_error("Got invalid colorimetry from ColorSpace: %s",
> +				ColorSpace::toString(stream_cfg.colorSpace).c_str());
> +	}
> +
>  	gst_caps_append_structure(caps, s);
>  
>  	return caps;
> @@ -234,6 +392,17 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>  	gst_structure_get_int(s, "height", &height);
>  	stream_cfg.size.width = width;
>  	stream_cfg.size.height = height;
> +
> +	/* 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);
> +	}
>  }
>  
>  #if !GST_CHECK_VERSION(1, 17, 1)
> -- 
> 2.37.2
>
Umang Jain Sept. 1, 2022, 10:49 a.m. UTC | #2
Hi

On 8/30/22 1:17 PM, Umang Jain wrote:
> From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
>
> Provide colorimetry <=> libcamera::ColorSpace mappings via:
> - GstVideoColorimetry colorimetry_from_colorspace(colorspace);
> - ColorSpace colorspace_from_colorimetry(colorimetry);
>
> Read the colorimetry field from caps into the stream configuration.
> After stream validation, the sensor supported colorimetry will
> be retrieved and the caps will be updated accordingly.
>
> Colorimetry support for gstlibcamera currently undertakes only one
> argument. Multiple colorimetry support shall be introduced in
> subsequent commits.
>
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   src/gstreamer/gstlibcamera-utils.cpp | 169 +++++++++++++++++++++++++++
>   1 file changed, 169 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 5a21a391..c7b44a8e 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -57,6 +57,152 @@ static struct {
>   	/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */
>   };
>   
> +static GstVideoColorimetry
> +colorimetry_from_colorspace(const ColorSpace &colorSpace)
> +{
> +	GstVideoColorimetry colorimetry;
> +
> +	switch (colorSpace.primaries) {
> +	case ColorSpace::Primaries::Raw:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
> +		break;
> +	case ColorSpace::Primaries::Smpte170m:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
> +		break;
> +	case ColorSpace::Primaries::Rec709:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
> +		break;
> +	case ColorSpace::Primaries::Rec2020:
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
> +		break;
> +	}
> +
> +	switch (colorSpace.transferFunction) {
> +	case ColorSpace::TransferFunction::Linear:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
> +		break;
> +	case ColorSpace::TransferFunction::Srgb:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
> +		break;
> +	case ColorSpace::TransferFunction::Rec709:
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
> +		break;
> +	}
> +
> +	switch (colorSpace.ycbcrEncoding) {
> +	case ColorSpace::YcbcrEncoding::None:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec601:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec709:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
> +		break;
> +	case ColorSpace::YcbcrEncoding::Rec2020:
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
> +		break;
> +	}
> +
> +	switch (colorSpace.range) {
> +	case ColorSpace::Range::Full:
> +		colorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;
> +		break;
> +	case ColorSpace::Range::Limited:
> +		colorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;
> +		break;
> +	}
> +
> +	return colorimetry;
> +}
> +
> +static std::optional<ColorSpace>
> +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)
> +{
> +	std::optional<ColorSpace> colorspace;

I started a rigourous testing on RPi today comparing cam's colorspace 
and gstreamer's colorspace mapping/adjustment/logs stuff and happen to 
catch this last-min bug here.

The optional colorspace value needs to be assigned (or initialised with 
a value here). Otherwise the return value from this function is always 
std::nullopt (since we never assign a value hence the optional ptr is 
acting as non-containing value)...

For now, I have:

     -       std::optional<ColorSpace> colorspace;
     +       std::optional<ColorSpace> colorspace = ColorSpace::Raw;

Any objections for this Laurent, Rishi ?
> +
> +	switch (colorimetry.primaries) {
> +	case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
> +		/* Unknown primaries map to raw colorspace in gstreamer */
> +		return ColorSpace::Raw;
> +	case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
> +		colorspace->primaries = ColorSpace::Primaries::Smpte170m;
> +		break;
> +	case GST_VIDEO_COLOR_PRIMARIES_BT709:
> +		colorspace->primaries = ColorSpace::Primaries::Rec709;
> +		break;
> +	case GST_VIDEO_COLOR_PRIMARIES_BT2020:
> +		colorspace->primaries = ColorSpace::Primaries::Rec2020;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry primaries %d not mapped in gstlibcamera",
> +			    colorimetry.primaries);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.transfer) {
> +	/* Transfer function mappings inspired from v4l2src plugin */
> +	case GST_VIDEO_TRANSFER_GAMMA18:
> +	case GST_VIDEO_TRANSFER_GAMMA20:
> +	case GST_VIDEO_TRANSFER_GAMMA22:
> +	case GST_VIDEO_TRANSFER_GAMMA28:
> +		GST_WARNING("GAMMA 18, 20, 22, 28 transfer functions not supported");
> +	/* fallthrough */
> +	case GST_VIDEO_TRANSFER_GAMMA10:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Linear;
> +		break;
> +	case GST_VIDEO_TRANSFER_SRGB:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Srgb;
> +		break;
> +	case GST_VIDEO_TRANSFER_BT601:
> +	case GST_VIDEO_TRANSFER_BT2020_12:
> +	case GST_VIDEO_TRANSFER_BT2020_10:
> +	case GST_VIDEO_TRANSFER_BT709:
> +		colorspace->transferFunction = ColorSpace::TransferFunction::Rec709;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry transfer function %d not mapped in gstlibcamera",
> +			    colorimetry.transfer);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.matrix) {
> +	case GST_VIDEO_COLOR_MATRIX_RGB:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
> +		break;
> +	/* FCC is about the same as BT601 with less digit */
> +	case GST_VIDEO_COLOR_MATRIX_FCC:
> +	case GST_VIDEO_COLOR_MATRIX_BT601:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;
> +		break;
> +	case GST_VIDEO_COLOR_MATRIX_BT709:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;
> +		break;
> +	case GST_VIDEO_COLOR_MATRIX_BT2020:
> +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry matrix %d not mapped in gstlibcamera",
> +			    colorimetry.matrix);
> +		return std::nullopt;
> +	}
> +
> +	switch (colorimetry.range) {
> +	case GST_VIDEO_COLOR_RANGE_0_255:
> +		colorspace->range = ColorSpace::Range::Full;
> +		break;
> +	case GST_VIDEO_COLOR_RANGE_16_235:
> +		colorspace->range = ColorSpace::Range::Limited;
> +		break;
> +	default:
> +		GST_WARNING("Colorimetry range %d not mapped in gstlibcamera",
> +			    colorimetry.range);
> +		return std::nullopt;
> +	}
> +
> +	return colorspace;
> +}
> +
>   static GstVideoFormat
>   pixel_format_to_gst_format(const PixelFormat &format)
>   {
> @@ -151,6 +297,18 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg
>   			  "width", G_TYPE_INT, stream_cfg.size.width,
>   			  "height", G_TYPE_INT, stream_cfg.size.height,
>   			  nullptr);
> +
> +	if (stream_cfg.colorSpace) {
> +		GstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());
> +		gchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> +
> +		if (colorimetry_str)
> +			gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
> +		else
> +			g_error("Got invalid colorimetry from ColorSpace: %s",
> +				ColorSpace::toString(stream_cfg.colorSpace).c_str());
> +	}
> +
>   	gst_caps_append_structure(caps, s);
>   
>   	return caps;
> @@ -234,6 +392,17 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
>   	gst_structure_get_int(s, "height", &height);
>   	stream_cfg.size.width = width;
>   	stream_cfg.size.height = height;
> +
> +	/* 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);
> +	}
>   }
>   
>   #if !GST_CHECK_VERSION(1, 17, 1)
Laurent Pinchart Sept. 1, 2022, 10:58 a.m. UTC | #3
Hi Umang,

On Thu, Sep 01, 2022 at 04:19:30PM +0530, Umang Jain wrote:
> On 8/30/22 1:17 PM, Umang Jain wrote:
> > From: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
> >
> > Provide colorimetry <=> libcamera::ColorSpace mappings via:
> > - GstVideoColorimetry colorimetry_from_colorspace(colorspace);
> > - ColorSpace colorspace_from_colorimetry(colorimetry);
> >
> > Read the colorimetry field from caps into the stream configuration.
> > After stream validation, the sensor supported colorimetry will
> > be retrieved and the caps will be updated accordingly.
> >
> > Colorimetry support for gstlibcamera currently undertakes only one
> > argument. Multiple colorimetry support shall be introduced in
> > subsequent commits.
> >
> > Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar@gmail.com>
> > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
> > ---
> >   src/gstreamer/gstlibcamera-utils.cpp | 169 +++++++++++++++++++++++++++
> >   1 file changed, 169 insertions(+)
> >
> > diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> > index 5a21a391..c7b44a8e 100644
> > --- a/src/gstreamer/gstlibcamera-utils.cpp
> > +++ b/src/gstreamer/gstlibcamera-utils.cpp
> > @@ -57,6 +57,152 @@ static struct {
> >   	/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */
> >   };
> >   
> > +static GstVideoColorimetry
> > +colorimetry_from_colorspace(const ColorSpace &colorSpace)
> > +{
> > +	GstVideoColorimetry colorimetry;
> > +
> > +	switch (colorSpace.primaries) {
> > +	case ColorSpace::Primaries::Raw:
> > +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
> > +		break;
> > +	case ColorSpace::Primaries::Smpte170m:
> > +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
> > +		break;
> > +	case ColorSpace::Primaries::Rec709:
> > +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
> > +		break;
> > +	case ColorSpace::Primaries::Rec2020:
> > +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
> > +		break;
> > +	}
> > +
> > +	switch (colorSpace.transferFunction) {
> > +	case ColorSpace::TransferFunction::Linear:
> > +		colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
> > +		break;
> > +	case ColorSpace::TransferFunction::Srgb:
> > +		colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
> > +		break;
> > +	case ColorSpace::TransferFunction::Rec709:
> > +		colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
> > +		break;
> > +	}
> > +
> > +	switch (colorSpace.ycbcrEncoding) {
> > +	case ColorSpace::YcbcrEncoding::None:
> > +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
> > +		break;
> > +	case ColorSpace::YcbcrEncoding::Rec601:
> > +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
> > +		break;
> > +	case ColorSpace::YcbcrEncoding::Rec709:
> > +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
> > +		break;
> > +	case ColorSpace::YcbcrEncoding::Rec2020:
> > +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
> > +		break;
> > +	}
> > +
> > +	switch (colorSpace.range) {
> > +	case ColorSpace::Range::Full:
> > +		colorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;
> > +		break;
> > +	case ColorSpace::Range::Limited:
> > +		colorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;
> > +		break;
> > +	}
> > +
> > +	return colorimetry;
> > +}
> > +
> > +static std::optional<ColorSpace>
> > +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)
> > +{
> > +	std::optional<ColorSpace> colorspace;
> 
> I started a rigourous testing on RPi today comparing cam's colorspace 
> and gstreamer's colorspace mapping/adjustment/logs stuff and happen to 
> catch this last-min bug here.
> 
> The optional colorspace value needs to be assigned (or initialised with 
> a value here). Otherwise the return value from this function is always 
> std::nullopt (since we never assign a value hence the optional ptr is 
> acting as non-containing value)...

Oops indeed.

> For now, I have:
> 
>      -       std::optional<ColorSpace> colorspace;
>      +       std::optional<ColorSpace> colorspace = ColorSpace::Raw;
> 
> Any objections for this Laurent, Rishi ?

Works for me.

> > +
> > +	switch (colorimetry.primaries) {
> > +	case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
> > +		/* Unknown primaries map to raw colorspace in gstreamer */
> > +		return ColorSpace::Raw;
> > +	case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
> > +		colorspace->primaries = ColorSpace::Primaries::Smpte170m;
> > +		break;
> > +	case GST_VIDEO_COLOR_PRIMARIES_BT709:
> > +		colorspace->primaries = ColorSpace::Primaries::Rec709;
> > +		break;
> > +	case GST_VIDEO_COLOR_PRIMARIES_BT2020:
> > +		colorspace->primaries = ColorSpace::Primaries::Rec2020;
> > +		break;
> > +	default:
> > +		GST_WARNING("Colorimetry primaries %d not mapped in gstlibcamera",
> > +			    colorimetry.primaries);
> > +		return std::nullopt;
> > +	}
> > +
> > +	switch (colorimetry.transfer) {
> > +	/* Transfer function mappings inspired from v4l2src plugin */
> > +	case GST_VIDEO_TRANSFER_GAMMA18:
> > +	case GST_VIDEO_TRANSFER_GAMMA20:
> > +	case GST_VIDEO_TRANSFER_GAMMA22:
> > +	case GST_VIDEO_TRANSFER_GAMMA28:
> > +		GST_WARNING("GAMMA 18, 20, 22, 28 transfer functions not supported");
> > +	/* fallthrough */
> > +	case GST_VIDEO_TRANSFER_GAMMA10:
> > +		colorspace->transferFunction = ColorSpace::TransferFunction::Linear;
> > +		break;
> > +	case GST_VIDEO_TRANSFER_SRGB:
> > +		colorspace->transferFunction = ColorSpace::TransferFunction::Srgb;
> > +		break;
> > +	case GST_VIDEO_TRANSFER_BT601:
> > +	case GST_VIDEO_TRANSFER_BT2020_12:
> > +	case GST_VIDEO_TRANSFER_BT2020_10:
> > +	case GST_VIDEO_TRANSFER_BT709:
> > +		colorspace->transferFunction = ColorSpace::TransferFunction::Rec709;
> > +		break;
> > +	default:
> > +		GST_WARNING("Colorimetry transfer function %d not mapped in gstlibcamera",
> > +			    colorimetry.transfer);
> > +		return std::nullopt;
> > +	}
> > +
> > +	switch (colorimetry.matrix) {
> > +	case GST_VIDEO_COLOR_MATRIX_RGB:
> > +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
> > +		break;
> > +	/* FCC is about the same as BT601 with less digit */
> > +	case GST_VIDEO_COLOR_MATRIX_FCC:
> > +	case GST_VIDEO_COLOR_MATRIX_BT601:
> > +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;
> > +		break;
> > +	case GST_VIDEO_COLOR_MATRIX_BT709:
> > +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;
> > +		break;
> > +	case GST_VIDEO_COLOR_MATRIX_BT2020:
> > +		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;
> > +		break;
> > +	default:
> > +		GST_WARNING("Colorimetry matrix %d not mapped in gstlibcamera",
> > +			    colorimetry.matrix);
> > +		return std::nullopt;
> > +	}
> > +
> > +	switch (colorimetry.range) {
> > +	case GST_VIDEO_COLOR_RANGE_0_255:
> > +		colorspace->range = ColorSpace::Range::Full;
> > +		break;
> > +	case GST_VIDEO_COLOR_RANGE_16_235:
> > +		colorspace->range = ColorSpace::Range::Limited;
> > +		break;
> > +	default:
> > +		GST_WARNING("Colorimetry range %d not mapped in gstlibcamera",
> > +			    colorimetry.range);
> > +		return std::nullopt;
> > +	}
> > +
> > +	return colorspace;
> > +}
> > +
> >   static GstVideoFormat
> >   pixel_format_to_gst_format(const PixelFormat &format)
> >   {
> > @@ -151,6 +297,18 @@ gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg
> >   			  "width", G_TYPE_INT, stream_cfg.size.width,
> >   			  "height", G_TYPE_INT, stream_cfg.size.height,
> >   			  nullptr);
> > +
> > +	if (stream_cfg.colorSpace) {
> > +		GstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());
> > +		gchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> > +
> > +		if (colorimetry_str)
> > +			gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
> > +		else
> > +			g_error("Got invalid colorimetry from ColorSpace: %s",
> > +				ColorSpace::toString(stream_cfg.colorSpace).c_str());
> > +	}
> > +
> >   	gst_caps_append_structure(caps, s);
> >   
> >   	return caps;
> > @@ -234,6 +392,17 @@ gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
> >   	gst_structure_get_int(s, "height", &height);
> >   	stream_cfg.size.width = width;
> >   	stream_cfg.size.height = height;
> > +
> > +	/* 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);
> > +	}
> >   }
> >   
> >   #if !GST_CHECK_VERSION(1, 17, 1)

Patch
diff mbox series

diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
index 5a21a391..c7b44a8e 100644
--- a/src/gstreamer/gstlibcamera-utils.cpp
+++ b/src/gstreamer/gstlibcamera-utils.cpp
@@ -57,6 +57,152 @@  static struct {
 	/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */
 };
 
+static GstVideoColorimetry
+colorimetry_from_colorspace(const ColorSpace &colorSpace)
+{
+	GstVideoColorimetry colorimetry;
+
+	switch (colorSpace.primaries) {
+	case ColorSpace::Primaries::Raw:
+		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
+		break;
+	case ColorSpace::Primaries::Smpte170m:
+		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
+		break;
+	case ColorSpace::Primaries::Rec709:
+		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
+		break;
+	case ColorSpace::Primaries::Rec2020:
+		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
+		break;
+	}
+
+	switch (colorSpace.transferFunction) {
+	case ColorSpace::TransferFunction::Linear:
+		colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
+		break;
+	case ColorSpace::TransferFunction::Srgb:
+		colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
+		break;
+	case ColorSpace::TransferFunction::Rec709:
+		colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
+		break;
+	}
+
+	switch (colorSpace.ycbcrEncoding) {
+	case ColorSpace::YcbcrEncoding::None:
+		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
+		break;
+	case ColorSpace::YcbcrEncoding::Rec601:
+		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
+		break;
+	case ColorSpace::YcbcrEncoding::Rec709:
+		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
+		break;
+	case ColorSpace::YcbcrEncoding::Rec2020:
+		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
+		break;
+	}
+
+	switch (colorSpace.range) {
+	case ColorSpace::Range::Full:
+		colorimetry.range = GST_VIDEO_COLOR_RANGE_0_255;
+		break;
+	case ColorSpace::Range::Limited:
+		colorimetry.range = GST_VIDEO_COLOR_RANGE_16_235;
+		break;
+	}
+
+	return colorimetry;
+}
+
+static std::optional<ColorSpace>
+colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)
+{
+	std::optional<ColorSpace> colorspace;
+
+	switch (colorimetry.primaries) {
+	case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
+		/* Unknown primaries map to raw colorspace in gstreamer */
+		return ColorSpace::Raw;
+	case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
+		colorspace->primaries = ColorSpace::Primaries::Smpte170m;
+		break;
+	case GST_VIDEO_COLOR_PRIMARIES_BT709:
+		colorspace->primaries = ColorSpace::Primaries::Rec709;
+		break;
+	case GST_VIDEO_COLOR_PRIMARIES_BT2020:
+		colorspace->primaries = ColorSpace::Primaries::Rec2020;
+		break;
+	default:
+		GST_WARNING("Colorimetry primaries %d not mapped in gstlibcamera",
+			    colorimetry.primaries);
+		return std::nullopt;
+	}
+
+	switch (colorimetry.transfer) {
+	/* Transfer function mappings inspired from v4l2src plugin */
+	case GST_VIDEO_TRANSFER_GAMMA18:
+	case GST_VIDEO_TRANSFER_GAMMA20:
+	case GST_VIDEO_TRANSFER_GAMMA22:
+	case GST_VIDEO_TRANSFER_GAMMA28:
+		GST_WARNING("GAMMA 18, 20, 22, 28 transfer functions not supported");
+	/* fallthrough */
+	case GST_VIDEO_TRANSFER_GAMMA10:
+		colorspace->transferFunction = ColorSpace::TransferFunction::Linear;
+		break;
+	case GST_VIDEO_TRANSFER_SRGB:
+		colorspace->transferFunction = ColorSpace::TransferFunction::Srgb;
+		break;
+	case GST_VIDEO_TRANSFER_BT601:
+	case GST_VIDEO_TRANSFER_BT2020_12:
+	case GST_VIDEO_TRANSFER_BT2020_10:
+	case GST_VIDEO_TRANSFER_BT709:
+		colorspace->transferFunction = ColorSpace::TransferFunction::Rec709;
+		break;
+	default:
+		GST_WARNING("Colorimetry transfer function %d not mapped in gstlibcamera",
+			    colorimetry.transfer);
+		return std::nullopt;
+	}
+
+	switch (colorimetry.matrix) {
+	case GST_VIDEO_COLOR_MATRIX_RGB:
+		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
+		break;
+	/* FCC is about the same as BT601 with less digit */
+	case GST_VIDEO_COLOR_MATRIX_FCC:
+	case GST_VIDEO_COLOR_MATRIX_BT601:
+		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec601;
+		break;
+	case GST_VIDEO_COLOR_MATRIX_BT709:
+		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec709;
+		break;
+	case GST_VIDEO_COLOR_MATRIX_BT2020:
+		colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Rec2020;
+		break;
+	default:
+		GST_WARNING("Colorimetry matrix %d not mapped in gstlibcamera",
+			    colorimetry.matrix);
+		return std::nullopt;
+	}
+
+	switch (colorimetry.range) {
+	case GST_VIDEO_COLOR_RANGE_0_255:
+		colorspace->range = ColorSpace::Range::Full;
+		break;
+	case GST_VIDEO_COLOR_RANGE_16_235:
+		colorspace->range = ColorSpace::Range::Limited;
+		break;
+	default:
+		GST_WARNING("Colorimetry range %d not mapped in gstlibcamera",
+			    colorimetry.range);
+		return std::nullopt;
+	}
+
+	return colorspace;
+}
+
 static GstVideoFormat
 pixel_format_to_gst_format(const PixelFormat &format)
 {
@@ -151,6 +297,18 @@  gst_libcamera_stream_configuration_to_caps(const StreamConfiguration &stream_cfg
 			  "width", G_TYPE_INT, stream_cfg.size.width,
 			  "height", G_TYPE_INT, stream_cfg.size.height,
 			  nullptr);
+
+	if (stream_cfg.colorSpace) {
+		GstVideoColorimetry colorimetry = colorimetry_from_colorspace(stream_cfg.colorSpace.value());
+		gchar *colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
+
+		if (colorimetry_str)
+			gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
+		else
+			g_error("Got invalid colorimetry from ColorSpace: %s",
+				ColorSpace::toString(stream_cfg.colorSpace).c_str());
+	}
+
 	gst_caps_append_structure(caps, s);
 
 	return caps;
@@ -234,6 +392,17 @@  gst_libcamera_configure_stream_from_caps(StreamConfiguration &stream_cfg,
 	gst_structure_get_int(s, "height", &height);
 	stream_cfg.size.width = width;
 	stream_cfg.size.height = height;
+
+	/* 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);
+	}
 }
 
 #if !GST_CHECK_VERSION(1, 17, 1)