[{"id":4532,"web_url":"https://patchwork.libcamera.org/comment/4532/","msgid":"<20200426151003.62et52rbp5wmtl2p@uno.localdomain>","date":"2020-04-26T15:10:03","subject":"Re: [libcamera-devel] [PATCH 2/2] [DNI] test: Test serialization of\n\tRectangle and Size controls","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent,\n I start from here to comment on the API\n\nOn Sat, Apr 25, 2020 at 11:56:39PM +0300, Laurent Pinchart wrote:\n> This patch should be rebased on real controls once available.\n>\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/control_ids.yaml | 18 ++++++++++++++++++\n> test/serialization/control_serialization.cpp | 5 +++++\n> 2 files changed, 23 insertions(+)\n>\n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index 4befec746a59..b1ae03e5b0ff 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -50,4 +50,22 @@ controls:\n> type: int32_t\n> description: Specify a fixed gain parameter\n>\n> + - TheRectangle:\n> + type: Rectangle\n> + description: A Rectangle property\n> +\n> + - TheRectangles:\n> + type: Rectangle\n> + description: A Rectangle array property\n> + size: [n]\n> +\n> + - TheSize:\n> + type: Size\n> + description: A Size property\n> +\n> + - TheSizes:\n> + type: Size\n> + description: A Size array property\n> + size: [n]\n> +\n> ...\n> diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> index 2989b52774fb..789e0d83f4e4 100644\n> --- a/test/serialization/control_serialization.cpp\n> +++ b/test/serialization/control_serialization.cpp\n> @@ -45,6 +45,11 @@ protected:\n> \t\tlist.set(controls::Brightness, 255);\n> \t\tlist.set(controls::Contrast, 128);\n> \t\tlist.set(controls::Saturation, 50);\n> +\t\tlist.set(controls::TheRectangle, Rectangle{ 100, 100, 640, 480 });\n> +\t\tlist.set(controls::TheRectangles, { Rectangle{ 100, 100, 640, 480 },\n> +\t\t\t\t\t\t Rectangle{ 200, 200, 1280, 720 } });\n> +\t\tlist.set(controls::TheSize, Size{ 640, 480 });\n> +\t\tlist.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });\n\nIs this a good idea ? How easy is that get it wrong assuming\n\n\t\tlist.set(controls::TheSizes, { 640, 480 }};\n\ndoes what one expects ? We'll get a type assertion error, which is not\nnice to debug.. I'm debated, this is useful, but might be really a\nbleeding edge of the API...\n\n>\n> \t\t/*\n> \t\t * Serialize the control list, this should fail as the control\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","Received":["from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 95EDE603FA\n\tfor ;\n\tSun, 26 Apr 2020 17:07:03 +0200 (CEST)","from uno.localdomain\n\t(host170-48-dynamic.3-87-r.retail.telecomitalia.it [87.3.48.170])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 8BFED200007;\n\tSun, 26 Apr 2020 15:07:02 +0000 (UTC)"],"Date":"Sun, 26 Apr 2020 17:10:03 +0200","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200426151003.62et52rbp5wmtl2p@uno.localdomain>","References":"<20200425205639.30566-1-laurent.pinchart@ideasonboard.com>\n\t<20200425205639.30566-2-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200425205639.30566-2-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 2/2] [DNI] test: Test serialization of\n\tRectangle and Size controls","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":"Sun, 26 Apr 2020 15:07:03 -0000"}},{"id":4533,"web_url":"https://patchwork.libcamera.org/comment/4533/","msgid":"<20200426153040.GA5950@pendragon.ideasonboard.com>","date":"2020-04-26T15:30:40","subject":"Re: [libcamera-devel] [PATCH 2/2] [DNI] test: Test serialization of\n\tRectangle and Size controls","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Sun, Apr 26, 2020 at 05:10:03PM +0200, Jacopo Mondi wrote:\n> Hi Laurent,\n> I start from here to comment on the API\n> \n> On Sat, Apr 25, 2020 at 11:56:39PM +0300, Laurent Pinchart wrote:\n> > This patch should be rebased on real controls once available.\n> >\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/control_ids.yaml | 18 ++++++++++++++++++\n> > test/serialization/control_serialization.cpp | 5 +++++\n> > 2 files changed, 23 insertions(+)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index 4befec746a59..b1ae03e5b0ff 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -50,4 +50,22 @@ controls:\n> > type: int32_t\n> > description: Specify a fixed gain parameter\n> >\n> > + - TheRectangle:\n> > + type: Rectangle\n> > + description: A Rectangle property\n> > +\n> > + - TheRectangles:\n> > + type: Rectangle\n> > + description: A Rectangle array property\n> > + size: [n]\n> > +\n> > + - TheSize:\n> > + type: Size\n> > + description: A Size property\n> > +\n> > + - TheSizes:\n> > + type: Size\n> > + description: A Size array property\n> > + size: [n]\n> > +\n> > ...\n> > diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp\n> > index 2989b52774fb..789e0d83f4e4 100644\n> > --- a/test/serialization/control_serialization.cpp\n> > +++ b/test/serialization/control_serialization.cpp\n> > @@ -45,6 +45,11 @@ protected:\n> > \t\tlist.set(controls::Brightness, 255);\n> > \t\tlist.set(controls::Contrast, 128);\n> > \t\tlist.set(controls::Saturation, 50);\n> > +\t\tlist.set(controls::TheRectangle, Rectangle{ 100, 100, 640, 480 });\n> > +\t\tlist.set(controls::TheRectangles, { Rectangle{ 100, 100, 640, 480 },\n> > +\t\t\t\t\t\t Rectangle{ 200, 200, 1280, 720 } });\n> > +\t\tlist.set(controls::TheSize, Size{ 640, 480 });\n> > +\t\tlist.set(controls::TheSizes, { Size{ 640, 480 }, Size{ 1280, 720 } });\n> \n> Is this a good idea ? How easy is that get it wrong assuming\n> \n> \t\tlist.set(controls::TheSizes, { 640, 480 }};\n> \n> does what one expects ? We'll get a type assertion error, which is not\n> nice to debug.. I'm debated, this is useful, but might be really a\n> bleeding edge of the API...\n\nAdding\n\n \t\tlist.set(controls::TheSizes, { 640, 480 }};\n\nthe compiler tells me\n\nIn file included from ../../test/serialization/control_serialization.cpp:10:\nIn file included from ../../include/libcamera/camera.h:15:\n../../include/libcamera/controls.h:393:8: error: no matching member function for call to 'set'\n val->set(Span>{ value.begin(), value.size() });\n ~~~~~^~~~~~\n../../test/serialization/control_serialization.cpp:53:8: note: in instantiation of function template specialization 'libcamera::ControlList::set, int>' requested here\n list.set(controls::TheSizes, { 640, 480 });\n ^\n../../include/libcamera/controls.h:185:7: note: candidate function template not viable: no known conversion from 'Span, [...]>' to 'const Span' for 1st argument\n void set(const T &value)\n ^\n../../include/libcamera/controls.h:173:7: note: candidate template ignored: requirement '!details::is_span >::value' was not satisfied [with T = libcamera::Span]\n void set(const T &value)\n ^\n1 error generated.\n\nThat's the thing I like about the controls API, it has compile-time type\nchecking. We don't get this for V4L2 controls though, as they're\nspecified by integer ID, and I'm really tempted to add Control<>\ninstances for each of the V4L2 controls we need, but that's not for now.\n\n> >\n> > \t\t/*\n> > \t\t * Serialize the control list, this should fail as the control","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 665EA62E55\n\tfor ;\n\tSun, 26 Apr 2020 17:30:56 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D04444F7;\n\tSun, 26 Apr 2020 17:30:55 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AsFdVw7v\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1587915056;\n\tbh=j8miL6wtNlEjXiNAYXlZyPOaWx2BShTwPwLFRkCz2sQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AsFdVw7vRGcBw+tguWqqG9X1n2hOz+QechlW3x77BcPQp6Y4vUpxsjqi35PY3ZPdr\n\t3xFDfdJ5RSjBo+OxcTqXHX/ZA4NNxWcalmyQlvwDmT+EcNNtF8LQOV76D0HJg/1Vi4\n\tBSuoUO4cwlR6Qw/yPqpT1Zav3eATgyoRnFSLwXZw=","Date":"Sun, 26 Apr 2020 18:30:40 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200426153040.GA5950@pendragon.ideasonboard.com>","References":"<20200425205639.30566-1-laurent.pinchart@ideasonboard.com>\n\t<20200425205639.30566-2-laurent.pinchart@ideasonboard.com>\n\t<20200426151003.62et52rbp5wmtl2p@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200426151003.62et52rbp5wmtl2p@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH 2/2] [DNI] test: Test serialization of\n\tRectangle and Size controls","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":"Sun, 26 Apr 2020 15:30:57 -0000"}}]