[{"id":463,"web_url":"https://patchwork.libcamera.org/comment/463/","msgid":"<20190121204603.GG4439@pendragon.ideasonboard.com>","date":"2019-01-21T20:46:03","subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Mon, Jan 21, 2019 at 06:27:03PM +0100, Jacopo Mondi wrote:\n> Add a class hierarchy internal to V4L2Device to handle set/get format\n> operations in the single/multi plane use case.\n> \n> Provide stubs only for get/setFormat methods at the moment.\n> ---\n> src/libcamera/include/v4l2_device.h | 34 +++++++++++++++++\n> src/libcamera/v4l2_device.cpp | 59 +++++++++++++++++++++++++++++\n> 2 files changed, 93 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> index 0101a4b..81992dc 100644\n> --- a/src/libcamera/include/v4l2_device.h\n> +++ b/src/libcamera/include/v4l2_device.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_V4L2_DEVICE_H__\n> #define __LIBCAMERA_V4L2_DEVICE_H__\n> \n> +#include \n> #include \n> \n> #include \n> @@ -54,11 +55,44 @@ public:\n> \tconst char *busName() const { return caps_.bus_info(); }\n> \n> \tint setControl(unsigned int control, int value);\n> +\tint getFormat();\n\nShould this be named just format() ?\n\n> +\tint setFormat();\n> \n> private:\n> +\tclass V4L2Format\n> +\t{\n> +\tpublic:\n> +\t\tvirtual ~V4L2Format() { }\n> +\t\tvirtual int setFormat() = 0;\n> +\t\tvirtual int getFormat() = 0;\n> +\n> +\tprotected:\n> +\t\tV4L2Format() { }\n> +\t\tV4L2Format(V4L2Format &) = delete;\n> +\t};\n> +\n> +\tclass V4L2FormatSPlane : public V4L2Format\n> +\t{\n> +\tpublic:\n> +\t\tV4L2FormatSPlane() { }\n> +\t\t~V4L2FormatSPlane() { }\n> +\t\tint setFormat();\n> +\t\tint getFormat();\n> +\t};\n> +\n> +\tclass V4L2FormatMPlane : public V4L2Format\n> +\t{\n> +\tpublic:\n> +\t\tV4L2FormatMPlane() { }\n> +\t\t~V4L2FormatMPlane() { }\n> +\t\tint setFormat();\n> +\t\tint getFormat();\n> +\t};\n> +\n> \tstd::string devnode_;\n> \tint fd_;\n> \tV4L2Capability caps_;\n> +\tstd::unique_ptr format_;\n\nI have a bit of trouble reviewing this patch as I don't understand how\nthis is supposed to be used, but it seems overly complicated to me. I\nthink a simple\n\n\tif (single planaer) {\n\t\t...\n\t} else {\n\t\t...\n\t}\n\nin getFormat() and setFormat() would be good enough.\n\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> index 7cd89d7..126f6f2 100644\n> --- a/src/libcamera/v4l2_device.cpp\n> +++ b/src/libcamera/v4l2_device.cpp\n> @@ -5,6 +5,8 @@\n> * v4l2_device.cpp - V4L2 Device\n> */\n> \n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -13,6 +15,7 @@\n> \n> #include \"log.h\"\n> #include \"media_object.h\"\n> +#include \"utils.h\"\n> #include \"v4l2_device.h\"\n> \n> /**\n> @@ -182,6 +185,12 @@ int V4L2Device::open()\n> \t\treturn -EINVAL;\n> \t}\n> \n> +\t/* Create single/multi plane format handler. */\n> +\tif (caps_.isSinglePlane())\n> +\t\tformat_ = utils::make_unique();\n> +\telse\n> +\t\tformat_ = utils::make_unique();\n> +\n> \treturn 0;\n> }\n> \n> @@ -262,4 +271,54 @@ int V4L2Device::setControl(unsigned int control, int value)\n> \treturn v4l2_ctrl.value;\n> }\n> \n> +/**\n> + * \\brief Retrieve the image format set on the V4L2 device\n> + *\n> + * TODO: define how the image format is encoded\n> + * FIXME: this function is a stub at the moment\n> + *\n> + * \\return 0 for success, a negative error code otherwise\n> + */\n> +int V4L2Device::getFormat()\n> +{\n> +\treturn format_->getFormat();\n> +}\n> +\n> +/**\n> + * \\brief Program an image format on the V4L2 device\n> + *\n> + * TODO: define how the image format is encoded\n> + * FIXME: this function is a stub at the moment\n> + *\n> + * \\return 0 for success, a negative error code otherwise\n> + */\n> +int V4L2Device::setFormat()\n> +{\n> +\treturn format_->setFormat();\n> +}\n> +\n> +/* FIXME: this function is a stub at the moment. */\n> +int V4L2Device::V4L2FormatSPlane::getFormat()\n> +{\n> +\treturn 0;\n> +}\n> +\n> +/* FIXME: this function is a stub at the moment. */\n> +int V4L2Device::V4L2FormatSPlane::setFormat()\n> +{\n> +\treturn 0;\n> +}\n> +\n> +/* FIXME: this function is a stub at the moment. */\n> +int V4L2Device::V4L2FormatMPlane::getFormat()\n> +{\n> +\treturn 0;\n> +}\n> +\n> +/* FIXME: this function is a stub at the moment. */\n> +int V4L2Device::V4L2FormatMPlane::setFormat()\n> +{\n> +\treturn 0;\n> +}\n> +\n> } /* namespace libcamera */","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 A0AC760B1D\n\tfor ;\n\tMon, 21 Jan 2019 21:46:04 +0100 (CET)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A02653E;\n\tMon, 21 Jan 2019 21:46:04 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1548103564;\n\tbh=HcuKtRMrmEx9s0fL9+XnBepex2oMBDUORzs9gFRq3/k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rzPLQEiDIbEepw/rf3sRdFi6vQZLY5VropdAHeYid0/7+oA19HbrzLGwrJPCYAkPV\n\tlW7vP0yewmCGBM4e2ndyfsmQ8xjjEUpI4awz0njvBFP0wQWrmAvLzNv0bRKon5FsS0\n\tnI8AIc1SQyQiMpcDgXPqKS2CBh++z+tHXdfpKczc=","Date":"Mon, 21 Jan 2019 22:46:03 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190121204603.GG4439@pendragon.ideasonboard.com>","References":"<20190121172705.19985-1-jacopo@jmondi.org>\n\t<20190121172705.19985-5-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190121172705.19985-5-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 21 Jan 2019 20:46:04 -0000"}},{"id":480,"web_url":"https://patchwork.libcamera.org/comment/480/","msgid":"<20190122113937.GL6484@bigcity.dyn.berto.se>","date":"2019-01-22T11:39:37","subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hej,\n\nOn 2019-01-21 22:46:03 +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n> \n> Thank you for the patch.\n> \n> On Mon, Jan 21, 2019 at 06:27:03PM +0100, Jacopo Mondi wrote:\n> > Add a class hierarchy internal to V4L2Device to handle set/get format\n> > operations in the single/multi plane use case.\n> > \n> > Provide stubs only for get/setFormat methods at the moment.\n> > ---\n> > src/libcamera/include/v4l2_device.h | 34 +++++++++++++++++\n> > src/libcamera/v4l2_device.cpp | 59 +++++++++++++++++++++++++++++\n> > 2 files changed, 93 insertions(+)\n> > \n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index 0101a4b..81992dc 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __LIBCAMERA_V4L2_DEVICE_H__\n> > #define __LIBCAMERA_V4L2_DEVICE_H__\n> > \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -54,11 +55,44 @@ public:\n> > \tconst char *busName() const { return caps_.bus_info(); }\n> > \n> > \tint setControl(unsigned int control, int value);\n> > +\tint getFormat();\n> \n> Should this be named just format() ?\n> \n> > +\tint setFormat();\n> > \n> > private:\n> > +\tclass V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tvirtual ~V4L2Format() { }\n> > +\t\tvirtual int setFormat() = 0;\n> > +\t\tvirtual int getFormat() = 0;\n> > +\n> > +\tprotected:\n> > +\t\tV4L2Format() { }\n> > +\t\tV4L2Format(V4L2Format &) = delete;\n> > +\t};\n> > +\n> > +\tclass V4L2FormatSPlane : public V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tV4L2FormatSPlane() { }\n> > +\t\t~V4L2FormatSPlane() { }\n> > +\t\tint setFormat();\n> > +\t\tint getFormat();\n> > +\t};\n> > +\n> > +\tclass V4L2FormatMPlane : public V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tV4L2FormatMPlane() { }\n> > +\t\t~V4L2FormatMPlane() { }\n> > +\t\tint setFormat();\n> > +\t\tint getFormat();\n> > +\t};\n> > +\n> > \tstd::string devnode_;\n> > \tint fd_;\n> > \tV4L2Capability caps_;\n> > +\tstd::unique_ptr format_;\n> \n> I have a bit of trouble reviewing this patch as I don't understand how\n> this is supposed to be used, but it seems overly complicated to me. I\n> think a simple\n> \n> \tif (single planaer) {\n> \t\t...\n> \t} else {\n> \t\t...\n> \t}\n> \n> in getFormat() and setFormat() would be good enough.\n> \n\nI tend to agree with this, but it's just my two euro cents.\n\n> > };\n> > \n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 7cd89d7..126f6f2 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -5,6 +5,8 @@\n> > * v4l2_device.cpp - V4L2 Device\n> > */\n> > \n> > +#include \n> > +\n> > #include \n> > #include \n> > #include \n> > @@ -13,6 +15,7 @@\n> > \n> > #include \"log.h\"\n> > #include \"media_object.h\"\n> > +#include \"utils.h\"\n> > #include \"v4l2_device.h\"\n> > \n> > /**\n> > @@ -182,6 +185,12 @@ int V4L2Device::open()\n> > \t\treturn -EINVAL;\n> > \t}\n> > \n> > +\t/* Create single/multi plane format handler. */\n> > +\tif (caps_.isSinglePlane())\n> > +\t\tformat_ = utils::make_unique();\n> > +\telse\n> > +\t\tformat_ = utils::make_unique();\n> > +\n> > \treturn 0;\n> > }\n> > \n> > @@ -262,4 +271,54 @@ int V4L2Device::setControl(unsigned int control, int value)\n> > \treturn v4l2_ctrl.value;\n> > }\n> > \n> > +/**\n> > + * \\brief Retrieve the image format set on the V4L2 device\n> > + *\n> > + * TODO: define how the image format is encoded\n> > + * FIXME: this function is a stub at the moment\n> > + *\n> > + * \\return 0 for success, a negative error code otherwise\n> > + */\n> > +int V4L2Device::getFormat()\n> > +{\n> > +\treturn format_->getFormat();\n> > +}\n\nIs not V4L2Device::getFormat() a virtual function which should not be \nimplemented?\n\n> > +\n> > +/**\n> > + * \\brief Program an image format on the V4L2 device\n> > + *\n> > + * TODO: define how the image format is encoded\n> > + * FIXME: this function is a stub at the moment\n> > + *\n> > + * \\return 0 for success, a negative error code otherwise\n> > + */\n> > +int V4L2Device::setFormat()\n> > +{\n> > +\treturn format_->setFormat();\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatSPlane::getFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatSPlane::setFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatMPlane::getFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatMPlane::setFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > } /* namespace libcamera */\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\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 mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E9CF360B21\n\tfor ;\n\tTue, 22 Jan 2019 12:39:38 +0100 (CET)","by mail-lj1-x242.google.com with SMTP id s5-v6so20253446ljd.12\n\tfor ;\n\tTue, 22 Jan 2019 03:39:38 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tc203sm2809097lfe.95.2019.01.22.03.39.37\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tTue, 22 Jan 2019 03:39:37 -0800 (PST)"],"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=lNQ7hboJm+2L35sf8+ByWA3d7bTUr1g1ATR08XrgnUE=;\n\tb=o7TEYubD550s5VPDCbehowgfWYWX5bkgca9p09XGR2BC3M6LfPTofe7JxHvbyGOch9\n\tAX+nPlDsQtkHpSVhVxgDFTRHMB2NisCdY5VV23BkjlTD1MdeY0QWcXRfiXYrmhXt96u2\n\tu0zXSNUJX7TgoM+hk34pU1IZsRw76S7BdZdimJkk4y7YtJBaKfZztJqhrvnvHfWqc80z\n\thBDLc4ZMpcMR3x2Kf6ipiycROBTvUX19Sa1fttZNjibPoZaI0mYLYsmQPdIO2MS9/7QP\n\t3vqmzBr5fnHUWEwjeEaybimGNqRUMGTzwxpNySOZEN2trItJZKD4N4gMZE+2IXFGq7O7\n\tna0A==","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=lNQ7hboJm+2L35sf8+ByWA3d7bTUr1g1ATR08XrgnUE=;\n\tb=dzxWB9rxGMWrUKwtGBmOKo5R3j5UzqHCR5O0jW2tHbiuZK0XPl5ZHodhkQ1Pxt1yNE\n\tMN75EVNspU6aAzz2y/SIdxGrxybOGSQtxArwgiW0S44JYfsR77V1OvPpZf+71SYQVMu8\n\tV1CC80J+6B2C9S83+HRYkbcSBNRWttHr3A01+K8Sk2DSAiplS0gKEnvNsPJ+7sWW1ciN\n\tj091ZBdGJXFWvVVnfm3RB7l8Id8nyVhud3gBJgz7W5+mmEmCNwQm71HznQXMATkGyhJN\n\tD1BI7E0lLCRbHsl0paGv3dVy+2p5uItJtryzdaHfps0ZJ0mKgfSzp+bAg0Y77YTWqsJ1\n\tlrXQ==","X-Gm-Message-State":"AJcUukf314P5nVP9ZjypknpK6QZitqPpuPDbE1oJTfPt9wQamZr2faFE\n\tY6zY10aJ8XbKicEo/AIOcx2z4Q==","X-Google-Smtp-Source":"ALg8bN5b89kNO6sxR+QV7R20GjZ6dOCx//opFxBkXG+qfyUB+Z6Gc5BlfU8GY+kBAKWv/zN6oO8sVg==","X-Received":"by 2002:a2e:9dcb:: with SMTP id\n\tx11-v6mr21695253ljj.158.1548157178188; \n\tTue, 22 Jan 2019 03:39:38 -0800 (PST)","Date":"Tue, 22 Jan 2019 12:39:37 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190122113937.GL6484@bigcity.dyn.berto.se>","References":"<20190121172705.19985-1-jacopo@jmondi.org>\n\t<20190121172705.19985-5-jacopo@jmondi.org>\n\t<20190121204603.GG4439@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":"<20190121204603.GG4439@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 22 Jan 2019 11:39:39 -0000"}},{"id":497,"web_url":"https://patchwork.libcamera.org/comment/497/","msgid":"<20190122141434.lvsme7vbunhwtefq@uno.localdomain>","date":"2019-01-22T14:14:34","subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Mon, Jan 21, 2019 at 10:46:03PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Mon, Jan 21, 2019 at 06:27:03PM +0100, Jacopo Mondi wrote:\n> > Add a class hierarchy internal to V4L2Device to handle set/get format\n> > operations in the single/multi plane use case.\n> >\n> > Provide stubs only for get/setFormat methods at the moment.\n> > ---\n> > src/libcamera/include/v4l2_device.h | 34 +++++++++++++++++\n> > src/libcamera/v4l2_device.cpp | 59 +++++++++++++++++++++++++++++\n> > 2 files changed, 93 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/v4l2_device.h b/src/libcamera/include/v4l2_device.h\n> > index 0101a4b..81992dc 100644\n> > --- a/src/libcamera/include/v4l2_device.h\n> > +++ b/src/libcamera/include/v4l2_device.h\n> > @@ -7,6 +7,7 @@\n> > #ifndef __LIBCAMERA_V4L2_DEVICE_H__\n> > #define __LIBCAMERA_V4L2_DEVICE_H__\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -54,11 +55,44 @@ public:\n> > \tconst char *busName() const { return caps_.bus_info(); }\n> >\n> > \tint setControl(unsigned int control, int value);\n> > +\tint getFormat();\n>\n> Should this be named just format() ?\n>\n> > +\tint setFormat();\n> >\n> > private:\n> > +\tclass V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tvirtual ~V4L2Format() { }\n> > +\t\tvirtual int setFormat() = 0;\n> > +\t\tvirtual int getFormat() = 0;\n> > +\n> > +\tprotected:\n> > +\t\tV4L2Format() { }\n> > +\t\tV4L2Format(V4L2Format &) = delete;\n> > +\t};\n> > +\n> > +\tclass V4L2FormatSPlane : public V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tV4L2FormatSPlane() { }\n> > +\t\t~V4L2FormatSPlane() { }\n> > +\t\tint setFormat();\n> > +\t\tint getFormat();\n> > +\t};\n> > +\n> > +\tclass V4L2FormatMPlane : public V4L2Format\n> > +\t{\n> > +\tpublic:\n> > +\t\tV4L2FormatMPlane() { }\n> > +\t\t~V4L2FormatMPlane() { }\n> > +\t\tint setFormat();\n> > +\t\tint getFormat();\n> > +\t};\n> > +\n> > \tstd::string devnode_;\n> > \tint fd_;\n> > \tV4L2Capability caps_;\n> > +\tstd::unique_ptr format_;\n>\n> I have a bit of trouble reviewing this patch as I don't understand how\n> this is supposed to be used, but it seems overly complicated to me. I\n> think a simple\n>\n> \tif (single planaer) {\n> \t\t...\n> \t} else {\n> \t\t...\n> \t}\n>\n> in getFormat() and setFormat() would be good enough.\n\nMaybe. I made a class hierarcy to handle the format set/get because I\nthought that would have been useful to store in the class instances\ninformations on the currently configured format and isolate it from\nthe rest of the V4L2 device object.\n\nBoth you and Niklas think this is an overkill, and I can drop it.\nI'm sure this is not \"too complicated to review\" for you, though.\n\nThanks\n j\n\n>\n> > };\n> >\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp\n> > index 7cd89d7..126f6f2 100644\n> > --- a/src/libcamera/v4l2_device.cpp\n> > +++ b/src/libcamera/v4l2_device.cpp\n> > @@ -5,6 +5,8 @@\n> > * v4l2_device.cpp - V4L2 Device\n> > */\n> >\n> > +#include \n> > +\n> > #include \n> > #include \n> > #include \n> > @@ -13,6 +15,7 @@\n> >\n> > #include \"log.h\"\n> > #include \"media_object.h\"\n> > +#include \"utils.h\"\n> > #include \"v4l2_device.h\"\n> >\n> > /**\n> > @@ -182,6 +185,12 @@ int V4L2Device::open()\n> > \t\treturn -EINVAL;\n> > \t}\n> >\n> > +\t/* Create single/multi plane format handler. */\n> > +\tif (caps_.isSinglePlane())\n> > +\t\tformat_ = utils::make_unique();\n> > +\telse\n> > +\t\tformat_ = utils::make_unique();\n> > +\n> > \treturn 0;\n> > }\n> >\n> > @@ -262,4 +271,54 @@ int V4L2Device::setControl(unsigned int control, int value)\n> > \treturn v4l2_ctrl.value;\n> > }\n> >\n> > +/**\n> > + * \\brief Retrieve the image format set on the V4L2 device\n> > + *\n> > + * TODO: define how the image format is encoded\n> > + * FIXME: this function is a stub at the moment\n> > + *\n> > + * \\return 0 for success, a negative error code otherwise\n> > + */\n> > +int V4L2Device::getFormat()\n> > +{\n> > +\treturn format_->getFormat();\n> > +}\n> > +\n> > +/**\n> > + * \\brief Program an image format on the V4L2 device\n> > + *\n> > + * TODO: define how the image format is encoded\n> > + * FIXME: this function is a stub at the moment\n> > + *\n> > + * \\return 0 for success, a negative error code otherwise\n> > + */\n> > +int V4L2Device::setFormat()\n> > +{\n> > +\treturn format_->setFormat();\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatSPlane::getFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatSPlane::setFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatMPlane::getFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > +/* FIXME: this function is a stub at the moment. */\n> > +int V4L2Device::V4L2FormatMPlane::setFormat()\n> > +{\n> > +\treturn 0;\n> > +}\n> > +\n> > } /* namespace libcamera */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from relay4-d.mail.gandi.net (relay4-d.mail.gandi.net\n\t[217.70.183.196])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 818A960B1B\n\tfor ;\n\tTue, 22 Jan 2019 15:14:34 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay4-d.mail.gandi.net (Postfix) with ESMTPSA id 12E5CE0016;\n\tTue, 22 Jan 2019 14:14:33 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Tue, 22 Jan 2019 15:14:34 +0100","From":"Jacopo Mondi ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190122141434.lvsme7vbunhwtefq@uno.localdomain>","References":"<20190121172705.19985-1-jacopo@jmondi.org>\n\t<20190121172705.19985-5-jacopo@jmondi.org>\n\t<20190121204603.GG4439@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"x2ihugpjjrci5tcu\"","Content-Disposition":"inline","In-Reply-To":"<20190121204603.GG4439@pendragon.ideasonboard.com>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 4/6] libcamera: v4l2_device: Add\n\tsingle/multiplane formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Tue, 22 Jan 2019 14:14:34 -0000"}}]