[{"id":17525,"web_url":"https://patchwork.libcamera.org/comment/17525/","msgid":"","date":"2021-06-14T10:56:39","subject":"Re: [libcamera-devel] [RFC PATCH 2/3] libcamera: Support passing\n\tColorSpaces to V4L2 drivers","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David.\n\nThank you for your patch.\n\nOn Tue, 8 Jun 2021 at 15:44, David Plowman \nwrote:\n\n> The ColorSpace class is added to the StreamConfiguration, and is now\n> passed to V4L2 devices where it is handled appropriately. Note how\n> this means that the colour space is configured per-stream (though\n> platforms may restrict this).\n> ---\n> include/libcamera/internal/v4l2_videodevice.h | 2 +\n> include/libcamera/stream.h | 3 +\n> src/libcamera/v4l2_videodevice.cpp | 112 ++++++++++++++++++\n> 3 files changed, 117 insertions(+)\n>\n> diff --git a/include/libcamera/internal/v4l2_videodevice.h\n> b/include/libcamera/internal/v4l2_videodevice.h\n> index 7938343b..5122d422 100644\n> --- a/include/libcamera/internal/v4l2_videodevice.h\n> +++ b/include/libcamera/internal/v4l2_videodevice.h\n> @@ -18,6 +18,7 @@\n>\n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -162,6 +163,7 @@ public:\n>\n> V4L2PixelFormat fourcc;\n> Size size;\n> + ColorSpace colorSpace;\n>\n> std::array planes;\n> unsigned int planesCount = 0;\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index bb47c390..f79c6987 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -13,6 +13,7 @@\n> #include \n>\n> #include \n> +#include \n> #include \n> #include \n>\n> @@ -47,6 +48,8 @@ struct StreamConfiguration {\n>\n> unsigned int bufferCount;\n>\n> + ColorSpace colorSpace;\n> +\n> Stream *stream() const { return stream_; }\n> void setStream(Stream *stream) { stream_ = stream; }\n> const StreamFormats &formats() const { return formats_; }\n> diff --git a/src/libcamera/v4l2_videodevice.cpp\n> b/src/libcamera/v4l2_videodevice.cpp\n> index 12c09dc7..d606f81d 100644\n> --- a/src/libcamera/v4l2_videodevice.cpp\n> +++ b/src/libcamera/v4l2_videodevice.cpp\n> @@ -730,6 +730,114 @@ int V4L2VideoDevice::getFormat(V4L2DeviceFormat\n> *format)\n> return getFormatSingleplane(format);\n> }\n>\n> +static const std::vector>\n> colorSpaceToV4l2 = {\n> + { ColorSpace::RAW, V4L2_COLORSPACE_RAW },\n> + { ColorSpace::JFIF, V4L2_COLORSPACE_JPEG },\n> + { ColorSpace::SMPTE170M, V4L2_COLORSPACE_SMPTE170M },\n> + { ColorSpace::REC709, V4L2_COLORSPACE_REC709 },\n> + { ColorSpace::REC2020, V4L2_COLORSPACE_BT2020 },\n> +};\n> +\n> +static const std::map\n> encodingToV4l2 = {\n> + { ColorSpace::Encoding::REC601, V4L2_YCBCR_ENC_601 },\n> + { ColorSpace::Encoding::REC709, V4L2_YCBCR_ENC_709 },\n> + { ColorSpace::Encoding::REC2020, V4L2_YCBCR_ENC_BT2020 },\n> +};\n> +\n> +static const std::map\n> transferFunctionToV4l2 = {\n> + { ColorSpace::TransferFunction::IDENTITY, V4L2_XFER_FUNC_NONE },\n> + { ColorSpace::TransferFunction::SRGB, V4L2_XFER_FUNC_SRGB },\n> + { ColorSpace::TransferFunction::REC709, V4L2_XFER_FUNC_709 },\n> +};\n> +\n> +static const std::map rangeToV4l2 =\n> {\n> + { ColorSpace::Range::FULL, V4L2_QUANTIZATION_FULL_RANGE },\n> + { ColorSpace::Range::LIMITED, V4L2_QUANTIZATION_LIM_RANGE },\n> +};\n> +\n> +template\n> +static void setColorSpace(const ColorSpace &colorSpace, T &v4l2PixFormat)\n>\n\nCould you expand on why this (and below) needs to be templated?\n\n\n> +{\n> + if (!colorSpace.isFullyDefined())\n> + LOG(V4L2, Warning) << \"Setting non-fully defined colour\n> space\"\n> + << colorSpace.toString();\n> +\n> + v4l2PixFormat.colorspace = V4L2_COLORSPACE_DEFAULT;\n> + v4l2PixFormat.ycbcr_enc = V4L2_YCBCR_ENC_DEFAULT;\n> + v4l2PixFormat.xfer_func = V4L2_XFER_FUNC_DEFAULT;\n> + v4l2PixFormat.quantization = V4L2_QUANTIZATION_DEFAULT;\n> +\n> + auto it_color = std::find_if(colorSpaceToV4l2.begin(),\n> colorSpaceToV4l2.end(),\n> + [&colorSpace](const\n> std::pair &item) { return colorSpace ==\n> item.first; });\n>\n\nCould you use a map for colorSpaceToV4l2 like you do for the others?\n\nRegards,\nNaush\n\n\n> + if (it_color != colorSpaceToV4l2.end())\n> + v4l2PixFormat.colorspace = it_color->second;\n> +\n> + auto it_encoding = encodingToV4l2.find(colorSpace.encoding);\n> + if (it_encoding != encodingToV4l2.end())\n> + v4l2PixFormat.ycbcr_enc = it_encoding->second;\n> +\n> + auto it_transfer =\n> transferFunctionToV4l2.find(colorSpace.transferFunction);\n> + if (it_transfer != transferFunctionToV4l2.end())\n> + v4l2PixFormat.xfer_func = it_transfer->second;\n> +\n> + auto it_range = rangeToV4l2.find(colorSpace.range);\n> + if (it_range != rangeToV4l2.end())\n> + v4l2PixFormat.quantization = it_range->second;\n> +}\n> +\n> +static const std::map v4l2ToColorSpace = {\n> + { V4L2_COLORSPACE_RAW, ColorSpace::RAW },\n> + { V4L2_COLORSPACE_JPEG, ColorSpace::JFIF },\n> + { V4L2_COLORSPACE_SRGB, ColorSpace::JFIF },\n> + { V4L2_COLORSPACE_SMPTE170M, ColorSpace::SMPTE170M },\n> + { V4L2_COLORSPACE_REC709, ColorSpace::REC709 },\n> + { V4L2_COLORSPACE_BT2020, ColorSpace::REC2020 },\n> +};\n> +\n> +static const std::map v4l2ToEncoding = {\n> + { V4L2_YCBCR_ENC_601, ColorSpace::Encoding::REC601 },\n> + { V4L2_YCBCR_ENC_709, ColorSpace::Encoding::REC709 },\n> + { V4L2_YCBCR_ENC_BT2020, ColorSpace::Encoding::REC2020 },\n> +};\n> +\n> +static const std::map\n> v4l2ToTransferFunction = {\n> + { V4L2_XFER_FUNC_NONE, ColorSpace::TransferFunction::IDENTITY },\n> + { V4L2_XFER_FUNC_SRGB, ColorSpace::TransferFunction::SRGB },\n> + { V4L2_XFER_FUNC_709, ColorSpace::TransferFunction::REC709 },\n> +};\n> +\n> +static const std::map v4l2ToRange = {\n> + { V4L2_QUANTIZATION_FULL_RANGE, ColorSpace::Range::FULL },\n> + { V4L2_QUANTIZATION_LIM_RANGE, ColorSpace::Range::LIMITED },\n> +};\n> +\n> +template\n> +ColorSpace getColorSpace(const T &v4l2PixFormat)\n> +{\n> + ColorSpace colorSpace;\n> +\n> + auto it_color = v4l2ToColorSpace.find(v4l2PixFormat.colorspace);\n> + if (it_color != v4l2ToColorSpace.end())\n> + colorSpace = it_color->second;\n> +\n> + auto it_encoding = v4l2ToEncoding.find(v4l2PixFormat.ycbcr_enc);\n> + if (it_encoding != v4l2ToEncoding.end())\n> + colorSpace.encoding = it_encoding->second;\n> +\n> + auto it_transfer =\n> v4l2ToTransferFunction.find(v4l2PixFormat.xfer_func);\n> + if (it_transfer != v4l2ToTransferFunction.end())\n> + colorSpace.transferFunction = it_transfer->second;\n> +\n> + auto it_range = v4l2ToRange.find(v4l2PixFormat.quantization);\n> + if (it_range != v4l2ToRange.end())\n> + colorSpace.range = it_range->second;\n> +\n> + if (!colorSpace.isFullyDefined())\n> + LOG(V4L2, Warning) << \"Returning non-fully defined colour\n> space\"\n> + << colorSpace.toString();\n> + return colorSpace;\n> +}\n> +\n> /**\n> * \\brief Try an image format on the V4L2 video device\n> * \\param[inout] format The image format to test applicability to the\n> video device\n> @@ -839,6 +947,7 @@ int\n> V4L2VideoDevice::getFormatMultiplane(V4L2DeviceFormat *format)\n> format->size.width = pix->width;\n> format->size.height = pix->height;\n> format->fourcc = V4L2PixelFormat(pix->pixelformat);\n> + format->colorSpace = getColorSpace(*pix);\n> format->planesCount = pix->num_planes;\n>\n> for (unsigned int i = 0; i < format->planesCount; ++i) {\n> @@ -861,6 +970,7 @@ int\n> V4L2VideoDevice::trySetFormatMultiplane(V4L2DeviceFormat *format, bool set)\n> pix->pixelformat = format->fourcc;\n> pix->num_planes = format->planesCount;\n> pix->field = V4L2_FIELD_NONE;\n> + setColorSpace(format->colorSpace, *pix);\n>\n> ASSERT(pix->num_planes <= std::size(pix->plane_fmt));\n>\n> @@ -909,6 +1019,7 @@ int\n> V4L2VideoDevice::getFormatSingleplane(V4L2DeviceFormat *format)\n> format->size.width = pix->width;\n> format->size.height = pix->height;\n> format->fourcc = V4L2PixelFormat(pix->pixelformat);\n> + format->colorSpace = getColorSpace(*pix);\n> format->planesCount = 1;\n> format->planes[0].bpl = pix->bytesperline;\n> format->planes[0].size = pix->sizeimage;\n> @@ -928,6 +1039,7 @@ int\n> V4L2VideoDevice::trySetFormatSingleplane(V4L2DeviceFormat *format, bool set)\n> pix->pixelformat = format->fourcc;\n> pix->bytesperline = format->planes[0].bpl;\n> pix->field = V4L2_FIELD_NONE;\n> + setColorSpace(format->colorSpace, *pix);\n> ret = ioctl(set ? VIDIOC_S_FMT : VIDIOC_TRY_FMT, &v4l2Format);\n> if (ret) {\n> LOG(V4L2, Error)\n> --\n> 2.20.1\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B5828C3218\n\tfor ;\n\tMon, 14 Jun 2021 10:56:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB6C76892B;\n\tMon, 14 Jun 2021 12:56:57 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 69B9B6029D\n\tfor ;\n\tMon, 14 Jun 2021 12:56:56 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id m21so20336020lfg.13\n\tfor ;\n\tMon, 14 Jun 2021 03:56:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"TqKz/dze\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=kqnZ2qYVh0CN6kigeOD0p99nP3yxFPVipsRlrh4wDKw=;\n\tb=TqKz/dzezwxjHSHarEHMHcww1l5SWik9+b0eYXwLGimFPVsNtAFfZGPvWCk3+Mu41G\n\t39hTZTBp03baeCJrBuougiaOHhKA2uTPpiZ83o1NgWJ3grI9KsYmZDmU8S+GZhUDbR5C\n\tjf5oVhVwMhIbMGuxXH2Mk6F4iM8yyujK3kj2ivhuYPOoFVdYjbkmjk9B5wv3XAgdvcr0\n\tra86poHTSvEqqKQ29Z9uFkxzoGDcK17MgqNXtmyFtzFLLGPqJe9EI3vIMR1bYX3ddV8Z\n\t7avLVNmJqj61hBsaQMM3tRpLtj3DqUpAM7eoQG1kyQqkb93oKWVEUHWF5BCc8uWovXj8\n\t8aGw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=kqnZ2qYVh0CN6kigeOD0p99nP3yxFPVipsRlrh4wDKw=;\n\tb=sBzc4Y+utoVprD48i9Xj3LBm10u0yU7GEzj9OAgJWihL35UdKcre+RbceZAmZaQC9I\n\t8hLcNpYQ+IVcADdlKZbwyu3KsNRmQdzQZtDoQEZ3mlL7K9Kkgwuk9NHS2YxiLFt6RKWf\n\txxhhipXnEJb3c8pCwIjDHcQ+5AVvhj0+5At5tFc5lfdTJuYlFWGMGBH4kP5xSxaw/JJQ\n\tX69TFnJpF8v9Bwo/xYxfahXj14bE0lpStmirTGO+WAdzhzLQCk2xMKw6c4663tEYgrQ1\n\t8hLCnvR0AbbfE0TuPSMzCIhusUHuYQb8mV0tQHNOp1SBLmykWfq2HbpWGK4g65crgh89\n\tkULA==","X-Gm-Message-State":"AOAM532XQBQfMTiz5sM6n9oltlo5Ubx6U+E7weBMKWiQ8n2ZyR6qc/dh\n\tREq0T7ZCRYIQbdtSkR1mDiYo/MUtIE9t1qS1YIEqcQ==","X-Google-Smtp-Source":"ABdhPJxJR44biS5sjmys47F8V0gMzmsEYGH9dBztWc/rgXnhVPtvrre4bpvC/3wtyqjEJyhLvHjf7XfWBII0Yq28inw=","X-Received":"by 2002:a05:6512:3054:: with SMTP id\n\tb20mr11434155lfb.375.1623668215742; \n\tMon, 14 Jun 2021 03:56:55 -0700 (PDT)","MIME-Version":"1.0","References":"<20210608144413.1529-1-david.plowman@raspberrypi.com>\n\t<20210608144413.1529-3-david.plowman@raspberrypi.com>","In-Reply-To":"<20210608144413.1529-3-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Mon, 14 Jun 2021 11:56:39 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000003b968905c4b7b733\"","Subject":"Re: [libcamera-devel] [RFC PATCH 2/3] libcamera: Support passing\n\tColorSpaces to V4L2 drivers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]