[libcamera-devel] [PATCH v1 1/2] gstreamer: Provide colorimetry <> ColorSpace mappings
Umang Jain
umang.jain at ideasonboard.com
Wed Aug 10 06:30:09 CEST 2022
Hi Laurent and Rishi,
On 8/9/22 02:50, Laurent Pinchart via libcamera-devel wrote:
> Hi Rishikesh,
>
> Thank you for the patch.
>
> On Sun, Aug 07, 2022 at 05:42:32PM +0530, Rishikesh Donadkar via libcamera-devel wrote:
>> 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.
> Your SoB line is missing.
This is should ideally be absent, as the mappings are handled as part of
more general colorspaces rework
[v3 PATCH 4/4] gstreamer: Provide colorimetry <> ColorSpace mappings
Rishi, in cases like this, when you base your work on some out-of-tree
patches, you can just the bits that are relevant:
For e.g. in this case - the 2/2 patch about multiple colorimetry and in
the cover letter you mention :
`Multiple colorimetry support`
....
Based on patch/series "...."
....
So that the out-of-tree patches (but present on the list already for
reviews) don't accidently get re-reviewed (or reviewed at a wrong place)
So, please ignore this patch (in v2 as well) and let's focus on multiple
colorimetry plumbing :-)
>
>> ---
>> src/gstreamer/gstlibcamera-utils.cpp | 158 +++++++++++++++++++++++++++
>> 1 file changed, 158 insertions(+)
>>
>> diff --git a/src/gstreamer/gstlibcamera-utils.cpp b/src/gstreamer/gstlibcamera-utils.cpp
>> index c97c0d43..24d8d035 100644
>> --- a/src/gstreamer/gstlibcamera-utils.cpp
>> +++ b/src/gstreamer/gstlibcamera-utils.cpp
>> @@ -45,6 +45,152 @@ 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;
>> +
>> + switch (colorSpace.primaries) {
>> + case ColorSpace::Primaries::Raw:
>> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_UNKNOWN;
>> + break;
>> + case ColorSpace::Primaries::Smpte170m:
>> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_SMPTE170M;
>> + break;
>> + case ColorSpace::Primaries::Rec709:
>> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT709;
>> + break;
>> + case ColorSpace::Primaries::Rec2020:
>> + colorimetry.primaries = GST_VIDEO_COLOR_PRIMARIES_BT2020;
>> + break;
>> + }
>> +
>> + switch (colorSpace.transferFunction) {
>> + case ColorSpace::TransferFunction::Linear:
>> + colorimetry.transfer = GST_VIDEO_TRANSFER_GAMMA10;
>> + break;
>> + case ColorSpace::TransferFunction::Srgb:
>> + colorimetry.transfer = GST_VIDEO_TRANSFER_SRGB;
>> + break;
>> + case ColorSpace::TransferFunction::Rec709:
>> + colorimetry.transfer = GST_VIDEO_TRANSFER_BT709;
>> + break;
>> + }
>> +
>> + switch (colorSpace.ycbcrEncoding) {
>> + case ColorSpace::YcbcrEncoding::None:
>> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_RGB;
>> + break;
>> + case ColorSpace::YcbcrEncoding::Rec601:
>> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT601;
>> + break;
>> + case ColorSpace::YcbcrEncoding::Rec709:
>> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT709;
>> + break;
>> + case ColorSpace::YcbcrEncoding::Rec2020:
>> + colorimetry.matrix = GST_VIDEO_COLOR_MATRIX_BT2020;
>> + break;
>> + }
>> +
>> + 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;
>> + }
>> +
>> + return colorimetry;
>> +}
>> +
>> +static ColorSpace
>> +colorspace_from_colorimetry(const GstVideoColorimetry &colorimetry)
>> +{
>> + ColorSpace colorspace = ColorSpace::Raw;
>> +
>> + switch (colorimetry.primaries) {
>> + case GST_VIDEO_COLOR_PRIMARIES_UNKNOWN:
>> + /* Unknown primaries map to raw colorspace in gstreamer */
> s/gstreamer/GStreamer./
>
>> + return ColorSpace::Raw;
>> + case GST_VIDEO_COLOR_PRIMARIES_SMPTE170M:
>> + colorspace.primaries = ColorSpace::Primaries::Smpte170m;
>> + break;
>> + case GST_VIDEO_COLOR_PRIMARIES_BT709:
>> + colorspace.primaries = ColorSpace::Primaries::Rec709;
>> + break;
>> + case GST_VIDEO_COLOR_PRIMARIES_BT2020:
>> + colorspace.primaries = ColorSpace::Primaries::Rec2020;
>> + break;
>> + default:
>> + GST_WARNING("Colorimetry primaries %d not mapped in gstlibcamera",
>> + colorimetry.primaries);
>> + return ColorSpace::Raw;
>> + }
>> +
>> + switch (colorimetry.transfer) {
>> + /* Transfer 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_SRGB:
>> + colorspace.transferFunction = ColorSpace::TransferFunction::Srgb;
>> + 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;
>> + default:
>> + GST_WARNING("Colorimetry transfer function %d not mapped in gstlibcamera",
>> + colorimetry.transfer);
>> + return ColorSpace::Raw;
>> + }
>> +
>> + switch (colorimetry.matrix) {
>> + case GST_VIDEO_COLOR_MATRIX_RGB:
>> + colorspace.ycbcrEncoding = ColorSpace::YcbcrEncoding::None;
>> + break;
>> + /* FCC is about the same as BT601 with less digit */
> s/digit/digits./
>
>> + 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;
>> + default:
>> + GST_WARNING("Colorimetry matrix %d not mapped in gstlibcamera",
>> + colorimetry.matrix);
>> + return ColorSpace::Raw;
>> + }
>> +
>> + 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;
>> + default:
>> + GST_WARNING("Colorimetry range %d not mapped in gstlibcamera",
>> + colorimetry.range);
>> + return ColorSpace::Raw;
>> + }
>> +
>> + return colorspace;
>> +}
>> +
>> static GstVideoFormat
>> pixel_format_to_gst_format(const PixelFormat &format)
>> {
>> @@ -139,6 +285,18 @@ 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)
>> + gst_structure_set(s, "colorimetry", G_TYPE_STRING, colorimetry_str, nullptr);
>> + else
>> + g_error("Got invalid colorimetry from ColorSpace: %s",
>> + ColorSpace::toString(stream_cfg.colorSpace).c_str());
>> + }
>> +
> This looks fine, but I think you're missing a part of the patch, as
> colorspace_from_colorimetry() is not used. One option is to move the
> function to patch 2/2, otherwise compiling this patch alone will
> generate a compilation warning.
>
> If you do that, the commit message should also be updated, to reflect
> that this patch only handles the colorimetry in the libcamera to
> GStreamer direction.
>
>> gst_caps_append_structure(caps, s);
>>
>> return caps;
More information about the libcamera-devel
mailing list