<div dir="ltr"><div dir="ltr"><div>Hello Rishikesh,</div><div><br></div><div>Thanks for the patch.</div><div><br></div><div>> gstreamer: convert from libcamera colorspace toGStreamer colorimetry</div><div> ----^----<br></div><div>You might want to fix the subject (s/toGstreamer/to Gstreamer)<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 7, 2022 at 11:45 AM Rishikesh Donadkar <<a href="mailto:rishikeshdonadkar@gmail.com" target="_blank">rishikeshdonadkar@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Libcamera StreamConfiguration class has colorSpace attribute, which<br>
holds the colorspace that is being applied to the camera after the<br>
validation of the camera configuration.<br>
<br>
Map the libcamera colorspace to GStreamer colorimetry and find<br>
the colorimetry corresponding to the colorspace that is being applied to<br>
the camera. This colorimetry if found will be pushed in the caps.<br>
<br>
Signed-off-by: Rishikesh Donadkar <<a href="mailto:rishikeshdonadkar@gmail.com" target="_blank">rishikeshdonadkar@gmail.com</a>><br>
---<br>
src/gstreamer/gstlibcamera-utils.cpp | 32 ++++++++++++++++++++++++++++<br>
1 file changed, 32 insertions(+)<br>
<br>
diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp<br>
index c97c0d43..60ac8c8e 100644<br>
--- a/src/gstreamer/gstlibcamera-utils.cpp<br>
+++ b/src/gstreamer/gstlibcamera-utils.cpp<br>
@@ -45,6 +45,12 @@ static struct {<br>
/* \todo NV42 is used in libcamera but is not mapped in GStreamer yet. */<br>
};<br>
<br>
+static const std::vector<std::pair<ColorSpace, std::string>> ColorSpaceTocolorimetry = {<br>
</blockquote><div><br></div><div>This might be a nitpick, but I think the variable name should be ColorSpaceToColorimetry (can someone confirm this please ?)<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ { ColorSpace::Srgb, GST_VIDEO_COLORIMETRY_SRGB },<br>
+ { ColorSpace::Rec709, GST_VIDEO_COLORIMETRY_BT709 },<br>
+ { ColorSpace::Rec2020, GST_VIDEO_COLORIMETRY_BT2020 },<br>
+};<br>
+<br>
static GstVideoFormat<br>
pixel_format_to_gst_format(const PixelFormat &format)<br>
{<br>
@@ -87,6 +93,32 @@ bare_structure_from_format(const PixelFormat &format)<br>
}<br>
}<br>
<br>
+static gchar *<br>
+colorimerty_from_colorspace(std::optional<ColorSpace> colorSpace)<br>
+{<br>
+ gchar *colorimetry_str = nullptr;<br>
+ gchar *colorimetry_found = nullptr;<br>
+ GstVideoColorimetry colorimetry;<br>
+ gboolean isColorimetryValid;<br>
+<br>
+ auto iterColorimetry = std::find_if(ColorSpaceTocolorimetry.begin(), ColorSpaceTocolorimetry.end(),<br>
+ [&colorSpace](const auto &item) {<br>
+ return colorSpace == item.first;<br>
+ });<br>
+ if (iterColorimetry != ColorSpaceTocolorimetry.end()) {<br>
+ colorimetry_found = (gchar *)iterColorimetry->second.c_str();<br>
+ isColorimetryValid = gst_video_colorimetry_from_string(&colorimetry, colorimetry_found);<br>
+ }<br>
+ if (isColorimetryValid) {<br>
+ colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);<br>
+ return colorimetry_str;<br>
+ } else {<br>
+ g_free(colorimetry_found);<br>
</blockquote><div><br></div><div>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.</div><div><br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ g_free(colorimetry_str);<br></blockquote><div><br></div><div>> Frees the memory pointed to by mem. If mem is NULL it simply returns. <br></div><div><br></div><div>Umm, you don't need to free this string here as the if statement above never ran, colorimetry_str is still a nullptr.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ return nullptr;<br>
+ }<br>
+}<br>
+<br></blockquote><div><br></div><div>So, now the whole if-else block can be simplified as follows:</div><div> <br>
+ if (isColorimetryValid) {<br>
+ colorimetry_str = gst_video_colorimetry_to_string(&colorimetry);<br>
+ }<br></div><div><a class="gmail_plusreply" id="m_-6286070911373763350plusReplyChip-0">+</a><br></div><div>+ return colorimetry_str;</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
GstCaps *<br>
gst_libcamera_stream_formats_to_caps(const StreamFormats &formats)<br>
{<br>
-- <br>
2.25.1<br>
<br></blockquote><div><br></div><div>Regards,</div><div>Vedant Paranjape <br></div></div></div>