[{"id":21604,"web_url":"https://patchwork.libcamera.org/comment/21604/","msgid":"<20211206112038.fzyvze5bdzd4c3ot@uno.localdomain>","date":"2021-12-06T11:20:38","subject":"Re: [libcamera-devel] [PATCH v9 5/8] libcamera: Add colorSpace\n\tfield to V4L2SubdeviceFormat","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Mon, Dec 06, 2021 at 10:50:28AM +0000, David Plowman wrote:\n> This adds a ColorSpace field to the V4L2SubdeviceFormat so that we can\n> set and request particular color spaces from V4L2.\n>\n> This commit simply adds the field and fixes some occurrences of brace\n> initializers that would otherwise be broken. A subsequent commit will\n> pass and retrieve the value correctly to/from V4l2 itself.\n>\n> Signed-off-by: David Plowman \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 2 ++\n> src/libcamera/camera_sensor.cpp | 2 ++\n> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++----\n> src/libcamera/pipeline/simple/simple.cpp | 8 ++++++--\n> src/libcamera/v4l2_subdevice.cpp | 11 +++++++++++\n> 5 files changed, 24 insertions(+), 6 deletions(-)\n>\n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index a6873b67..358bf2b6 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> #include \"libcamera/internal/formats.h\"\n> @@ -27,6 +28,7 @@ class MediaDevice;\n> struct V4L2SubdeviceFormat {\n> \tuint32_t mbus_code;\n> \tSize size;\n> +\tstd::optional colorSpace;\n>\n> \tconst std::string toString() const;\n> \tuint8_t bitsPerPixel() const;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 4c142a58..14358333 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n>\n> #include \n> @@ -586,6 +587,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector &mbu\n> \tV4L2SubdeviceFormat format{\n> \t\t.mbus_code = bestCode,\n> \t\t.size = *bestSize,\n> +\t\t.colorSpace = ColorSpace::Raw,\n> \t};\n>\n> \treturn format;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 59dda56b..f4e8c663 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n> \t\treturn {};\n> \t}\n>\n> -\tV4L2SubdeviceFormat format{\n> -\t\t.mbus_code = bestCode,\n> -\t\t.size = bestSize,\n> -\t};\n> +\tV4L2SubdeviceFormat format{};\n> +\tformat.mbus_code = bestCode;\n> +\tformat.size = bestSize;\n>\n> \treturn format;\n> }\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 701fb4be..a3108fc0 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -457,7 +457,9 @@ int SimpleCameraData::init()\n> \t * formats on the video node.\n> \t */\n> \tfor (unsigned int code : sensor_->mbusCodes()) {\n> -\t\tV4L2SubdeviceFormat format{ code, sensor_->resolution() };\n> +\t\tV4L2SubdeviceFormat format{};\n> +\t\tformat.mbus_code = code;\n> +\t\tformat.size = sensor_->resolution();\n>\n> \t\tret = setupFormats(&format, V4L2Subdevice::TryFormat);\n> \t\tif (ret < 0) {\n> @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \t\treturn ret;\n>\n> \tconst SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> -\tV4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> +\tV4L2SubdeviceFormat format{};\n> +\tformat.mbus_code = pipeConfig->code;\n> +\tformat.size = data->sensor_->resolution();\n>\n> \tret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> \tif (ret < 0)\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 61e15b69..981645e0 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -169,6 +169,17 @@ const std::map formatInfoMap = {\n> * \\brief The image size in pixels\n> */\n>\n> +/**\n> + * \\var V4L2SubdeviceFormat::colorSpace\n> + * \\brief The color space of the pixels\n> + *\n> + * The color space of the image. When setting the format this may be\n> + * unset, in which case the driver gets to use its default color space.\n> + * If this value is unset after a call to validate(), then the color space\n> + * chosen by the driver could not be represented by the ColorSpace class\n> + * (and should probably be added).\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the format\n> * \\return A string describing the V4L2SubdeviceFormat\n> --\n> 2.20.1\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 9CA13BF415\n\tfor ;\n\tMon, 6 Dec 2021 11:19:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 62CE66086C;\n\tMon, 6 Dec 2021 12:19:50 +0100 (CET)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5F64B60725\n\tfor ;\n\tMon, 6 Dec 2021 12:19:48 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 3AC1E40009;\n\tMon, 6 Dec 2021 11:19:45 +0000 (UTC)"],"Date":"Mon, 6 Dec 2021 12:20:38 +0100","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20211206112038.fzyvze5bdzd4c3ot@uno.localdomain>","References":"<20211206105032.13876-1-david.plowman@raspberrypi.com>\n\t<20211206105032.13876-6-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211206105032.13876-6-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v9 5/8] libcamera: Add colorSpace\n\tfield to V4L2SubdeviceFormat","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":"tfiga@google.com, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21632,"web_url":"https://patchwork.libcamera.org/comment/21632/","msgid":"","date":"2021-12-07T13:33:43","subject":"Re: [libcamera-devel] [PATCH v9 5/8] libcamera: Add colorSpace\n\tfield to V4L2SubdeviceFormat","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Mon, Dec 06, 2021 at 10:50:28AM +0000, David Plowman wrote:\n> This adds a ColorSpace field to the V4L2SubdeviceFormat so that we can\n> set and request particular color spaces from V4L2.\n> \n> This commit simply adds the field and fixes some occurrences of brace\n> initializers that would otherwise be broken. A subsequent commit will\n> pass and retrieve the value correctly to/from V4l2 itself.\n> \n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/internal/v4l2_subdevice.h | 2 ++\n> src/libcamera/camera_sensor.cpp | 2 ++\n> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +++----\n> src/libcamera/pipeline/simple/simple.cpp | 8 ++++++--\n> src/libcamera/v4l2_subdevice.cpp | 11 +++++++++++\n> 5 files changed, 24 insertions(+), 6 deletions(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_subdevice.h b/include/libcamera/internal/v4l2_subdevice.h\n> index a6873b67..358bf2b6 100644\n> --- a/include/libcamera/internal/v4l2_subdevice.h\n> +++ b/include/libcamera/internal/v4l2_subdevice.h\n> @@ -14,6 +14,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \"libcamera/internal/formats.h\"\n> @@ -27,6 +28,7 @@ class MediaDevice;\n> struct V4L2SubdeviceFormat {\n> \tuint32_t mbus_code;\n> \tSize size;\n> +\tstd::optional colorSpace;\n\n#include \n\n> \n> \tconst std::string toString() const;\n> \tuint8_t bitsPerPixel() const;\n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 4c142a58..14358333 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -15,6 +15,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \n> @@ -586,6 +587,7 @@ V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector &mbu\n> \tV4L2SubdeviceFormat format{\n> \t\t.mbus_code = bestCode,\n> \t\t.size = *bestSize,\n> +\t\t.colorSpace = ColorSpace::Raw,\n> \t};\n> \n> \treturn format;\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 59dda56b..f4e8c663 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -322,10 +322,9 @@ V4L2SubdeviceFormat CIO2Device::getSensorFormat(const std::vector\n> \t\treturn {};\n> \t}\n> \n> -\tV4L2SubdeviceFormat format{\n> -\t\t.mbus_code = bestCode,\n> -\t\t.size = bestSize,\n> -\t};\n> +\tV4L2SubdeviceFormat format{};\n> +\tformat.mbus_code = bestCode;\n> +\tformat.size = bestSize;\n\nNitpicking, I would have used the same code construct in both\ncamera_sensor.cpp and cio2.cpp.\n\nReviewed-by: Laurent Pinchart \n\n> \n> \treturn format;\n> }\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 701fb4be..a3108fc0 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -457,7 +457,9 @@ int SimpleCameraData::init()\n> \t * formats on the video node.\n> \t */\n> \tfor (unsigned int code : sensor_->mbusCodes()) {\n> -\t\tV4L2SubdeviceFormat format{ code, sensor_->resolution() };\n> +\t\tV4L2SubdeviceFormat format{};\n> +\t\tformat.mbus_code = code;\n> +\t\tformat.size = sensor_->resolution();\n> \n> \t\tret = setupFormats(&format, V4L2Subdevice::TryFormat);\n> \t\tif (ret < 0) {\n> @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> \t\treturn ret;\n> \n> \tconst SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> -\tV4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> +\tV4L2SubdeviceFormat format{};\n> +\tformat.mbus_code = pipeConfig->code;\n> +\tformat.size = data->sensor_->resolution();\n> \n> \tret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> \tif (ret < 0)\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 61e15b69..981645e0 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -169,6 +169,17 @@ const std::map formatInfoMap = {\n> * \\brief The image size in pixels\n> */\n> \n> +/**\n> + * \\var V4L2SubdeviceFormat::colorSpace\n> + * \\brief The color space of the pixels\n> + *\n> + * The color space of the image. When setting the format this may be\n> + * unset, in which case the driver gets to use its default color space.\n> + * If this value is unset after a call to validate(), then the color space\n> + * chosen by the driver could not be represented by the ColorSpace class\n> + * (and should probably be added).\n> + */\n> +\n> /**\n> * \\brief Assemble and return a string describing the format\n> * \\return A string describing the V4L2SubdeviceFormat","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 B0399BDB13\n\tfor ;\n\tTue, 7 Dec 2021 13:34:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 16284605C4;\n\tTue, 7 Dec 2021 14:34:14 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11A2960592\n\tfor ;\n\tTue, 7 Dec 2021 14:34:13 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1272E556;\n\tTue, 7 Dec 2021 14:34:11 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Z42HgfzH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1638884052;\n\tbh=nsYc5FZ8CgKwmKkewClCa6RG0sg2utT/x2eJxJ5UdwA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Z42HgfzHB0ksbp0ePcvgyvrT2J5wFQOAzw8g9YElQp2G+cr3pEIhLPfkbuOP+cam0\n\tEOz94HDCFUC6xBlJ3CvWCZ0t1pSMQB8ajfGv8G9DhxibkcolspBsC2v/pZsdV9Up68\n\t5QmkivKKArsUiZdaBQQqDUFstKn2l4KlDgSrjEho=","Date":"Tue, 7 Dec 2021 15:33:43 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20211206105032.13876-1-david.plowman@raspberrypi.com>\n\t<20211206105032.13876-6-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211206105032.13876-6-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v9 5/8] libcamera: Add colorSpace\n\tfield to V4L2SubdeviceFormat","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":"tfiga@google.com, libcamera-devel@lists.libcamera.org,\n\thverkuil-cisco@xs4all.nl","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]