[libcamera-devel] [PATCH v2 3/4] gstreamer:Convert between requested colorimetry and libcamera ColorSpace

[Vedant Paranjape]

Hello Rishikesh,

Thanks for the patch.

> gstreamer:Convert between requested colorimetry and libcamera ColorSpace

You might want to fix the subject line s/gstreamer:Convert/gstreamer: Convert

On Thu, Jul 7, 2022 at 11:45 AM Rishikesh Donadkar
<rishikeshdonadkar at gmail.com> wrote:
>
> Add map colorimetryToColorSpace and the function

s/Add map colorimetryToColorSpace/This patch adds a std::map called
colorimetryToColorSpace

Additionally, it might be good to describe in short what this map
stores, even though the name suggests so, there's no harm in adding
few words.

> colorspace_form_colorimetry() to convert from gstreamer colorimetry
> to libcamera colorspace. The obtained colorspace form the conversion

s/colorspace form/colorspace from

> will be applied to the colorSpace field of the StreamConfiguration.
>
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index eb9c49da..feefead9 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -51,6 +51,12 @@ static const std::vector<std::pair<ColorSpace, std::string>> ColorSpaceTocolorim
>         { ColorSpace::Rec2020, GST_VIDEO_COLORIMETRY_BT2020 },
>  };
>
> +static const std::map<std::string, ColorSpace> colorimetryToColorSpace = {
> +       { GST_VIDEO_COLORIMETRY_SRGB, ColorSpace::Srgb },
> +       { GST_VIDEO_COLORIMETRY_BT709, ColorSpace::Rec709 },
> +       { GST_VIDEO_COLORIMETRY_BT2020, ColorSpace::Rec2020 },
> +};
> +
>  static GstVideoFormat
>  pixel_format_to_gst_format(const PixelFormat &format)
>  {
> @@ -119,6 +125,15 @@ colorimerty_from_colorspace(std::optional<ColorSpace> colorSpace)
>         }
>  }
>
> +void colorspace_form_colorimetry(std::optional<ColorSpace> &colorspace, const gchar *colorimetry)

This function is used local to this source file, it needs to be
defined static. Why not set the return type to
std::optional<ColorSpace> instead of passing a variable(colorspace) by
reference and setting value to it.

Regards,
Vedant Paranjape


> +{
> +       auto iterColorSpace = colorimetryToColorSpace.find(colorimetry);
> +       if (iterColorSpace != colorimetryToColorSpace.end()) {
> +               colorspace = iterColorSpace->second;
> +               return;
> +       }
> +}
> +
>  GstCaps *
>  gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>  {
> --
> 2.25.1
>