[{"id":21111,"web_url":"https://patchwork.libcamera.org/comment/21111/","msgid":"<068f487e-aef8-2341-d234-1177cecdb517@ideasonboard.com>","date":"2021-11-23T07:23:35","subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch\n\nOn 11/18/21 8:49 PM, David Plowman wrote:\n> This is so that applications can choose appropriate color spaces which\n> will then be passed down to the V4L2 devices.\n\n\nWas expecting a plumbing to cam here, but it might follow up in later in \nthe series or on top. Anyways not a blocker so\n\nReviewed-by: Umang Jain \n\n> The ColorSpace field is actually optional. If it is not set you will\n> get the driver's default color space.\n>\n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/stream.h | 3 +++\n> src/libcamera/stream.cpp | 14 ++++++++++++++\n> 2 files changed, 17 insertions(+)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 0c55e716..bea88eb4 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -47,6 +48,8 @@ struct StreamConfiguration {\n> \n> \tunsigned int bufferCount;\n> \n> +\tstd::optional colorSpace;\n> +\n> \tStream *stream() const { return stream_; }\n> \tvoid setStream(Stream *stream) { stream_ = stream; }\n> \tconst StreamFormats &formats() const { return formats_; }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index b421e17e..300b3af7 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -329,6 +329,20 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> * \\brief Requested number of buffers to allocate for the stream\n> */\n> \n> +/**\n> + * \\var StreamConfiguration::colorSpace\n> + * \\brief The ColorSpace for this stream\n> + *\n> + * A suitable color space may be set here or chosen by an application.\n> + * Alternatively the color space may be left unset, in which case it will\n> + * be up to the driver to choose a color space.\n> + *\n> + * If the system cannot deliver the requested color space, the validate()\n> + * method will overwrite the value here with what it can deliver. Note that\n> + * platforms may enforce extra contraints here, such as requiring all output\n> + * streams to share the same color space.\n> + */\n> +\n> /**\n> * \\fn StreamConfiguration::stream()\n> * \\brief Retrieve the stream associated with the configuration","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 4548FBF415\n\tfor ;\n\tTue, 23 Nov 2021 07:23:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 866FB6036F;\n\tTue, 23 Nov 2021 08:23:41 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2881A60227\n\tfor ;\n\tTue, 23 Nov 2021 08:23:40 +0100 (CET)","from [192.168.1.106] (unknown [103.251.226.81])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6B7CAA1B\n\tfor ;\n\tTue, 23 Nov 2021 08:23:39 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fZ7MH8Y8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637652219;\n\tbh=HgDtMtshwcmgzutuzeYRQTo/fbE4u5kJnEW17pd6gcQ=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=fZ7MH8Y8BJvTaQqNk1plxcfGD+zoNeG8+oFF1qZsAbqRn5RJMoSt1MVdv8clZ5nJs\n\tpacK1UnjWnukFweJHob2QJwNNu3FYzS+lNByRHNFjTlvkuSmL00wxpdekCFDd+7q7i\n\ttuux77MS0eYu1PyLr1aUhdeojzhw8HeDODaC/lSs=","To":"libcamera-devel@lists.libcamera.org","References":"<20211118151933.15627-1-david.plowman@raspberrypi.com>\n\t<20211118151933.15627-3-david.plowman@raspberrypi.com>","From":"Umang Jain ","Message-ID":"<068f487e-aef8-2341-d234-1177cecdb517@ideasonboard.com>","Date":"Tue, 23 Nov 2021 12:53:35 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.10.2","MIME-Version":"1.0","In-Reply-To":"<20211118151933.15627-3-david.plowman@raspberrypi.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Transfer-Encoding":"7bit","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":21207,"web_url":"https://patchwork.libcamera.org/comment/21207/","msgid":"<20211124233926.g6vblvg3b5skfagr@uno.localdomain>","date":"2021-11-24T23:39:26","subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi David,\n\nOn Thu, Nov 18, 2021 at 03:19:28PM +0000, David Plowman wrote:\n> This is so that applications can choose appropriate color spaces which\n> will then be passed down to the V4L2 devices.\n>\n> The ColorSpace field is actually optional. If it is not set you will\n> get the driver's default color space.\n>\n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/stream.h | 3 +++\n> src/libcamera/stream.cpp | 14 ++++++++++++++\n> 2 files changed, 17 insertions(+)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index 0c55e716..bea88eb4 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n>\n> +#include \n> #include \n> #include \n> #include \n> @@ -47,6 +48,8 @@ struct StreamConfiguration {\n>\n> \tunsigned int bufferCount;\n>\n> +\tstd::optional colorSpace;\n> +\n> \tStream *stream() const { return stream_; }\n> \tvoid setStream(Stream *stream) { stream_ = stream; }\n> \tconst StreamFormats &formats() const { return formats_; }\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index b421e17e..300b3af7 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -329,6 +329,20 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> * \\brief Requested number of buffers to allocate for the stream\n> */\n>\n> +/**\n> + * \\var StreamConfiguration::colorSpace\n> + * \\brief The ColorSpace for this stream\n> + *\n> + * A suitable color space may be set here or chosen by an application.\n\nWhat is the difference between \"set here\" OR \"chosen by an application\" ?\nAren't application in charge to select a ColorSpace in the\nconfiguration ?\n\n\n> + * Alternatively the color space may be left unset, in which case it will\n> + * be up to the driver to choose a color space.\n\nI wil model this from the application point of view.\n\n* This field allow to select a ColorSpace to use for the Stream.\n*\n* The field is optional and application can chose to leave it unset.\n* The library will adjust it to what the Camera can provide and the\n* CameraConfiguration this StreamConfiguration is part of will be\n* adjusted.\n\n> + *\n> + * If the system cannot deliver the requested color space, the validate()\n> + * method will overwrite the value here with what it can deliver. Note that\n> + * platforms may enforce extra contraints here, such as requiring all output\n> + * streams to share the same color space.\n\nWhat about something along the lines of:\n\n* If a specific ColorSpace is requested here but the Camera cannot\n* produce it, the StreamConfiguration will be Adjusted to what can be\n* delivered, as there might be platform specific contraints to respect\n* such as all output streams sharing the same color space.\n\nFeel free to adjust, as a native speaker you'll surely do better.\n\nThanks\n j\n\n> + */\n> +\n> /**\n> * \\fn StreamConfiguration::stream()\n> * \\brief Retrieve the stream associated with the configuration\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 93D96BDB13\n\tfor ;\n\tWed, 24 Nov 2021 23:38:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8E0C60371;\n\tThu, 25 Nov 2021 00:38:36 +0100 (CET)","from relay11.mail.gandi.net (relay11.mail.gandi.net\n\t[217.70.178.231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 92B8360228\n\tfor ;\n\tThu, 25 Nov 2021 00:38:35 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay11.mail.gandi.net (Postfix) with ESMTPSA id EFBAE100005;\n\tWed, 24 Nov 2021 23:38:33 +0000 (UTC)"],"Date":"Thu, 25 Nov 2021 00:39:26 +0100","From":"Jacopo Mondi ","To":"David Plowman ","Message-ID":"<20211124233926.g6vblvg3b5skfagr@uno.localdomain>","References":"<20211118151933.15627-1-david.plowman@raspberrypi.com>\n\t<20211118151933.15627-3-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20211118151933.15627-3-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","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":21232,"web_url":"https://patchwork.libcamera.org/comment/21232/","msgid":"","date":"2021-11-25T11:21:25","subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Jacopo\n\nThanks for the review!\n\nOn Wed, 24 Nov 2021 at 23:38, Jacopo Mondi wrote:\n>\n> Hi David,\n>\n> On Thu, Nov 18, 2021 at 03:19:28PM +0000, David Plowman wrote:\n> > This is so that applications can choose appropriate color spaces which\n> > will then be passed down to the V4L2 devices.\n> >\n> > The ColorSpace field is actually optional. If it is not set you will\n> > get the driver's default color space.\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > include/libcamera/stream.h | 3 +++\n> > src/libcamera/stream.cpp | 14 ++++++++++++++\n> > 2 files changed, 17 insertions(+)\n> >\n> > diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> > index 0c55e716..bea88eb4 100644\n> > --- a/include/libcamera/stream.h\n> > +++ b/include/libcamera/stream.h\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> >\n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -47,6 +48,8 @@ struct StreamConfiguration {\n> >\n> > unsigned int bufferCount;\n> >\n> > + std::optional 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/stream.cpp b/src/libcamera/stream.cpp\n> > index b421e17e..300b3af7 100644\n> > --- a/src/libcamera/stream.cpp\n> > +++ b/src/libcamera/stream.cpp\n> > @@ -329,6 +329,20 @@ StreamConfiguration::StreamConfiguration(const StreamFormats &formats)\n> > * \\brief Requested number of buffers to allocate for the stream\n> > */\n> >\n> > +/**\n> > + * \\var StreamConfiguration::colorSpace\n> > + * \\brief The ColorSpace for this stream\n> > + *\n> > + * A suitable color space may be set here or chosen by an application.\n>\n> What is the difference between \"set here\" OR \"chosen by an application\" ?\n> Aren't application in charge to select a ColorSpace in the\n> configuration ?\n\nWell, no great difference really. I suppose a pipeline handler may or\nmay not set a value. Then the application code may or may not\noverwrite this.\n\n>\n>\n> > + * Alternatively the color space may be left unset, in which case it will\n> > + * be up to the driver to choose a color space.\n>\n> I wil model this from the application point of view.\n>\n> * This field allow to select a ColorSpace to use for the Stream.\n> *\n> * The field is optional and application can chose to leave it unset.\n> * The library will adjust it to what the Camera can provide and the\n> * CameraConfiguration this StreamConfiguration is part of will be\n> * adjusted.\n\nOf course, a pipeline handler may (or may not) provide a value here\ntoo. But I can try and re-word it to be a bit more like this!\n\n>\n> > + *\n> > + * If the system cannot deliver the requested color space, the validate()\n> > + * method will overwrite the value here with what it can deliver. Note that\n> > + * platforms may enforce extra contraints here, such as requiring all output\n> > + * streams to share the same color space.\n>\n> What about something along the lines of:\n>\n> * If a specific ColorSpace is requested here but the Camera cannot\n> * produce it, the StreamConfiguration will be Adjusted to what can be\n> * delivered, as there might be platform specific contraints to respect\n> * such as all output streams sharing the same color space.\n>\n> Feel free to adjust, as a native speaker you'll surely do better.\n\nYes, I'll try and adopt some of this in the next revision.\n\nThanks!\nDavid\n\n>\n> Thanks\n> j\n>\n> > + */\n> > +\n> > /**\n> > * \\fn StreamConfiguration::stream()\n> > * \\brief Retrieve the stream associated with the configuration\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 902AEBDB13\n\tfor ;\n\tThu, 25 Nov 2021 11:21:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AC69E6036F;\n\tThu, 25 Nov 2021 12:21:38 +0100 (CET)","from mail-wm1-x329.google.com (mail-wm1-x329.google.com\n\t[IPv6:2a00:1450:4864:20::329])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E69A60231\n\tfor ;\n\tThu, 25 Nov 2021 12:21:37 +0100 (CET)","by mail-wm1-x329.google.com with SMTP id\n\tj140-20020a1c2392000000b003399ae48f58so8015868wmj.5\n\tfor ;\n\tThu, 25 Nov 2021 03:21:37 -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=\"l4kBHQxM\"; 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=OikqsIoS0td+bPLb3jIARuJwtJV2iEPGneVunE6FOII=;\n\tb=l4kBHQxM5Q1Y1KJuIqOmm6t49aMkiQtZ8J1N1d8e/9c4Nfnc/xiQHEu7i2J3hAH8l3\n\tO6U10KRzCpg+d4o8jb8mbqBLzLXfmtjThrP8otMYGxFmrrlaEe+z3lidT3z5JtVJ1MJW\n\tqIlzKXR8Rps6+d8QDFaxd7YEstrn9xmD0BRospFVF8qm/AUqKr1wJRRbqhLI09vVPRQb\n\tgC5eXQD220ZqZQgotxXhtaM94+86iP76K7vwXLF+8QBg9HDT4O+u2i/usvc1GRTi1WJV\n\tRtNyhUHCa+CuArZS7LR7ILs944YlEWVrbvwwZA2Ek4dlx1wwWkih27xqNUVgiOvDvhfK\n\tSRUA==","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=OikqsIoS0td+bPLb3jIARuJwtJV2iEPGneVunE6FOII=;\n\tb=ZY+3kVi5dXaVQepK0wXxb+gtOZlYyyd2+pzncqPINpJKwTCvbdpXIRqvB1NmRnNxms\n\toKkvVKDmDPxg9+zTZg2oMZu3fbskjCu7IbMwNK6VdQ0wttHPqfv9tbSGkSBfsBXUUww2\n\tIb8h9vYX1+x7XbA7a5wG2Ap5HdOXOKxbfXBubil//enMAyHjl6QUH8HwY9cO4N9APtBB\n\tZqJoBDw4uEREutczIYPqdicOwNMmHUoBzCEGHsRaUeLn1INsAlHpyyWM7Pp8HOaeYKOw\n\tOpgxRhrey9DxlwuT2BzQ4EwOyPuKzPUxCXgfw/FnPkmvUfGRTR+ZWKbDnMkk7zwLJDDw\n\tzY/w==","X-Gm-Message-State":"AOAM530JRwXqXSqrWxdaMIZEVgJdprmLjsSkr1ZMB3UTQxsOVk6PkaQo\n\tTEGvXf53nLKHcOIYM3H0PmmMh+i7WIyaqjzG1uhiKGab/VJ2Kw==","X-Google-Smtp-Source":"ABdhPJyDPxtxw5p+wlc+xpr2hx8qe8UvBFlfT22Gg0ClpeUFlMdNpnfle4MMBG155QiJPTUsjE2vih7pz2gYuW5Cceo=","X-Received":"by 2002:a05:600c:c1:: with SMTP id\n\tu1mr6152291wmm.163.1637839297176; \n\tThu, 25 Nov 2021 03:21:37 -0800 (PST)","MIME-Version":"1.0","References":"<20211118151933.15627-1-david.plowman@raspberrypi.com>\n\t<20211118151933.15627-3-david.plowman@raspberrypi.com>\n\t<20211124233926.g6vblvg3b5skfagr@uno.localdomain>","In-Reply-To":"<20211124233926.g6vblvg3b5skfagr@uno.localdomain>","From":"David Plowman ","Date":"Thu, 25 Nov 2021 11:21:25 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v6 2/7] libcamera: Add ColorSpace\n\tfields to StreamConfiguration","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\" "}}]