[libcamera-devel] [PATCH v2 1/4] gstreamer: convert from libcamera colorspace toGStreamer colorimetry.

[Vedant Paranjape]

Hello Rishikesh,

I have one more comment relating to Patch 1 and 2.

Since colorimerty_from_colorspace is called in patch 2 only after
colorspace has been confirmed to have a value, why not change the
datatype of the argument to ColorSpace instead of
std::optional<ColorSpace>, you can use ColorSpace.value() to access
the value stored in the std::optional.

Just as a footnote, I found this blog useful to understand the
functioning of std::optional:
https://devblogs.microsoft.com/cppblog/stdoptional-how-when-and-why/

+       if (colorspace) {
+               colorimetry = colorimerty_from_colorspace(co
lorspace);

On Thu, Jul 7, 2022 at 11:45 AM Rishikesh Donadkar
<rishikeshdonadkar at gmail.com> wrote:
>
> 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 the libcamera colorspace to GStreamer colorimetry and find
> the colorimetry corresponding to the colorspace that is being applied to
> the camera. This colorimetry if found will be pushed in the caps.
>
> Signed-off-by: Rishikesh Donadkar <rishikeshdonadkar at gmail.com>
> ---
>  src/gstreamer/gstlibcamera-utils.cpp | 32 ++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
> index c97c0d43..60ac8c8e 100644
> --- a/src/gstreamer/gstlibcamera-utils.cpp
> +++ b/src/gstreamer/gstlibcamera-utils.cpp
> @@ -45,6 +45,12 @@ 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 GstVideoFormat
>  pixel_format_to_gst_format(const PixelFormat &format)
>  {
> @@ -87,6 +93,32 @@ bare_structure_from_format(const PixelFormat &format)
>         }
>  }
>
> +static gchar *
> +colorimerty_from_colorspace(std::optional<ColorSpace> colorSpace)
> +{
> +       gchar *colorimetry_str = nullptr;
> +       gchar *colorimetry_found = nullptr;
> +       GstVideoColorimetry colorimetry;
> +       gboolean isColorimetryValid;
> +
> +       auto iterColorimetry = std::find_if(ColorSpaceTocolorimetry.begin(), ColorSpaceTocolorimetry.end(),
> +                                           [&colorSpace](const auto &item) {
> +                                                   return colorSpace == item.first;
> +                                           });
> +       if (iterColorimetry != ColorSpaceTocolorimetry.end()) {
> +               colorimetry_found = (gchar *)iterColorimetry->second.c_str();
> +               isColorimetryValid = gst_video_colorimetry_from_string(&colorimetry, colorimetry_found);
> +       }
> +       if (isColorimetryValid) {
> +               colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);
> +               return colorimetry_str;
> +       } else {
> +               g_free(colorimetry_found);
> +               g_free(colorimetry_str);
> +               return nullptr;
> +       }
> +}
> +
>  GstCaps *
>  gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>  {
> --
> 2.25.1
>