[{"id":21523,"web_url":"https://patchwork.libcamera.org/comment/21523/","msgid":"<20211201132140.vl6lmskj6palfj6u@uno.localdomain>","date":"2021-12-01T13:21:40","subject":"Re: [libcamera-devel] [PATCH v7 0/7] Colour spaces","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n forgot to notify it yesterday, but the series breaks the IPU3 and\n the Simple pipeline handlers.\n\nThe following will fix it\n\ndiff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\nindex 59dda56bdd3d..f4e8c6632c2f 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 return {};\n }\n\n- V4L2SubdeviceFormat format{\n- .mbus_code = bestCode,\n- .size = bestSize,\n- };\n+ V4L2SubdeviceFormat format{};\n+ format.mbus_code = bestCode;\n+ format.size = bestSize;\n\n return format;\n }\ndiff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\nindex 701fb4be0b71..a3108fc0b0f6 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 * formats on the video node.\n */\n for (unsigned int code : sensor_->mbusCodes()) {\n- V4L2SubdeviceFormat format{ code, sensor_->resolution() };\n+ V4L2SubdeviceFormat format{};\n+ format.mbus_code = code;\n+ format.size = sensor_->resolution();\n\n ret = setupFormats(&format, V4L2Subdevice::TryFormat);\n if (ret < 0) {\n@@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n return ret;\n\n const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n- V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n+ V4L2SubdeviceFormat format{};\n+ format.mbus_code = pipeConfig->code;\n+ format.size = data->sensor_->resolution();\n\n ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n if (ret < 0)\n\n\nOn Fri, Nov 26, 2021 at 10:40:38AM +0000, David Plowman wrote:\n> Hi everyone\n>\n> Here's version 7 of the colour space patches.\n>\n> Mostly it's just tidying up, improving documentation and making things\n> a bit clearer, so thank you very much for all those suggestions.\n>\n> The other change is that I've decided to leave it up to the caller to\n> check what has happened when you try to set a colour space. As some\n> have pointed out, some pads already don't have colour spaces, and\n> there might be more in future. I think it's only really the pipeline\n> handler that understands this context, so it's best so leave it to the\n> pipeline handler to check.\n>\n> I also decided to leave the to/fromColorSpace methods public. I could\n> imagine a day in the future when a pipeline handler might want to\n> check (for example) that V4L2 understands the ColorSpace it wants to\n> use, and these methods give a handy way to do that.\n>\n> Thanks again for all the reviews so far!\n>\n> Best regards\n> David\n>\n> David Plowman (7):\n> libcamera: Add ColorSpace class\n> libcamera: Add ColorSpace fields to StreamConfiguration\n> libcamera: Convert between ColorSpace class and V4L2 formats\n> libcamera: Support passing ColorSpaces to V4L2 video devices\n> libcamera: Support passing ColorSpaces to V4L2 subdevices\n> libcamera: Add validateColorSpaces to CameraConfiguration class\n> libcamera: pipeline: raspberrypi: Support color spaces\n>\n> include/libcamera/camera.h | 2 +\n> include/libcamera/color_space.h | 72 +++++\n> include/libcamera/internal/v4l2_device.h | 7 +\n> include/libcamera/internal/v4l2_subdevice.h | 2 +\n> include/libcamera/internal/v4l2_videodevice.h | 2 +\n> include/libcamera/meson.build | 1 +\n> include/libcamera/stream.h | 3 +\n> src/libcamera/camera.cpp | 38 +++\n> src/libcamera/camera_sensor.cpp | 1 +\n> src/libcamera/color_space.cpp | 305 ++++++++++++++++++\n> src/libcamera/meson.build | 1 +\n> .../pipeline/raspberrypi/raspberrypi.cpp | 42 +++\n> src/libcamera/stream.cpp | 19 ++\n> src/libcamera/v4l2_device.cpp | 192 +++++++++++\n> src/libcamera/v4l2_subdevice.cpp | 25 +-\n> src/libcamera/v4l2_videodevice.cpp | 32 ++\n> 16 files changed, 743 insertions(+), 1 deletion(-)\n> create mode 100644 include/libcamera/color_space.h\n> create mode 100644 src/libcamera/color_space.cpp\n>\n> --\n> 2.30.2\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 077BABF415\n\tfor ;\n\tWed, 1 Dec 2021 13:20:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8B357604FC;\n\tWed, 1 Dec 2021 14:20:52 +0100 (CET)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[217.70.178.230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 489BF6011A\n\tfor ;\n\tWed, 1 Dec 2021 14:20:51 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay10.mail.gandi.net (Postfix) with ESMTPSA id DAB08240002;\n\tWed, 1 Dec 2021 13:20:48 +0000 (UTC)"],"Date":"Wed, 1 Dec 2021 14:21:40 +0100","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20211201132140.vl6lmskj6palfj6u@uno.localdomain>","References":"<20211126104045.4756-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211126104045.4756-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v7 0/7] Colour spaces","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":"Tomasz Figa , libcamera-devel@lists.libcamera.org,\n\tHans Verkuil ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21528,"web_url":"https://patchwork.libcamera.org/comment/21528/","msgid":"","date":"2021-12-01T14:06:04","subject":"Re: [libcamera-devel] [PATCH v7 0/7] Colour spaces","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nAh, thanks for those fixes. I must have turned off building all that\nother stuff at some point. Let me fold those into version 8 with your\nother suggestions and then I'll post that.\n\nBest regards\n\nDavid\n\nOn Wed, 1 Dec 2021 at 13:20, Jacopo Mondi wrote:\n>\n> Hi David,\n> forgot to notify it yesterday, but the series breaks the IPU3 and\n> the Simple pipeline handlers.\n>\n> The following will fix it\n>\n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 59dda56bdd3d..f4e8c6632c2f 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> return {};\n> }\n>\n> - V4L2SubdeviceFormat format{\n> - .mbus_code = bestCode,\n> - .size = bestSize,\n> - };\n> + V4L2SubdeviceFormat format{};\n> + format.mbus_code = bestCode;\n> + format.size = bestSize;\n>\n> return format;\n> }\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 701fb4be0b71..a3108fc0b0f6 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> * formats on the video node.\n> */\n> for (unsigned int code : sensor_->mbusCodes()) {\n> - V4L2SubdeviceFormat format{ code, sensor_->resolution() };\n> + V4L2SubdeviceFormat format{};\n> + format.mbus_code = code;\n> + format.size = sensor_->resolution();\n>\n> ret = setupFormats(&format, V4L2Subdevice::TryFormat);\n> if (ret < 0) {\n> @@ -908,7 +910,9 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> return ret;\n>\n> const SimpleCameraData::Configuration *pipeConfig = config->pipeConfig();\n> - V4L2SubdeviceFormat format{ pipeConfig->code, data->sensor_->resolution() };\n> + V4L2SubdeviceFormat format{};\n> + format.mbus_code = pipeConfig->code;\n> + format.size = data->sensor_->resolution();\n>\n> ret = data->setupFormats(&format, V4L2Subdevice::ActiveFormat);\n> if (ret < 0)\n>\n>\n> On Fri, Nov 26, 2021 at 10:40:38AM +0000, David Plowman wrote:\n> > Hi everyone\n> >\n> > Here's version 7 of the colour space patches.\n> >\n> > Mostly it's just tidying up, improving documentation and making things\n> > a bit clearer, so thank you very much for all those suggestions.\n> >\n> > The other change is that I've decided to leave it up to the caller to\n> > check what has happened when you try to set a colour space. As some\n> > have pointed out, some pads already don't have colour spaces, and\n> > there might be more in future. I think it's only really the pipeline\n> > handler that understands this context, so it's best so leave it to the\n> > pipeline handler to check.\n> >\n> > I also decided to leave the to/fromColorSpace methods public. I could\n> > imagine a day in the future when a pipeline handler might want to\n> > check (for example) that V4L2 understands the ColorSpace it wants to\n> > use, and these methods give a handy way to do that.\n> >\n> > Thanks again for all the reviews so far!\n> >\n> > Best regards\n> > David\n> >\n> > David Plowman (7):\n> > libcamera: Add ColorSpace class\n> > libcamera: Add ColorSpace fields to StreamConfiguration\n> > libcamera: Convert between ColorSpace class and V4L2 formats\n> > libcamera: Support passing ColorSpaces to V4L2 video devices\n> > libcamera: Support passing ColorSpaces to V4L2 subdevices\n> > libcamera: Add validateColorSpaces to CameraConfiguration class\n> > libcamera: pipeline: raspberrypi: Support color spaces\n> >\n> > include/libcamera/camera.h | 2 +\n> > include/libcamera/color_space.h | 72 +++++\n> > include/libcamera/internal/v4l2_device.h | 7 +\n> > include/libcamera/internal/v4l2_subdevice.h | 2 +\n> > include/libcamera/internal/v4l2_videodevice.h | 2 +\n> > include/libcamera/meson.build | 1 +\n> > include/libcamera/stream.h | 3 +\n> > src/libcamera/camera.cpp | 38 +++\n> > src/libcamera/camera_sensor.cpp | 1 +\n> > src/libcamera/color_space.cpp | 305 ++++++++++++++++++\n> > src/libcamera/meson.build | 1 +\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 42 +++\n> > src/libcamera/stream.cpp | 19 ++\n> > src/libcamera/v4l2_device.cpp | 192 +++++++++++\n> > src/libcamera/v4l2_subdevice.cpp | 25 +-\n> > src/libcamera/v4l2_videodevice.cpp | 32 ++\n> > 16 files changed, 743 insertions(+), 1 deletion(-)\n> > create mode 100644 include/libcamera/color_space.h\n> > create mode 100644 src/libcamera/color_space.cpp\n> >\n> > --\n> > 2.30.2\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 444FABF415\n\tfor ;\n\tWed, 1 Dec 2021 14:06:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC08F6071A;\n\tWed, 1 Dec 2021 15:06:16 +0100 (CET)","from mail-wm1-x335.google.com (mail-wm1-x335.google.com\n\t[IPv6:2a00:1450:4864:20::335])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C74726011A\n\tfor ;\n\tWed, 1 Dec 2021 15:06:15 +0100 (CET)","by mail-wm1-x335.google.com with SMTP id\n\to19-20020a1c7513000000b0033a93202467so1230042wmc.2\n\tfor ;\n\tWed, 01 Dec 2021 06:06:15 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Vy2yqXiY\"; 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=ZY0N6CgHFVU6vG+E1XcPfqWDfdedCmEV7CR5VXj1wAM=;\n\tb=Vy2yqXiYo7Ep60xalPf9cqKOJbHosxzFzAK69BgMIK3/ixE/QLPJ7/Y3ROZw5DDfok\n\tvg2ziEbwgx5ZttW983lqQyyAjk5NZJ5B/KVvG6aDiGSCpfGYYfqr/w9dAIwvvmOibbO9\n\tdYIuKgH/fcOC6kinP8ROpDA7lcollRZgkX3nk/163KNL+w9BN8tp8hHoEvjZ6Wu2UxmX\n\tOU9lXtkNhASUI1vlBhMECDVgDTiSfcr2rtzxkGBJTVOYpOJ7dwlhSaMTmJLi8+1TQOLP\n\tXTqMkdoMDcvcwR5ACsHtB/3K1jPxYAKR/sWcDFDg+u/SbjR/ErkOAW7iY3qsA/4Vk/3f\n\tFCHw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=ZY0N6CgHFVU6vG+E1XcPfqWDfdedCmEV7CR5VXj1wAM=;\n\tb=6uKNjmrUNkkrV5WnPn3TmQZYiGcgKm0kmTEl1zUh7rDQW3cLYbiVT3wH6Vyd4KRSdV\n\t4yh6V0WRj4BS8wbDpJG8xwcyFmzlkgcckyaO9l/KcLlhk4Qbk7pWbehX6cIp1jO+yFUp\n\t3bvtyOzk8Q2ixTn8BV34OVgsupqb7l75hvMKbtRgYklg+KUXQatdbg+sqRxyd1C0a2ny\n\teOReWneG+qBhq3m2AEUyT/T41SgsxJdKCAVl896pE3eBi5d6tJwQPUd/2HWaoakR4v1B\n\tjGT4YSyAz2bxHMwwweITLAlmkQqOwTIwpUxi0SrQ4XjvuGynuCkVQ0Rb2WtB9Paj2wBY\n\tWgqQ==","X-Gm-Message-State":"AOAM533bNvZ2UBwIkrPEn6qsQY484yN7iPOFFfJnQMlGSqZJ7qhT7+SD\n\t4h+JOtcRBE4y3CoSPaluA7a4hdjDRkzX5p2IQ3gQkw==","X-Google-Smtp-Source":"ABdhPJyXQX17+7FPbzYf9Q9tIowHBEtcNj/Vdj+PgbIzglAL5310rC9gPyCRBIercbW3UCkE9akUqOvstNuVHzuLS+0=","X-Received":"by 2002:a7b:ca54:: with SMTP id m20mr7274052wml.21.1638367575376;\n\tWed, 01 Dec 2021 06:06:15 -0800 (PST)","MIME-Version":"1.0","References":"<20211126104045.4756-1-david.plowman@raspberrypi.com>\n\t<20211201132140.vl6lmskj6palfj6u@uno.localdomain>","In-Reply-To":"<20211201132140.vl6lmskj6palfj6u@uno.localdomain>","From":"David Plowman ","Date":"Wed, 1 Dec 2021 14:06:04 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v7 0/7] Colour spaces","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":"Tomasz Figa ,\n\tlibcamera devel ,\n\tHans Verkuil ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]