[{"id":17524,"web_url":"https://patchwork.libcamera.org/comment/17524/","msgid":"","date":"2021-06-14T10:40:43","subject":"Re: [libcamera-devel] [RFC PATCH 1/3] libcamera: Add ColorSpace\n\tclass","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 work.\n\nOn Tue, 8 Jun 2021 at 15:44, David Plowman \nwrote:\n\n> This class represents a colour space by defining its YCbCr encoding,\n> the transfer (gamma) function is uses, and whether the output is full\n> or limited range.\n> ---\n> include/libcamera/color_space.h | 94 +++++++++++++++++++++++++++++++++\n> include/libcamera/meson.build | 1 +\n> src/libcamera/color_space.cpp | 43 +++++++++++++++\n> src/libcamera/meson.build | 1 +\n> 4 files changed, 139 insertions(+)\n> create mode 100644 include/libcamera/color_space.h\n> create mode 100644 src/libcamera/color_space.cpp\n>\n> diff --git a/include/libcamera/color_space.h\n> b/include/libcamera/color_space.h\n> new file mode 100644\n> index 00000000..3d990f99\n> --- /dev/null\n> +++ b/include/libcamera/color_space.h\n> @@ -0,0 +1,94 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> + *\n> + * color_space.h - color space definitions\n> + */\n> +\n> +#ifndef __LIBCAMERA_COLOR_SPACE_H__\n> +#define __LIBCAMERA_COLOR_SPACE_H__\n> +\n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class ColorSpace\n> +{\n> +public:\n> + enum class Encoding : int {\n> + UNDEFINED,\n> + RAW,\n> + REC601,\n> + REC709,\n> + REC2020,\n> + VIDEO,\n> + };\n> +\n> + enum class TransferFunction : int {\n> + UNDEFINED,\n> + IDENTITY,\n> + SRGB,\n> + REC709,\n> + };\n> +\n> + enum class Range : int {\n> + UNDEFINED,\n> + FULL,\n> + LIMITED,\n> + };\n> +\n> + constexpr ColorSpace(Encoding e, TransferFunction t, Range r)\n> + : encoding(e), transferFunction(t), range(r)\n> + {\n> + }\n> +\n> + constexpr ColorSpace()\n> + : ColorSpace(Encoding::UNDEFINED,\n> TransferFunction::UNDEFINED, Range::UNDEFINED)\n> + {\n> + }\n> +\n> + static const ColorSpace UNDEFINED;\n> + static const ColorSpace RAW;\n> + static const ColorSpace JFIF;\n> + static const ColorSpace SMPTE170M;\n> + static const ColorSpace REC709;\n> + static const ColorSpace REC2020;\n> + static const ColorSpace VIDEO;\n> +\n> + Encoding encoding;\n> + TransferFunction transferFunction;\n> + Range range;\n> +\n> + bool isFullyDefined() const\n> + {\n> + return encoding != Encoding::UNDEFINED &&\n> + transferFunction != TransferFunction::UNDEFINED &&\n> + range != Range::UNDEFINED;\n> + }\n> +\n> + const std::string toString() const;\n> +};\n> +\n> +constexpr ColorSpace ColorSpace::UNDEFINED = { Encoding::UNDEFINED,\n> TransferFunction::UNDEFINED, Range::UNDEFINED };\n>\n\nDo you think we need ColorSpace::UNDEFINED? Not sure why anyone would need\nto use it?\n\n\n> +constexpr ColorSpace ColorSpace::RAW = { Encoding::RAW,\n> TransferFunction::IDENTITY, Range::FULL };\n> +constexpr ColorSpace ColorSpace::JFIF = { Encoding::REC601,\n> TransferFunction::SRGB, Range::FULL };\n> +constexpr ColorSpace ColorSpace::SMPTE170M = { Encoding::REC601,\n> TransferFunction::REC709, Range::LIMITED };\n> +constexpr ColorSpace ColorSpace::REC709 = { Encoding::REC709,\n> TransferFunction::REC709, Range::LIMITED };\n> +constexpr ColorSpace ColorSpace::REC2020 = { Encoding::REC2020,\n> TransferFunction::REC709, Range::LIMITED };\n> +constexpr ColorSpace ColorSpace::VIDEO = { Encoding::VIDEO,\n> TransferFunction::REC709, Range::LIMITED };\n> +\n> +static inline bool operator==(const ColorSpace &lhs, const ColorSpace\n> &rhs)\n> +{\n> + return lhs.encoding == rhs.encoding &&\n> + lhs.transferFunction == rhs.transferFunction &&\n> + lhs.range == rhs.range;\n> +}\n> +\n> +static inline bool operator!=(const ColorSpace &lhs, const ColorSpace\n> &rhs)\n> +{\n> + return !(lhs == rhs);\n> +}\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_COLOR_SPACE_H__ */\n> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build\n> index 086c958b..7d61d285 100644\n> --- a/include/libcamera/meson.build\n> +++ b/include/libcamera/meson.build\n> @@ -6,6 +6,7 @@ libcamera_public_headers = files([\n> 'camera.h',\n> 'camera_manager.h',\n> 'class.h',\n> + 'color_space.h',\n> 'compiler.h',\n> 'controls.h',\n> 'file_descriptor.h',\n> diff --git a/src/libcamera/color_space.cpp b/src/libcamera/color_space.cpp\n> new file mode 100644\n> index 00000000..888762ae\n> --- /dev/null\n> +++ b/src/libcamera/color_space.cpp\n> @@ -0,0 +1,43 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Raspberry Pi (Trading) Limited\n> + *\n> + * color_space.cpp - color spaces.\n> + */\n> +\n> +#include \n> +\n> +/**\n> + * \\file color_space.h\n> + * \\brief Class and enums to represent colour spaces.\n> + */\n> +\n> +namespace libcamera {\n> +\n> +const std::string ColorSpace::toString() const\n> +{\n> + static const char *encodings[] = {\n> + \"UNDEFINED\",\n> + \"RAW\",\n> + \"REC601\",\n> + \"REC709\",\n> + \"REC2020\",\n> + };\n> + static const char *transferFunctions[] = {\n> + \"UNDEFINED\",\n> + \"IDENTITY\",\n> + \"SRGB\",\n> + \"REC709\",\n> + };\n> + static const char *ranges[] = {\n> + \"UNDEFINED\",\n> + \"FULL\",\n> + \"LIMITED\",\n> + };\n> +\n> + return std::string(encodings[static_cast(encoding)]) + \"+\" +\n> +\n> std::string(transferFunctions[static_cast(transferFunction)]) + \"+\" +\n> + std::string(ranges[static_cast(range)]);\n>\n\nI wonder if using stringstream might be more efficient here?\n\nRegards,\nNaush\n\n\n\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 7e19a177..f7b48a32 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -11,6 +11,7 @@ libcamera_sources = files([\n> 'camera_sensor.cpp',\n> 'camera_sensor_properties.cpp',\n> 'class.cpp',\n> + 'color_space.cpp',\n> 'controls.cpp',\n> 'control_serializer.cpp',\n> 'control_validator.cpp',\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 25733C3218\n\tfor ;\n\tMon, 14 Jun 2021 10:41:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 715B268932;\n\tMon, 14 Jun 2021 12:41:02 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 844B76029D\n\tfor ;\n\tMon, 14 Jun 2021 12:41:00 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id r16so19487146ljk.9\n\tfor ;\n\tMon, 14 Jun 2021 03:41:00 -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=\"M3E2PPGm\"; 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=OUGyxqrTXSXTTC1RGEMoE7R31ukCmU9CbDf+7FL+RrI=;\n\tb=M3E2PPGm3fTojNI6U627vvSfo6jwMzt+/ujXt9ryN7M8WVjRKPxBwov+vkWTiKDsWK\n\tuhtqiPNUB7ZOS58DTb8zhVDKKV7EjKzKyH88a8OqEw1bmITrhOQVj0RF9Cv4Kt8/pHeN\n\tJ5ENtznrKehfC52CwB3xyq5VqWqc3ApIOj4MI4/e5P6I8f/d3fnvubpZqnnbm2n6uy2F\n\twgATjo5jYsFzTWGYoaavSYGoXfrutZKcURclLtZ2ypC/Z+PuDt+bNKY3Of3arpzPpvpV\n\t0cu5IOWOo64sjAa6o6U9UVSy6OBjlf0M7hQ7shTS1TkGHUSpnHlLa7xjiY3c36ttIwqY\n\trzww==","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=OUGyxqrTXSXTTC1RGEMoE7R31ukCmU9CbDf+7FL+RrI=;\n\tb=UL7nkmSmigG/p02e1OtfY5d21sEROjLZ75zAnuRgnKVq8LXcTOW6U4K6tY0BHEj7tL\n\tpkdqnuCrS8T+PPPB4XMICML3FRTvKyLLBZiGxEt4qOlem7jk1urml39aqv7K5e3DEYOm\n\tIGvnn+ECfRPAsGVBFBD8bs1gmdjLNKQgsVGb0SguyNQc8V7U3Ux+aK1uxy7cCsSe00rc\n\taa7S4hp4GsWBQ03jAIxBpA8IHrRFpKzavLXmkNGNAve9MYZkm1nI8g8wpTobN9fZWfeD\n\tdZpEm5Rzk0a6iL2Z1qKd0zPN9lPYYMVrJbfW7yG56cM3SvByu9rif9alqvYkAS1qMwJ+\n\tC/0Q==","X-Gm-Message-State":"AOAM531R0jZE+su+oOCajfcDl2a3yRD4EndF6Er3Ygcxtg2QHubwzgOv\n\tSIASi3iQCADljX9ezTtlkHFpXJDW4fCLvbRcFkPpaQ==","X-Google-Smtp-Source":"ABdhPJyjiVQDdV5QBD6epQS2xiiYSCJzbLxvDtmR3Q22jSsURPj5XVCCddIX+x7f/tl9s7vTA/qJMsq+y+td+Z/SVtQ=","X-Received":"by 2002:a2e:a230:: with SMTP id\n\ti16mr13415310ljm.169.1623667259624; \n\tMon, 14 Jun 2021 03:40:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20210608144413.1529-1-david.plowman@raspberrypi.com>\n\t<20210608144413.1529-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20210608144413.1529-2-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Mon, 14 Jun 2021 11:40:43 +0100","Message-ID":"","To":"David Plowman ","Content-Type":"multipart/alternative; boundary=\"0000000000003e5f1805c4b77efe\"","Subject":"Re: [libcamera-devel] [RFC PATCH 1/3] libcamera: Add ColorSpace\n\tclass","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\" "}}]