[{"id":2823,"web_url":"https://patchwork.libcamera.org/comment/2823/","msgid":"<20191008083542.yi3hbfs3vew2jgcd@uno.localdomain>","date":"2019-10-08T08:35:42","subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Oct 08, 2019 at 02:45:29AM +0200, Niklas Söderlund wrote:\n> A new ControlList container is needed to hold metadata coming out of\n> the IPA. The list of supported controls in this list is expected to\n> grow, so for now do not add a validator for the list.\n>\n> Signed-off-by: Niklas Söderlund \n> Reviewed-by: Laurent Pinchart \n> ---\n> include/libcamera/request.h | 2 ++\n> src/libcamera/request.cpp | 14 ++++++++++++++\n> 2 files changed, 16 insertions(+)\n>\n> diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> index e3db5243aaf3cf30..2d5a5964e99eb75f 100644\n> --- a/include/libcamera/request.h\n> +++ b/include/libcamera/request.h\n> @@ -37,6 +37,7 @@ public:\n> \t~Request();\n>\n> \tControlList &controls() { return *controls_; }\n> +\tControlList &metadata() { return *metadata_; }\n\nJust thinking out loud, should the returned & be a const ? Might\napplication want to change the content of the metadata array ?\n\nThanks\n j\n> \tconst std::map &buffers() const { return bufferMap_; }\n> \tint addBuffer(std::unique_ptr buffer);\n> \tBuffer *findBuffer(Stream *stream) const;\n> @@ -58,6 +59,7 @@ private:\n> \tCamera *camera_;\n> \tCameraControlValidator *validator_;\n> \tControlList *controls_;\n> +\tControlList *metadata_;\n> \tstd::map bufferMap_;\n> \tstd::unordered_set pending_;\n>\n> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> index 19f6d0b9a0aeb03c..23d3ab6f422c406e 100644\n> --- a/src/libcamera/request.cpp\n> +++ b/src/libcamera/request.cpp\n> @@ -65,6 +65,11 @@ Request::Request(Camera *camera, uint64_t cookie)\n> \t */\n> \tvalidator_ = new CameraControlValidator(camera);\n> \tcontrols_ = new ControlList(validator_);\n> +\n> +\t/**\n> +\t * \\todo: Add a validator for metadata controls.\n> +\t */\n> +\tmetadata_ = new ControlList(nullptr);\n> }\n>\n> Request::~Request()\n> @@ -74,6 +79,7 @@ Request::~Request()\n> \t\tdelete buffer;\n> \t}\n>\n> +\tdelete metadata_;\n> \tdelete controls_;\n> \tdelete validator_;\n> }\n> @@ -161,6 +167,14 @@ Buffer *Request::findBuffer(Stream *stream) const\n> \treturn it->second;\n> }\n>\n> +/**\n> + * \\fn Request::metadata()\n> + * \\brief Retrieve the request's metadata\n> + * \\todo Offer a read-only API towards applications while keeping a read/write\n> + * API internally.\n> + * \\return The metadata associated with the request\n> + */\n> +\n> /**\n> * \\fn Request::cookie()\n> * \\brief Retrieve the cookie set when the request was created\n> --\n> 2.23.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A30FE60BC6\n\tfor ;\n\tTue, 8 Oct 2019 10:33:56 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 3D6581BF220;\n\tTue, 8 Oct 2019 08:33:55 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 8 Oct 2019 10:35:42 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191008083542.yi3hbfs3vew2jgcd@uno.localdomain>","References":"<20191008004534.1585142-1-niklas.soderlund@ragnatech.se>\n\t<20191008004534.1585142-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2fnec42xwij2hivg\"","Content-Disposition":"inline","In-Reply-To":"<20191008004534.1585142-6-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","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","X-List-Received-Date":"Tue, 08 Oct 2019 08:33:56 -0000"}},{"id":2831,"web_url":"https://patchwork.libcamera.org/comment/2831/","msgid":"<20191008112219.GF16543@pendragon.ideasonboard.com>","date":"2019-10-08T11:22:19","subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Tue, Oct 08, 2019 at 10:35:42AM +0200, Jacopo Mondi wrote:\n> On Tue, Oct 08, 2019 at 02:45:29AM +0200, Niklas Söderlund wrote:\n> > A new ControlList container is needed to hold metadata coming out of\n> > the IPA. The list of supported controls in this list is expected to\n> > grow, so for now do not add a validator for the list.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > include/libcamera/request.h | 2 ++\n> > src/libcamera/request.cpp | 14 ++++++++++++++\n> > 2 files changed, 16 insertions(+)\n> >\n> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > index e3db5243aaf3cf30..2d5a5964e99eb75f 100644\n> > --- a/include/libcamera/request.h\n> > +++ b/include/libcamera/request.h\n> > @@ -37,6 +37,7 @@ public:\n> > \t~Request();\n> >\n> > \tControlList &controls() { return *controls_; }\n> > +\tControlList &metadata() { return *metadata_; }\n> \n> Just thinking out loud, should the returned & be a const ? Might\n> application want to change the content of the metadata array ?\n\nApplications shouldn't change it, but pipeline handlers need write\naccess to the metadata. I think we need a better interface for this, in\norder to set tighter permissions for applications, but that's a broad\nissue with the current code base, so I don't think it needs to be fixed\nas part of this patch series.\n\n> > \tconst std::map &buffers() const { return bufferMap_; }\n> > \tint addBuffer(std::unique_ptr buffer);\n> > \tBuffer *findBuffer(Stream *stream) const;\n> > @@ -58,6 +59,7 @@ private:\n> > \tCamera *camera_;\n> > \tCameraControlValidator *validator_;\n> > \tControlList *controls_;\n> > +\tControlList *metadata_;\n> > \tstd::map bufferMap_;\n> > \tstd::unordered_set pending_;\n> >\n> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > index 19f6d0b9a0aeb03c..23d3ab6f422c406e 100644\n> > --- a/src/libcamera/request.cpp\n> > +++ b/src/libcamera/request.cpp\n> > @@ -65,6 +65,11 @@ Request::Request(Camera *camera, uint64_t cookie)\n> > \t */\n> > \tvalidator_ = new CameraControlValidator(camera);\n> > \tcontrols_ = new ControlList(validator_);\n> > +\n> > +\t/**\n> > +\t * \\todo: Add a validator for metadata controls.\n> > +\t */\n> > +\tmetadata_ = new ControlList(nullptr);\n> > }\n> >\n> > Request::~Request()\n> > @@ -74,6 +79,7 @@ Request::~Request()\n> > \t\tdelete buffer;\n> > \t}\n> >\n> > +\tdelete metadata_;\n> > \tdelete controls_;\n> > \tdelete validator_;\n> > }\n> > @@ -161,6 +167,14 @@ Buffer *Request::findBuffer(Stream *stream) const\n> > \treturn it->second;\n> > }\n> >\n> > +/**\n> > + * \\fn Request::metadata()\n> > + * \\brief Retrieve the request's metadata\n> > + * \\todo Offer a read-only API towards applications while keeping a read/write\n> > + * API internally.\n> > + * \\return The metadata associated with the request\n> > + */\n> > +\n> > /**\n> > * \\fn Request::cookie()\n> > * \\brief Retrieve the cookie set when the request was created","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A31F861565\n\tfor ;\n\tTue, 8 Oct 2019 13:22:22 +0200 (CEST)","from pendragon.ideasonboard.com (unknown [80.194.85.162])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1B77755F;\n\tTue, 8 Oct 2019 13:22:22 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570533742;\n\tbh=pEjv7Ub0yt4Pj4BgOlUJZN6YLBZNtHOtxMHPn2Mtfk8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NtvtvOH8/N/EYIcFzaTBaDus21SlIHbey+t30qETo8RYGIgXuR3f9fLFfOjKWH3gu\n\tYaQsEXRdqvytBRzRQhKLwn0xNAm9U49tbzJwI9KUjS/FKmswZ3C2qxq14S5JtQ4JRH\n\tdraALMv3Z/H3Uq9EWX6iUfLf/K99j0wrkM7ZuMeA=","Date":"Tue, 8 Oct 2019 14:22:19 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20191008112219.GF16543@pendragon.ideasonboard.com>","References":"<20191008004534.1585142-1-niklas.soderlund@ragnatech.se>\n\t<20191008004534.1585142-6-niklas.soderlund@ragnatech.se>\n\t<20191008083542.yi3hbfs3vew2jgcd@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191008083542.yi3hbfs3vew2jgcd@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","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","X-List-Received-Date":"Tue, 08 Oct 2019 11:22:22 -0000"}},{"id":2857,"web_url":"https://patchwork.libcamera.org/comment/2857/","msgid":"<20191009231238.GB32293@bigcity.dyn.berto.se>","date":"2019-10-09T23:12:38","subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi,\n\nOn 2019-10-08 14:22:19 +0300, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> On Tue, Oct 08, 2019 at 10:35:42AM +0200, Jacopo Mondi wrote:\n> > On Tue, Oct 08, 2019 at 02:45:29AM +0200, Niklas Söderlund wrote:\n> > > A new ControlList container is needed to hold metadata coming out of\n> > > the IPA. The list of supported controls in this list is expected to\n> > > grow, so for now do not add a validator for the list.\n> > >\n> > > Signed-off-by: Niklas Söderlund \n> > > Reviewed-by: Laurent Pinchart \n> > > ---\n> > > include/libcamera/request.h | 2 ++\n> > > src/libcamera/request.cpp | 14 ++++++++++++++\n> > > 2 files changed, 16 insertions(+)\n> > >\n> > > diff --git a/include/libcamera/request.h b/include/libcamera/request.h\n> > > index e3db5243aaf3cf30..2d5a5964e99eb75f 100644\n> > > --- a/include/libcamera/request.h\n> > > +++ b/include/libcamera/request.h\n> > > @@ -37,6 +37,7 @@ public:\n> > > \t~Request();\n> > >\n> > > \tControlList &controls() { return *controls_; }\n> > > +\tControlList &metadata() { return *metadata_; }\n> > \n> > Just thinking out loud, should the returned & be a const ? Might\n> > application want to change the content of the metadata array ?\n> \n> Applications shouldn't change it, but pipeline handlers need write\n> access to the metadata. I think we need a better interface for this, in\n> order to set tighter permissions for applications, but that's a broad\n> issue with the current code base, so I don't think it needs to be fixed\n> as part of this patch series.\n\nI agree with both of you. The meta data should not be able to write to \nthe Requests metadata. I have noted a todo in the documentation for \nRequest::metadata() to highlight this already ;-)\n\n> \n> > > \tconst std::map &buffers() const { return bufferMap_; }\n> > > \tint addBuffer(std::unique_ptr buffer);\n> > > \tBuffer *findBuffer(Stream *stream) const;\n> > > @@ -58,6 +59,7 @@ private:\n> > > \tCamera *camera_;\n> > > \tCameraControlValidator *validator_;\n> > > \tControlList *controls_;\n> > > +\tControlList *metadata_;\n> > > \tstd::map bufferMap_;\n> > > \tstd::unordered_set pending_;\n> > >\n> > > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp\n> > > index 19f6d0b9a0aeb03c..23d3ab6f422c406e 100644\n> > > --- a/src/libcamera/request.cpp\n> > > +++ b/src/libcamera/request.cpp\n> > > @@ -65,6 +65,11 @@ Request::Request(Camera *camera, uint64_t cookie)\n> > > \t */\n> > > \tvalidator_ = new CameraControlValidator(camera);\n> > > \tcontrols_ = new ControlList(validator_);\n> > > +\n> > > +\t/**\n> > > +\t * \\todo: Add a validator for metadata controls.\n> > > +\t */\n> > > +\tmetadata_ = new ControlList(nullptr);\n> > > }\n> > >\n> > > Request::~Request()\n> > > @@ -74,6 +79,7 @@ Request::~Request()\n> > > \t\tdelete buffer;\n> > > \t}\n> > >\n> > > +\tdelete metadata_;\n> > > \tdelete controls_;\n> > > \tdelete validator_;\n> > > }\n> > > @@ -161,6 +167,14 @@ Buffer *Request::findBuffer(Stream *stream) const\n> > > \treturn it->second;\n> > > }\n> > >\n> > > +/**\n> > > + * \\fn Request::metadata()\n> > > + * \\brief Retrieve the request's metadata\n> > > + * \\todo Offer a read-only API towards applications while keeping a read/write\n> > > + * API internally.\n> > > + * \\return The metadata associated with the request\n> > > + */\n> > > +\n> > > /**\n> > > * \\fn Request::cookie()\n> > > * \\brief Retrieve the cookie set when the request was created\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x244.google.com (mail-lj1-x244.google.com\n\t[IPv6:2a00:1450:4864:20::244])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 337526157C\n\tfor ;\n\tThu, 10 Oct 2019 01:12:41 +0200 (CEST)","by mail-lj1-x244.google.com with SMTP id a22so4230794ljd.0\n\tfor ;\n\tWed, 09 Oct 2019 16:12:41 -0700 (PDT)","from localhost (h-93-159.A463.priv.bahnhof.se. [46.59.93.159])\n\tby smtp.gmail.com with ESMTPSA id\n\tz20sm846344ljk.63.2019.10.09.16.12.38\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 09 Oct 2019 16:12:39 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=fjedYo7ic/eBOb/HxxVI51DDhKqfTjQ0poWBBeBB3Jk=;\n\tb=xTXbkAJHzw13PhrNGeCbTYw2LODyjhSoKwo4xV201iDT7VxY+i4/yUBGiAuFKorpku\n\tm98JBkYPH081FPQkBuD5COoCKy/Qy673K+iQZxACNrLZJnczkE62OawExvu8qfwZQr7m\n\tM2lVxTFKhAuRWur4zbXTWlsLu0GkQOHeakejM6Tvj/LMFIanAfIqWZJtBD+asc2Vpfo8\n\t/JFFig3v6ZMTYsVkG96JooosgF6SLFqQdJKuxyz4FTr8aBePxX1yqpcypSSxxd29CaFG\n\tJfqxginX0wxvqWbzMUEFEOXv+aLZemmbI9jf9koiYaBDcP4PiXtAIss7nxL0yRoJucRi\n\tuLSA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=fjedYo7ic/eBOb/HxxVI51DDhKqfTjQ0poWBBeBB3Jk=;\n\tb=AHyVko0IehREydn6u9ZTobbMvFEnMkMGos2kxPVacB7dICqGZLyCra4Nd5gTJtgg+m\n\tePXUj8Vrjd6B1bfEUCN8r+9lRCvNzkcfES5G9o/Yn2ZqPen139h/9ah3PF6Sqluupd4P\n\ty7mnu03uecIYF+AlTTYn9t0xw7JPl87EK2MruBHtktGHUYTiKan3bdZXL/atAavw2WYT\n\taq2zSzzaqZtvdxcLYdnyU4+NpSH5mqPH9i0JvCr3FOOtmNjtadLruunIee63bbKMBpHM\n\tMCJ60e90ZepuJj4RVVRaJ/XHAwj/mvrzHs+puMq8AJzxJ8+zAGggOXylQEGTtVPLD9t9\n\tptvA==","X-Gm-Message-State":"APjAAAWliNtNfxDcM55i6X8K3i48bJkfRaAzMlNUVj2l/UbpoGvvkPm1\n\tMqAr8+guOR0e7x3XEDeqGI5RtA==","X-Google-Smtp-Source":"APXvYqzoGqu8z+0VGIGYbXzzfhVpn5ppHZFZomA2Z2RVt1QscIwYe008rjg9HJxeRm+kyOX9aaeM0A==","X-Received":"by 2002:a2e:6a04:: with SMTP id f4mr3698746ljc.164.1570662760360;\n\tWed, 09 Oct 2019 16:12:40 -0700 (PDT)","Date":"Thu, 10 Oct 2019 01:12:38 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20191009231238.GB32293@bigcity.dyn.berto.se>","References":"<20191008004534.1585142-1-niklas.soderlund@ragnatech.se>\n\t<20191008004534.1585142-6-niklas.soderlund@ragnatech.se>\n\t<20191008083542.yi3hbfs3vew2jgcd@uno.localdomain>\n\t<20191008112219.GF16543@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191008112219.GF16543@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH v5 05/10] libcamera: request: Add\n\tmetadata information","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","X-List-Received-Date":"Wed, 09 Oct 2019 23:12:41 -0000"}}]