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

[Vedant Paranjape]

Hello Rishikesh,

Thanks for the patch.

> gstreamer: convert from libcamera colorspace toGStreamer colorimetry

----^----
You might want to fix the subject (s/toGstreamer/to Gstreamer)

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 = {
>

This might be a nitpick, but I think the variable name should be
ColorSpaceToColorimetry (can someone confirm this please ?)


> +       { 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);
>

I don't think you need to free this, it is owned by the string
(iterColorimetry->second) and it's lifetime will be taken care by the
string.

+               g_free(colorimetry_str);
>

> Frees the memory pointed to by mem. If mem is NULL it simply returns.

Umm, you don't need to free this string here as the if statement above
never ran, colorimetry_str is still a nullptr.

+               return nullptr;
> +       }
> +}
> +
>

So, now the whole if-else block can be simplified as follows:

+       if (isColorimetryValid) {
+               colorimetry_str =
gst_video_colorimetry_to_string(&colorimetry);
+       }
+
+ return colorimetry_str;

 GstCaps *
>  gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)
>  {
> --
> 2.25.1
>
>
Regards,
Vedant Paranjape
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20220707/856a28a6/attachment.htm>