[libcamera-devel] [PATCH 3/3] gstreamer: Provide colorimetry <> libcamera::ColorSpace mappings
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Jul 27 03:17:21 CEST 2022
Hi Umang and Rishikesh,
Thank you for the patch.
On Sun, Jul 24, 2022 at 08:13:55PM +0530, Umang Jain via libcamera-devel wrote:
> From: Rishikesh Donadkar <rishikeshdonadkar at 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 gstlibcamerasrc currently undertakes only one
> argument. Multiple colorimetry support shall be introduced in
> subsequent commits.
>
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> src/gstreamer/gstlibcamera-utils.cpp | 175 +++++++++++++++++++++++++++
> 1 file changed, 175 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index c97c0d43..fb4f0e5c 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -45,6 +45,157 @@ 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;
I would initialize the structure with all fields set to UNKNOWN here...
> +
> + switch (colorSpace.primaries) {
> + case ColorSpace::Primaries::Rec709:
> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
> + break;
> + case ColorSpace::Primaries::Rec2020:
> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
> + break;
> + case ColorSpace::Primaries::Smpte170m:
> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
> + break;
> + default:
> + GST_WARNING("ColorSpace primaries not mapped in GstLibcameraSrc");
> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
... and drop the default case. As ColorSpace::Primaries is an enum
class, if you have cases for all the values, the compiler will warn only
if one (or more) of the enum values isn't handled in explicit cases.
This way you can avoid warnings, and have a guarantee that if the
libcamera colorspaces are later extended, the compiler will remind that
this function has to be updated. Same for the other components of the
colorspace below.
> + }
> +
> + switch (colorSpace.transferFunction) {
> + case ColorSpace::TransferFunction::Rec709:
> + colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
> + break;
> + case ColorSpace::TransferFunction::Srgb:
> + colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
> + break;
> + case ColorSpace::TransferFunction::Linear:
> + colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
> + break;
> + default:
> + GST_WARNING("ColorSpace transfer function not mapped in GstLibcameraSrc");
> + colorimetry.transfer = GST_VIDEO_TRANSFER_UNKNOWN;
> + }
> +
> + switch (colorSpace.ycbcrEncoding) {
> + case ColorSpace::YcbcrEncoding::Rec709:
> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
> + break;
> + case ColorSpace::YcbcrEncoding::Rec2020:
> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
> + break;
> + case ColorSpace::YcbcrEncoding::Rec601:
> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
> + break;
> + default:
> + GST_WARNING("Colorspace YcbcrEncoding not mapped in GstLibcameraSrc");
> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_UNKNOWN;
Should we map YcbcrEncoding::None to GST_VIDEO_COLOR_MATRIX_RGB ?
Otherwise capturing RGB or raw bayer will produce a warning. Same for
the Raw primaries, it seems that the best mapping would be
GST_VIDEO_COLOR_PRIMARIES_UNKNOWN (unless I'm missing something), but I
don't think we should warn in that case.
> + }
> +
> + 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;
> + default:
> + GST_WARNING("Colorspace range not mapped in GstLibcameraSrc");
> + colorimetry.range = GST_VIDEO_COLOR_RANGE_UNKNOWN;
> + }
> +
> + return colorimetry;
> +}
> +
> +static std::optional<ColorSpace>
> +colorspace_from_colorimetry(GstVideoColorimetry *colorimetry)
I'd pass a const reference instead of a pointer, as there's no use case
for a null colorimetry.
> +{
> + std::optional<ColorSpace> colorspace = ColorSpace::Default;
No need for std::optional here or in the return type.
> +
> + switch (colorimetry->primaries) {
> + case GST_VIDEO_COLOR_PRIMARIES_BT709:
> + colorspace->primaries = ColorSpace::Primaries::Rec709;
> + break;
> + case GST_VIDEO_COLOR_PRIMARIES_BT2020:
> + colorspace->primaries = ColorSpace::Primaries::Rec2020;
> + break;
> + case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
> + colorspace->primaries = ColorSpace::Primaries::Smpte170m;
> + break;
> + case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
> + colorspace->primaries = ColorSpace::Primaries::Default;
> + break;
> + default:
> + GST_WARNING("Unknown primaries in colorimetry %d", colorimetry->primaries);
> + }
> +
> + switch (colorimetry->transfer) {
> + /* Tranfer 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_BT601:
> + case GST_VIDEO_TRANSFER_BT2020_12:
> + case GST_VIDEO_TRANSFER_BT2020_10:
> + case GST_VIDEO_TRANSFER_BT709:
> + colorspace->transferFunction = ColorSpace::TransferFunction::Rec709;
> + break;
> + case GST_VIDEO_TRANSFER_SRGB:
> + colorspace->transferFunction = ColorSpace::TransferFunction::Srgb;
> + break;
> + case GST_VIDEO_TRANSFER_UNKNOWN:
> + colorspace->transferFunction = ColorSpace::TransferFunction::Default;
> + break;
> + default:
> + GST_WARNING("Unknown colorimetry transfer %d", colorimetry->transfer);
> + }
> +
> + switch (colorimetry->matrix) {
> + /* 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;
> + case GST_VIDEO_COLOR_MATRIX_RGB:
Shouldn't this map to YcbcrEncoding::None ?
> + case GST_VIDEO_COLOR_MATRIX_UNKNOWN:
> + colorspace->ycbcrEncoding = ColorSpace::YcbcrEncoding::Default;
> + break;
> + default:
> + GST_WARNING("Unknown colorimetry matrix %d", colorimetry->matrix);
> + }
> +
> + 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;
> + case GST_VIDEO_COLOR_RANGE_UNKNOWN:
> + colorspace->range = ColorSpace::Range::Default;
> + break;
> + default:
> + GST_WARNING("Unknown range in colorimetry %d", colorimetry->range);
> + }
> +
> + return colorspace;
> +}
> +
> static GstVideoFormat
> pixel_format_to_gst_format(const PixelFormat &format)
> {
> @@ -139,6 +290,17 @@ 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 != nullptr)
if (colorimetry_str)
> + gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
> + else
> + g_warning("libcamera::ColorSpace found but GstVideoColorimetry unknown");
> + }
> +
> gst_caps_append_structure(caps, s);
>
> return caps;
> @@ -222,6 +384,19 @@ 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_caps = gst_structure_get_string(s, "colorimetry");
> + GstVideoColorimetry colorimetry;
> +
> + if(gst_video_colorimetry_from_string(&colorimetry, colorimetry_caps)) {
Missing space after "if".
> + std::optional<ColorSpace> colorSpace = colorspace_from_colorimetry(&colorimetry);
> + stream_cfg.colorSpace = colorSpace;
stream_cfg.colorSpace = colorspace_from_colorimetry(&colorimetry);
> + } else {
> + g_print("Invalid colorimetry %s", colorimetry_caps);
> + }
> + }
> }
>
> #if !GST_CHECK_VERSION(1, 17, 1)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list