[libcamera-devel] [PATCH v1 1/5] gstreamer: Convert form colorspace to colorimetry.

[Nicolas Dufresne]

Hi Rishikesh,

In the subject line, "Convert form" should be "Convert from". I would probably
use some way so readers knows that your converting from GStreamer to Libcamera
representation of colorspace/colorimetry.

Le dimanche 03 juillet 2022 à 13:03 +0530, Rishikesh Donadkar a écrit :
> Libcamera StreamConfiguration class has colorSpace attribute, which
> holds the colorspace that is being applied to the camera after the
> validation of the camera configuration.
> 
> Map between the libcamera colorspace and GStreamer colorimetry and find
> the closest colorimetry corresponding to the colorspace in stream
> configuration.
> 
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 71 ++++++++++++++++++++++++++++
>  1 file changed, 71 insertions(+)
> 
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index 3f242286..20c39919 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -45,6 +45,34 @@ static struct {
>  	/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */
>  };
>  
> +static const std::vector<std::pair<ColorSpace, std::string>> ColorSpaceTocolorimetry = {
> +	{ ColorSpace::Srgb, GST_VIDEO_COLORIMETRY_SRGB },
> +	{ ColorSpace::Rec709, GST_VIDEO_COLORIMETRY_BT709 },
> +	{ ColorSpace::Rec2020, GST_VIDEO_COLORIMETRY_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::Primaries, GstVideoColorPrimaries> ToGstVideoColorPrimaries = {
> +	{ ColorSpace::Primaries::Smpte170m, GST_VIDEO_COLOR_PRIMARIES_SMPTE170M },
> +	{ ColorSpace::Primaries::Rec709, GST_VIDEO_COLOR_PRIMARIES_BT709 },
> +	{ ColorSpace::Primaries::Rec2020, GST_VIDEO_COLOR_PRIMARIES_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::TransferFunction, GstVideoTransferFunction> ToGstVideoTransferFunction = {
> +	{ ColorSpace::TransferFunction::Srgb, GST_VIDEO_TRANSFER_SRGB },
> +	{ ColorSpace::TransferFunction::Rec709, GST_VIDEO_TRANSFER_BT709 },
> +};
> +
> +static const std::map<ColorSpace::YcbcrEncoding, GstVideoColorMatrix> ToGstVideoColorMatrix = {
> +	{ ColorSpace::YcbcrEncoding::Rec601, GST_VIDEO_COLOR_MATRIX_BT601 },
> +	{ ColorSpace::YcbcrEncoding::Rec709, GST_VIDEO_COLOR_MATRIX_BT709 },
> +	{ ColorSpace::YcbcrEncoding::Rec2020, GST_VIDEO_COLOR_MATRIX_BT2020 },
> +};
> +
> +static const std::map<ColorSpace::Range, GstVideoColorRange> ToGstVideoColorRange = {
> +	{ ColorSpace::Range::Full, GST_VIDEO_COLOR_RANGE_0_255 },
> +	{ ColorSpace::Range::Limited, GST_VIDEO_COLOR_RANGE_16_235 },
> +};
> +
>  static GstVideoFormat
>  pixel_format_to_gst_format(const PixelFormat &format)
>  {
> @@ -87,6 +115,49 @@ bare_structure_from_format(const PixelFormat &format)
>  	}
>  }
>  
> +static const gchar *
> +colorimerty_from_colorspace(std::optional<ColorSpace> colorSpace)
> +{
> +	const gchar *colorimetry_str;
> +	GstVideoColorimetry colorimetry;
> +
> +	auto iterColorimetry = std::find_if(ColorSpaceTocolorimetry.begin(), ColorSpaceTocolorimetry.end(),
> +					    [&colorSpace](const auto &item) {
> +						    return colorSpace == item.first;
> +					    });
> +	if (iterColorimetry != ColorSpaceTocolorimetry.end()) {
> +		colorimetry_str = (gchar *)iterColorimetry->second.c_str();

Any reason for this cast ?

> +		return colorimetry_str;

I believe, even libcamera may pick a well known colorspace with small
alteration. So instead of this, you maybe want to call
gst_video_colorimetry_from_string() and keep processing the rest of the
colorSpace information.

You should also pick a well known default, or just add the possibility to not
set the colorimetry field at all.

> +	}
> +
> +	auto iterPrimaries = ToGstVideoColorPrimaries.find(colorSpace->primaries);
> +	if (iterPrimaries != ToGstVideoColorPrimaries.end())
> +		colorimetry.primaries = iterPrimaries->second;
> +	else
> +		colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;

This should never be used. It is only there so that we can set an invalid
default to the structure, or to signal an parsing error.

> +
> +	auto iterTransferFunction = ToGstVideoTransferFunction.find(colorSpace->transferFunction);
> +	if (iterTransferFunction != ToGstVideoTransferFunction.end())
> +		colorimetry.transfer = iterTransferFunction->second;
> +	else
> +		colorimetry.transfer = GST_VIDEO_TRANSFER_UNKNOWN;

Same.

> +
> +	auto iterYcbcrEncoding = ToGstVideoColorMatrix.find(colorSpace->ycbcrEncoding);
> +	if (iterYcbcrEncoding != ToGstVideoColorMatrix.end())
> +		colorimetry.matrix = iterYcbcrEncoding->second;
> +	else
> +		colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_UNKNOWN;

Same.

> +
> +	auto iterRange = ToGstVideoColorRange.find(colorSpace->range);
> +	if (iterRange != ToGstVideoColorRange.end())
> +		colorimetry.range = iterRange->second;
> +	else
> +		colorimetry.range = GST_VIDEO_COLOR_RANGE_UNKNOWN;

Same.

> +
> +	colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> +	return colorimetry_str;

As the return is a const char *, the colorimetry string is leaked here.

> +}
> +
>  GstCaps *
>  gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>  {