[{"id":829,"web_url":"https://patchwork.libcamera.org/comment/829/","msgid":"<20190221153156.GD11484@bigcity.dyn.berto.se>","date":"2019-02-21T15:31:56","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your patch.\n\nOn 2019-02-19 17:56:18 +0100, Jacopo Mondi wrote:\n> Implement enumFormat() methods to enumerate the available image\n> resolutions on the subdevice.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_subdevice.h | 11 ++++\n> src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++\n> 2 files changed, 101 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index 82fa6685ab52..c7045776555c 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -14,6 +14,16 @@ namespace libcamera {\n> class MediaEntity;\n> struct Rectangle;\n> \n> +struct V4L2SubdeviceFormatEnum {\n> +\tunsigned int index;\n> +\tuint32_t mbus_code;\n> +\n> +\tuint32_t minWidth;\n> +\tuint32_t maxWidth;\n> +\tuint32_t minHeight;\n> +\tuint32_t maxHeight;\n> +};\n> +\n> struct V4L2SubdeviceFormat {\n> \tuint32_t mbus_code;\n> \tuint32_t width;\n> @@ -36,6 +46,7 @@ public:\n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> \n> +\tint enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);\n> \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index b436f73cc75f..5665154a2762 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -26,6 +26,52 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(V4L2Subdev)\n> \n> +/**\n> + * \\struct V4L2SubdeviceFormatEnum\n> + * \\brief The V4L2 sub-device image size enumeration\n> + *\n> + * This structure describes an image resolution as enumerated by the V4L2\n> + * sub-device. The structure is used as format exchange between the caller and\n> + * the enumFormat() method. The caller is responsible to fill the media bus\n> + * code to use and the index of the format to be enumerated.\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::index\n> + * \\brief The index of the format to be enumerated\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::mbus_code\n> + * \\brief The pixel format identification code for the formats to enumerate\n> + *\n> + * \\sa V4L2SubdeviceFormat for media bus format pixel code description.\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::minWidth\n> + * \\brief The minimum width of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::maxWidth\n> + * \\brief The maximum width of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::minHeight\n> + * \\brief The minimum height of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::maxHeight\n> + * \\brief The maximum height of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> /**\n> * \\struct V4L2SubdeviceFormat\n> * \\brief The V4L2 sub-device image format and sizes\n> @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> }\n> \n> +/**\n> + * \\brief Enumerate the sub-device image resolutions\n> + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> + * \\param[inout] formatEnum The format enumeration description\n> + *\n> + * The method retrieve the image resolution of the image format with index\n> + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.\n> + *\n> + * To enumerate image resolutions, the caller shall start enumerating all\n> + * formats by setting the formatEnum.index field to 0, and increasing it by\n> + * one for any successive call, until the -EINVAL return code is returned.\n\nShould we not help pipeline handlers by hiding this? And instead return \na vector of V4L2SubdeviceFormatEnum with all formats?\n\n> + *\n> + * \\return 0 on success, or a negative error code otherwise\n> + */\n> +int V4L2Subdevice::enumFormat(unsigned int pad,\n> +\t\t\t V4L2SubdeviceFormatEnum *formatEnum)\n> +{\n> +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> +\n> +\tsizeEnum.index = formatEnum->index;\n> +\tsizeEnum.code = formatEnum->mbus_code;\n> +\tsizeEnum.pad = pad;\n\nThis is confusing, index and code comes from formatEnum while pad is a \nseparate argument.\n\n> +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n\nI think this should be configurable by the caller or other way \ncustomisable. In the not so distant future we will need to add support \nfor V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it \n:-)\n\n> +\n> +\tint ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);\n> +\tif (ret) {\n> +\t\tret = -errno;\n> +\t\tif (ret == -EINVAL)\n> +\t\t\treturn ret;\n> +\n> +\t\tLOG(V4L2Subdev, Error)\n> +\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> +\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tformatEnum->maxWidth = sizeEnum.max_width;\n> +\tformatEnum->minWidth = sizeEnum.min_width;\n> +\tformatEnum->maxHeight = sizeEnum.max_height;\n> +\tformatEnum->minHeight = sizeEnum.min_height;\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> -- \n> 2.20.1\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 mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CBBAA600FB\n\tfor ;\n\tThu, 21 Feb 2019 16:31:59 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id f24-v6so24371828ljk.0\n\tfor ;\n\tThu, 21 Feb 2019 07:31:59 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tx16sm6756407lff.72.2019.02.21.07.31.57\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 21 Feb 2019 07:31:57 -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=/kgEmELW1Iu/1dqqX1D48xQOXzLXzDYiIa0RAuMkLYw=;\n\tb=Bjqq8Do0/n+uw605jJN51ScXvT0xwLOTmEosM+FZDYMtQpu+ru9vJILHEoMlzhKpl/\n\tD5mom8eAfptjcFaaoVyq1IgIQyF8mZRsm9LhSCUk+1AakuYZMsURSdaW6wMFjVDYizQl\n\tHjJ44qIYfjxCu7QuZFZ2Qk4mi1Q6eDfa7NgVbQm4j5SDOx1I1DA7QHUIW6cpg/wqt1Ay\n\tDQ6Nwa5ja5syKQk9ysKwIdFj0HGAs1xvxhfbS/gNaU7SMYjpKj2zItAU9M6YWK2tK1KL\n\tGAN0pAxNVmuhBtJEzBOLu97Ct3y4eP3PY9ukrdBNJ6pOOGHGM4dH4l0Lpvv1lXRCM4C8\n\t+Ljw==","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=/kgEmELW1Iu/1dqqX1D48xQOXzLXzDYiIa0RAuMkLYw=;\n\tb=CWyF6pABgc65jOcoTW1h1lh4Pe3zz2gE4OjHSN2dteP8g2B/9k3UM9wlIh8CwdhM/7\n\tM/HIO9w2HSYgVyBtOtXcjy4desZEwpzNSg0SLP5S1ktpxWxWtZMHYU4Tvs9/oCco2wvq\n\t/+sIRFzPC8aUO+iUjX550Jhpf4wtjGknu02sGSswExN7J39gv45fV64nWpRP3tqKuIXO\n\tcMVbteSe+2J8fpowV47lagNjm+FHO420AZUVprjawZY/jlXUai0jtmRhcM543mFEl9pH\n\tHdEo8AGZ3LRrCufL2hYd6Takmxq1Y4iN5V/LgQN2TL2msemeDtxcNbhpATdm3H8X91yR\n\tb2NQ==","X-Gm-Message-State":"AHQUAuYfr9CJC7RFiZn8Se5lyrfFXdlPflvsHbrfMEiiv/7pogijW9d7\n\tcI4AggBAzEKxLo/LZTg5gJoAqr58xyQ=","X-Google-Smtp-Source":"AHgI3IaaCx2H66XCUlNR3Y1RR11NksSs+eabzY+WWXlnGXuGFKBHEeX6vH2VK2zxs8lo8Bj1x+idyg==","X-Received":"by 2002:a2e:898e:: with SMTP id\n\tc14mr17437805lji.115.1550763118736; \n\tThu, 21 Feb 2019 07:31:58 -0800 (PST)","Date":"Thu, 21 Feb 2019 16:31:56 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190221153156.GD11484@bigcity.dyn.berto.se>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190219165620.2385-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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":"Thu, 21 Feb 2019 15:32:00 -0000"}},{"id":838,"web_url":"https://patchwork.libcamera.org/comment/838/","msgid":"<20190221232519.GH3485@pendragon.ideasonboard.com>","date":"2019-02-21T23:25:19","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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 Tue, Feb 19, 2019 at 05:56:18PM +0100, Jacopo Mondi wrote:\n> Implement enumFormat() methods to enumerate the available image\n> resolutions on the subdevice.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/include/v4l2_subdevice.h | 11 ++++\n> src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++\n> 2 files changed, 101 insertions(+)\n> \n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index 82fa6685ab52..c7045776555c 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -14,6 +14,16 @@ namespace libcamera {\n> class MediaEntity;\n> struct Rectangle;\n> \n> +struct V4L2SubdeviceFormatEnum {\n> +\tunsigned int index;\n> +\tuint32_t mbus_code;\n> +\n> +\tuint32_t minWidth;\n> +\tuint32_t maxWidth;\n> +\tuint32_t minHeight;\n> +\tuint32_t maxHeight;\n> +};\n> +\n> struct V4L2SubdeviceFormat {\n> \tuint32_t mbus_code;\n> \tuint32_t width;\n> @@ -36,6 +46,7 @@ public:\n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> \n> +\tint enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);\n> \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index b436f73cc75f..5665154a2762 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -26,6 +26,52 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(V4L2Subdev)\n> \n> +/**\n> + * \\struct V4L2SubdeviceFormatEnum\n> + * \\brief The V4L2 sub-device image size enumeration\n> + *\n> + * This structure describes an image resolution as enumerated by the V4L2\n> + * sub-device. The structure is used as format exchange between the caller and\n> + * the enumFormat() method. The caller is responsible to fill the media bus\n> + * code to use and the index of the format to be enumerated.\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::index\n> + * \\brief The index of the format to be enumerated\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::mbus_code\n> + * \\brief The pixel format identification code for the formats to enumerate\n> + *\n> + * \\sa V4L2SubdeviceFormat for media bus format pixel code description.\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::minWidth\n> + * \\brief The minimum width of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::maxWidth\n> + * \\brief The maximum width of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::minHeight\n> + * \\brief The minimum height of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> +/**\n> + * \\var V4L2SubdeviceFormatEnum::maxHeight\n> + * \\brief The maximum height of the enumerated format as reported by the V4L2\n> + * sub-device\n> + */\n> +\n> /**\n> * \\struct V4L2SubdeviceFormat\n> * \\brief The V4L2 sub-device image format and sizes\n> @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> }\n> \n> +/**\n> + * \\brief Enumerate the sub-device image resolutions\n> + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> + * \\param[inout] formatEnum The format enumeration description\n> + *\n> + * The method retrieve the image resolution of the image format with index\n> + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.\n> + *\n> + * To enumerate image resolutions, the caller shall start enumerating all\n> + * formats by setting the formatEnum.index field to 0, and increasing it by\n> + * one for any successive call, until the -EINVAL return code is returned.\n> + *\n> + * \\return 0 on success, or a negative error code otherwise\n> + */\n> +int V4L2Subdevice::enumFormat(unsigned int pad,\n> +\t\t\t V4L2SubdeviceFormatEnum *formatEnum)\n> +{\n> +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> +\n> +\tsizeEnum.index = formatEnum->index;\n> +\tsizeEnum.code = formatEnum->mbus_code;\n> +\tsizeEnum.pad = pad;\n> +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> +\n> +\tint ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);\n> +\tif (ret) {\n> +\t\tret = -errno;\n> +\t\tif (ret == -EINVAL)\n> +\t\t\treturn ret;\n> +\n> +\t\tLOG(V4L2Subdev, Error)\n> +\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> +\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n\nLet's not repeat the mistake of the V4L2 API that requires calling the\nsame ioctl in a loop. Instead of just wrapping the ioctl, let's make\nthis function more useful by enumerating all supported formats and\nreturning a list. You may even want to have two nested loops, one over\nformats, and the other over sizes.\n\n> +\n> +\tformatEnum->maxWidth = sizeEnum.max_width;\n> +\tformatEnum->minWidth = sizeEnum.min_width;\n> +\tformatEnum->maxHeight = sizeEnum.max_height;\n> +\tformatEnum->minHeight = sizeEnum.min_height;\n> +\n> +\treturn 0;\n> +}\n> +\n> /**\n> * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> * \\param[in] pad The 0-indexed pad number the format is to be retrieved from","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 E81D8600FB\n\tfor ;\n\tFri, 22 Feb 2019 00:25:24 +0100 (CET)","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 48C9F255;\n\tFri, 22 Feb 2019 00:25:24 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1550791524;\n\tbh=oQEqcBhs6ShZ8itMAOSCpAX/TcdCs1gka8HmuoO4JmQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uIaaviMS3q+jgfs8d6aV+8QAFtz7Cu7y5/BUqH77E2imaB89Cz56t726b/B5WjWz2\n\tyuwDi4YB3pB0a+1WMRtr40WGIRXT4JD3S8dPfi2vXzKn4gj6gXHnp14z4dCiY8ESDT\n\tSNSvqcDxGSoKZPMkXkrafl/oMLpBPqT+zdAcmwCI=","Date":"Fri, 22 Feb 2019 01:25:19 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190221232519.GH3485@pendragon.ideasonboard.com>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20190219165620.2385-2-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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":"Thu, 21 Feb 2019 23:25:25 -0000"}},{"id":872,"web_url":"https://patchwork.libcamera.org/comment/872/","msgid":"<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>","date":"2019-02-25T09:28:33","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Thu, Feb 21, 2019 at 04:31:56PM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your patch.\n>\n> On 2019-02-19 17:56:18 +0100, Jacopo Mondi wrote:\n> > Implement enumFormat() methods to enumerate the available image\n> > resolutions on the subdevice.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/include/v4l2_subdevice.h | 11 ++++\n> > src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++\n> > 2 files changed, 101 insertions(+)\n> >\n> > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> > index 82fa6685ab52..c7045776555c 100644\n> > --- a/src/libcamera/include/v4l2_subdevice.h\n> > +++ b/src/libcamera/include/v4l2_subdevice.h\n> > @@ -14,6 +14,16 @@ namespace libcamera {\n> > class MediaEntity;\n> > struct Rectangle;\n> >\n> > +struct V4L2SubdeviceFormatEnum {\n> > +\tunsigned int index;\n> > +\tuint32_t mbus_code;\n> > +\n> > +\tuint32_t minWidth;\n> > +\tuint32_t maxWidth;\n> > +\tuint32_t minHeight;\n> > +\tuint32_t maxHeight;\n> > +};\n> > +\n> > struct V4L2SubdeviceFormat {\n> > \tuint32_t mbus_code;\n> > \tuint32_t width;\n> > @@ -36,6 +46,7 @@ public:\n> > \tint setCrop(unsigned int pad, Rectangle *rect);\n> > \tint setCompose(unsigned int pad, Rectangle *rect);\n> >\n> > +\tint enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);\n> > \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> > \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> >\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index b436f73cc75f..5665154a2762 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -26,6 +26,52 @@ namespace libcamera {\n> >\n> > LOG_DEFINE_CATEGORY(V4L2Subdev)\n> >\n> > +/**\n> > + * \\struct V4L2SubdeviceFormatEnum\n> > + * \\brief The V4L2 sub-device image size enumeration\n> > + *\n> > + * This structure describes an image resolution as enumerated by the V4L2\n> > + * sub-device. The structure is used as format exchange between the caller and\n> > + * the enumFormat() method. The caller is responsible to fill the media bus\n> > + * code to use and the index of the format to be enumerated.\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::index\n> > + * \\brief The index of the format to be enumerated\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::mbus_code\n> > + * \\brief The pixel format identification code for the formats to enumerate\n> > + *\n> > + * \\sa V4L2SubdeviceFormat for media bus format pixel code description.\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::minWidth\n> > + * \\brief The minimum width of the enumerated format as reported by the V4L2\n> > + * sub-device\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::maxWidth\n> > + * \\brief The maximum width of the enumerated format as reported by the V4L2\n> > + * sub-device\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::minHeight\n> > + * \\brief The minimum height of the enumerated format as reported by the V4L2\n> > + * sub-device\n> > + */\n> > +\n> > +/**\n> > + * \\var V4L2SubdeviceFormatEnum::maxHeight\n> > + * \\brief The maximum height of the enumerated format as reported by the V4L2\n> > + * sub-device\n> > + */\n> > +\n> > /**\n> > * \\struct V4L2SubdeviceFormat\n> > * \\brief The V4L2 sub-device image format and sizes\n> > @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> > \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> > }\n> >\n> > +/**\n> > + * \\brief Enumerate the sub-device image resolutions\n> > + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> > + * \\param[inout] formatEnum The format enumeration description\n> > + *\n> > + * The method retrieve the image resolution of the image format with index\n> > + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.\n> > + *\n> > + * To enumerate image resolutions, the caller shall start enumerating all\n> > + * formats by setting the formatEnum.index field to 0, and increasing it by\n> > + * one for any successive call, until the -EINVAL return code is returned.\n>\n> Should we not help pipeline handlers by hiding this? And instead return\n> a vector of V4L2SubdeviceFormatEnum with all formats?\n>\n\nYes, you and Laurent had the same suggestion and is surely better.\nI'll re-implement this.\n\n> > + *\n> > + * \\return 0 on success, or a negative error code otherwise\n> > + */\n> > +int V4L2Subdevice::enumFormat(unsigned int pad,\n> > +\t\t\t V4L2SubdeviceFormatEnum *formatEnum)\n> > +{\n> > +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> > +\n> > +\tsizeEnum.index = formatEnum->index;\n> > +\tsizeEnum.code = formatEnum->mbus_code;\n> > +\tsizeEnum.pad = pad;\n>\n> This is confusing, index and code comes from formatEnum while pad is a\n> separate argument.\n>\n\nAll V4L2Subdevice methods accepts a pad as first argument. I would\nkeep this consistent.\n\n> > +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n>\n> I think this should be configurable by the caller or other way\n> customisable. In the not so distant future we will need to add support\n> for V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it\n> :-)\n\nI'm really not sure about this one, I could add a field to\nV4L2SubdeviceFormatEnum for this...\n\n>\n> > +\n> > +\tint ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);\n> > +\tif (ret) {\n> > +\t\tret = -errno;\n> > +\t\tif (ret == -EINVAL)\n> > +\t\t\treturn ret;\n> > +\n> > +\t\tLOG(V4L2Subdev, Error)\n> > +\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> > +\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tformatEnum->maxWidth = sizeEnum.max_width;\n> > +\tformatEnum->minWidth = sizeEnum.min_width;\n> > +\tformatEnum->maxHeight = sizeEnum.max_height;\n> > +\tformatEnum->minHeight = sizeEnum.min_height;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > /**\n> > * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> > * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","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 5948B600FE\n\tfor ;\n\tMon, 25 Feb 2019 10:28:05 +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 D17B7E0002;\n\tMon, 25 Feb 2019 09:28:04 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 25 Feb 2019 10:28:33 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>\n\t<20190221153156.GD11484@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"gouoyny4zoupi2i7\"","Content-Disposition":"inline","In-Reply-To":"<20190221153156.GD11484@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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, 25 Feb 2019 09:28:05 -0000"}},{"id":874,"web_url":"https://patchwork.libcamera.org/comment/874/","msgid":"<20190225094435.diy2qjlhi5f4qx7e@uno.localdomain>","date":"2019-02-25T09:44:35","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"On Mon, Feb 25, 2019 at 10:28:33AM +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n>\n> On Thu, Feb 21, 2019 at 04:31:56PM +0100, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your patch.\n> >\n> > On 2019-02-19 17:56:18 +0100, Jacopo Mondi wrote:\n> > > Implement enumFormat() methods to enumerate the available image\n> > > resolutions on the subdevice.\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/include/v4l2_subdevice.h | 11 ++++\n> > > src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++\n> > > 2 files changed, 101 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> > > index 82fa6685ab52..c7045776555c 100644\n> > > --- a/src/libcamera/include/v4l2_subdevice.h\n> > > +++ b/src/libcamera/include/v4l2_subdevice.h\n> > > @@ -14,6 +14,16 @@ namespace libcamera {\n> > > class MediaEntity;\n> > > struct Rectangle;\n> > >\n> > > +struct V4L2SubdeviceFormatEnum {\n> > > +\tunsigned int index;\n> > > +\tuint32_t mbus_code;\n> > > +\n> > > +\tuint32_t minWidth;\n> > > +\tuint32_t maxWidth;\n> > > +\tuint32_t minHeight;\n> > > +\tuint32_t maxHeight;\n> > > +};\n> > > +\n> > > struct V4L2SubdeviceFormat {\n> > > \tuint32_t mbus_code;\n> > > \tuint32_t width;\n> > > @@ -36,6 +46,7 @@ public:\n> > > \tint setCrop(unsigned int pad, Rectangle *rect);\n> > > \tint setCompose(unsigned int pad, Rectangle *rect);\n> > >\n> > > +\tint enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);\n> > > \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> > > \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> > >\n> > > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > > index b436f73cc75f..5665154a2762 100644\n> > > --- a/src/libcamera/v4l2_subdevice.cpp\n> > > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > > @@ -26,6 +26,52 @@ namespace libcamera {\n> > >\n> > > LOG_DEFINE_CATEGORY(V4L2Subdev)\n> > >\n> > > +/**\n> > > + * \\struct V4L2SubdeviceFormatEnum\n> > > + * \\brief The V4L2 sub-device image size enumeration\n> > > + *\n> > > + * This structure describes an image resolution as enumerated by the V4L2\n> > > + * sub-device. The structure is used as format exchange between the caller and\n> > > + * the enumFormat() method. The caller is responsible to fill the media bus\n> > > + * code to use and the index of the format to be enumerated.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::index\n> > > + * \\brief The index of the format to be enumerated\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::mbus_code\n> > > + * \\brief The pixel format identification code for the formats to enumerate\n> > > + *\n> > > + * \\sa V4L2SubdeviceFormat for media bus format pixel code description.\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::minWidth\n> > > + * \\brief The minimum width of the enumerated format as reported by the V4L2\n> > > + * sub-device\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::maxWidth\n> > > + * \\brief The maximum width of the enumerated format as reported by the V4L2\n> > > + * sub-device\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::minHeight\n> > > + * \\brief The minimum height of the enumerated format as reported by the V4L2\n> > > + * sub-device\n> > > + */\n> > > +\n> > > +/**\n> > > + * \\var V4L2SubdeviceFormatEnum::maxHeight\n> > > + * \\brief The maximum height of the enumerated format as reported by the V4L2\n> > > + * sub-device\n> > > + */\n> > > +\n> > > /**\n> > > * \\struct V4L2SubdeviceFormat\n> > > * \\brief The V4L2 sub-device image format and sizes\n> > > @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> > > \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Enumerate the sub-device image resolutions\n> > > + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> > > + * \\param[inout] formatEnum The format enumeration description\n> > > + *\n> > > + * The method retrieve the image resolution of the image format with index\n> > > + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.\n> > > + *\n> > > + * To enumerate image resolutions, the caller shall start enumerating all\n> > > + * formats by setting the formatEnum.index field to 0, and increasing it by\n> > > + * one for any successive call, until the -EINVAL return code is returned.\n> >\n> > Should we not help pipeline handlers by hiding this? And instead return\n> > a vector of V4L2SubdeviceFormatEnum with all formats?\n> >\n>\n> Yes, you and Laurent had the same suggestion and is surely better.\n> I'll re-implement this.\n>\n> > > + *\n> > > + * \\return 0 on success, or a negative error code otherwise\n> > > + */\n> > > +int V4L2Subdevice::enumFormat(unsigned int pad,\n> > > +\t\t\t V4L2SubdeviceFormatEnum *formatEnum)\n> > > +{\n> > > +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> > > +\n> > > +\tsizeEnum.index = formatEnum->index;\n> > > +\tsizeEnum.code = formatEnum->mbus_code;\n> > > +\tsizeEnum.pad = pad;\n> >\n> > This is confusing, index and code comes from formatEnum while pad is a\n> > separate argument.\n> >\n>\n> All V4L2Subdevice methods accepts a pad as first argument. I would\n> keep this consistent.\n>\n> > > +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> >\n> > I think this should be configurable by the caller or other way\n> > customisable. In the not so distant future we will need to add support\n> > for V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it\n> > :-)\n>\n> I'm really not sure about this one, I could add a field to\n> V4L2SubdeviceFormatEnum for this...\n>\n\nRe-thinking about this, I would leave this out until we don't define a\nway to handle FORMAT_TRY globally, as otherwise I would have to expose\nthat flag outside of V4L2Subdevice which I would prefer not to...\n\n> >\n> > > +\n> > > +\tint ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);\n> > > +\tif (ret) {\n> > > +\t\tret = -errno;\n> > > +\t\tif (ret == -EINVAL)\n> > > +\t\t\treturn ret;\n> > > +\n> > > +\t\tLOG(V4L2Subdev, Error)\n> > > +\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> > > +\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> > > +\t\treturn ret;\n> > > +\t}\n> > > +\n> > > +\tformatEnum->maxWidth = sizeEnum.max_width;\n> > > +\tformatEnum->minWidth = sizeEnum.min_width;\n> > > +\tformatEnum->maxHeight = sizeEnum.max_height;\n> > > +\tformatEnum->minHeight = sizeEnum.min_height;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> > > * \\param[in] pad The 0-indexed pad number the format is to be retrieved from\n> > > --\n> > > 2.20.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\n> >\n> > --\n> > Regards,\n> > Niklas Söderlund\n\n\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 relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 86E54600FE\n\tfor ;\n\tMon, 25 Feb 2019 10:44:07 +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 relay1-d.mail.gandi.net (Postfix) with ESMTPSA id EBD06240002;\n\tMon, 25 Feb 2019 09:44:06 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Mon, 25 Feb 2019 10:44:35 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190225094435.diy2qjlhi5f4qx7e@uno.localdomain>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>\n\t<20190221153156.GD11484@bigcity.dyn.berto.se>\n\t<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"hetho4ljgljb4owq\"","Content-Disposition":"inline","In-Reply-To":"<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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, 25 Feb 2019 09:44:07 -0000"}},{"id":878,"web_url":"https://patchwork.libcamera.org/comment/878/","msgid":"<20190225195055.GE4756@pendragon.ideasonboard.com>","date":"2019-02-25T19:50:55","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Feb 25, 2019 at 10:44:35AM +0100, Jacopo Mondi wrote:\n> On Mon, Feb 25, 2019 at 10:28:33AM +0100, Jacopo Mondi wrote:\n> > On Thu, Feb 21, 2019 at 04:31:56PM +0100, Niklas Söderlund wrote:\n> >> On 2019-02-19 17:56:18 +0100, Jacopo Mondi wrote:\n> >>> Implement enumFormat() methods to enumerate the available image\n> >>> resolutions on the subdevice.\n> >>>\n> >>> Signed-off-by: Jacopo Mondi \n> >>> ---\n> >>> src/libcamera/include/v4l2_subdevice.h | 11 ++++\n> >>> src/libcamera/v4l2_subdevice.cpp | 90 ++++++++++++++++++++++++++\n> >>> 2 files changed, 101 insertions(+)\n> >>>\n> >>> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> >>> index 82fa6685ab52..c7045776555c 100644\n> >>> --- a/src/libcamera/include/v4l2_subdevice.h\n> >>> +++ b/src/libcamera/include/v4l2_subdevice.h\n> >>> @@ -14,6 +14,16 @@ namespace libcamera {\n> >>> class MediaEntity;\n> >>> struct Rectangle;\n> >>>\n> >>> +struct V4L2SubdeviceFormatEnum {\n> >>> +\tunsigned int index;\n> >>> +\tuint32_t mbus_code;\n> >>> +\n> >>> +\tuint32_t minWidth;\n> >>> +\tuint32_t maxWidth;\n> >>> +\tuint32_t minHeight;\n> >>> +\tuint32_t maxHeight;\n> >>> +};\n> >>> +\n> >>> struct V4L2SubdeviceFormat {\n> >>> \tuint32_t mbus_code;\n> >>> \tuint32_t width;\n> >>> @@ -36,6 +46,7 @@ public:\n> >>> \tint setCrop(unsigned int pad, Rectangle *rect);\n> >>> \tint setCompose(unsigned int pad, Rectangle *rect);\n> >>>\n> >>> +\tint enumFormat(unsigned int pad, V4L2SubdeviceFormatEnum *formatEnum);\n> >>> \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> >>> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> >>>\n> >>> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> >>> index b436f73cc75f..5665154a2762 100644\n> >>> --- a/src/libcamera/v4l2_subdevice.cpp\n> >>> +++ b/src/libcamera/v4l2_subdevice.cpp\n> >>> @@ -26,6 +26,52 @@ namespace libcamera {\n> >>>\n> >>> LOG_DEFINE_CATEGORY(V4L2Subdev)\n> >>>\n> >>> +/**\n> >>> + * \\struct V4L2SubdeviceFormatEnum\n> >>> + * \\brief The V4L2 sub-device image size enumeration\n> >>> + *\n> >>> + * This structure describes an image resolution as enumerated by the V4L2\n> >>> + * sub-device. The structure is used as format exchange between the caller and\n> >>> + * the enumFormat() method. The caller is responsible to fill the media bus\n> >>> + * code to use and the index of the format to be enumerated.\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::index\n> >>> + * \\brief The index of the format to be enumerated\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::mbus_code\n> >>> + * \\brief The pixel format identification code for the formats to enumerate\n> >>> + *\n> >>> + * \\sa V4L2SubdeviceFormat for media bus format pixel code description.\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::minWidth\n> >>> + * \\brief The minimum width of the enumerated format as reported by the V4L2\n> >>> + * sub-device\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::maxWidth\n> >>> + * \\brief The maximum width of the enumerated format as reported by the V4L2\n> >>> + * sub-device\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::minHeight\n> >>> + * \\brief The minimum height of the enumerated format as reported by the V4L2\n> >>> + * sub-device\n> >>> + */\n> >>> +\n> >>> +/**\n> >>> + * \\var V4L2SubdeviceFormatEnum::maxHeight\n> >>> + * \\brief The maximum height of the enumerated format as reported by the V4L2\n> >>> + * sub-device\n> >>> + */\n> >>> +\n> >>> /**\n> >>> * \\struct V4L2SubdeviceFormat\n> >>> * \\brief The V4L2 sub-device image format and sizes\n> >>> @@ -171,6 +217,50 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> >>> \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> >>> }\n> >>>\n> >>> +/**\n> >>> + * \\brief Enumerate the sub-device image resolutions\n> >>> + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> >>> + * \\param[inout] formatEnum The format enumeration description\n> >>> + *\n> >>> + * The method retrieve the image resolution of the image format with index\n> >>> + * formatEnum.index for the pixel format specified by formatEnum.mbus_code.\n> >>> + *\n> >>> + * To enumerate image resolutions, the caller shall start enumerating all\n> >>> + * formats by setting the formatEnum.index field to 0, and increasing it by\n> >>> + * one for any successive call, until the -EINVAL return code is returned.\n> >>\n> >> Should we not help pipeline handlers by hiding this? And instead return\n> >> a vector of V4L2SubdeviceFormatEnum with all formats?\n> >>\n> >\n> > Yes, you and Laurent had the same suggestion and is surely better.\n> > I'll re-implement this.\n> >\n> >>> + *\n> >>> + * \\return 0 on success, or a negative error code otherwise\n> >>> + */\n> >>> +int V4L2Subdevice::enumFormat(unsigned int pad,\n> >>> +\t\t\t V4L2SubdeviceFormatEnum *formatEnum)\n> >>> +{\n> >>> +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> >>> +\n> >>> +\tsizeEnum.index = formatEnum->index;\n> >>> +\tsizeEnum.code = formatEnum->mbus_code;\n> >>> +\tsizeEnum.pad = pad;\n> >>\n> >> This is confusing, index and code comes from formatEnum while pad is a\n> >> separate argument.\n> >>\n> >\n> > All V4L2Subdevice methods accepts a pad as first argument. I would\n> > keep this consistent.\n> >\n> >>> +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> >>\n> >> I think this should be configurable by the caller or other way\n> >> customisable. In the not so distant future we will need to add support\n> >> for V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it\n> >> :-)\n> >\n> > I'm really not sure about this one, I could add a field to\n> > V4L2SubdeviceFormatEnum for this...\n> >\n> \n> Re-thinking about this, I would leave this out until we don't define a\n> way to handle FORMAT_TRY globally, as otherwise I would have to expose\n> that flag outside of V4L2Subdevice which I would prefer not to...\n\nI think you made the right decision.\n\n> >>> +\n> >>> +\tint ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE, &sizeEnum);\n> >>> +\tif (ret) {\n> >>> +\t\tret = -errno;\n> >>> +\t\tif (ret == -EINVAL)\n> >>> +\t\t\treturn ret;\n> >>> +\n> >>> +\t\tLOG(V4L2Subdev, Error)\n> >>> +\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> >>> +\t\t\t<< \" of \" << deviceNode_ << \": \" << strerror(-ret);\n> >>> +\t\treturn ret;\n> >>> +\t}\n> >>> +\n> >>> +\tformatEnum->maxWidth = sizeEnum.max_width;\n> >>> +\tformatEnum->minWidth = sizeEnum.min_width;\n> >>> +\tformatEnum->maxHeight = sizeEnum.max_height;\n> >>> +\tformatEnum->minHeight = sizeEnum.min_height;\n> >>> +\n> >>> +\treturn 0;\n> >>> +}\n> >>> +\n> >>> /**\n> >>> * \\brief Retrieve the image format set on one of the V4L2 subdevice pads\n> >>> * \\param[in] pad The 0-indexed pad number the format is to be retrieved from","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 9B60D610B6\n\tfor ;\n\tMon, 25 Feb 2019 20:51:05 +0100 (CET)","from pendragon.ideasonboard.com (unknown [83.145.195.18])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D3AB667;\n\tMon, 25 Feb 2019 20:51:04 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1551124265;\n\tbh=ehL4A4FtfGiQqnuqheP9R8Wli8YKTEQhc8fi2t8jVBw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gZ8zRX0KnQYKJZrTL/1d939wo4GCri0VzhlumkKaUxzvE/B49QQxzxdb1O7KCrQYI\n\tmeiFD1huLjBxo9gEECAw7sQkMww8DEK013H1GNK+68yUiWNjlMlgJ+Vy3k4TbEcxfo\n\tmNXYZVp80yrsxyy9oaT/7IMCuYd7LYzK8C56iDgc=","Date":"Mon, 25 Feb 2019 21:50:55 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"Niklas =?utf-8?q?S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190225195055.GE4756@pendragon.ideasonboard.com>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>\n\t<20190221153156.GD11484@bigcity.dyn.berto.se>\n\t<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>\n\t<20190225094435.diy2qjlhi5f4qx7e@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190225094435.diy2qjlhi5f4qx7e@uno.localdomain>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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, 25 Feb 2019 19:51:05 -0000"}},{"id":881,"web_url":"https://patchwork.libcamera.org/comment/881/","msgid":"<20190225223516.GG899@bigcity.dyn.berto.se>","date":"2019-02-25T22:35:16","subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo, Laurent,\n\n> > >\n> > > All V4L2Subdevice methods accepts a pad as first argument. I would\n> > > keep this consistent.\n> > >\n> > >>> +\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> > >>\n> > >> I think this should be configurable by the caller or other way\n> > >> customisable. In the not so distant future we will need to add support\n> > >> for V4L2_SUBDEV_FORMAT_TRY in a lot of places better to prepare for it\n> > >> :-)\n> > >\n> > > I'm really not sure about this one, I could add a field to\n> > > V4L2SubdeviceFormatEnum for this...\n> > >\n> > \n> > Re-thinking about this, I would leave this out until we don't define a\n> > way to handle FORMAT_TRY globally, as otherwise I would have to expose\n> > that flag outside of V4L2Subdevice which I would prefer not to...\n> \n> I think you made the right decision.\n\nAfter mocking around a bit more in the code I now agree with you both. \nHard coding of V4L2_SUBDEV_FORMAT_ACTIVE is how it's done in other \nplaces so we can change this when we add V4L2_SUBDEV_FORMAT_TRY in the \nfuture. Sorry for not noticing this in my first review.","headers":{"Return-Path":"","Received":["from mail-lf1-x132.google.com (mail-lf1-x132.google.com\n\t[IPv6:2a00:1450:4864:20::132])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCF35600FC\n\tfor ;\n\tMon, 25 Feb 2019 23:35:18 +0100 (CET)","by mail-lf1-x132.google.com with SMTP id n15so8142020lfe.5\n\tfor ;\n\tMon, 25 Feb 2019 14:35:18 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\t2sm2524059ljh.41.2019.02.25.14.35.17\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 25 Feb 2019 14:35:17 -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=A/upeoXb01yHWgYUYCUJLm1mKxXY35ljHHaIyHCVzDU=;\n\tb=hxhIANp3dhKD9b6N5mvRF3pY8pOccWL1gHGK34zAFKPkU3joB80eFX0f7oNQuozswU\n\txzgm/KtVCcbN0m6CtNjZXT8ONCOMFf32IjG2/6T0mcS7torPXSKhFWEkuBVYOWRSYomp\n\tYLIRzd5RTBEjLVnlNZ1LWlnabfcTdpGWACe0Wfb7hc2cTYYV6LMkfCwoJPfBZZlHR/Ly\n\tszNpU9Ts65RuFXhqr9S9IugH14lrWCTkAWzEKULZjeMcWtHgWNTN99r4Di52vOJK9Fr6\n\tOBYQnOGE1YkVK4sxMPLy+VRevQmlkigl+aUDbWqHSTgnprsnYTEePi+lQ6dTi9ok6w6+\n\tDuoA==","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=A/upeoXb01yHWgYUYCUJLm1mKxXY35ljHHaIyHCVzDU=;\n\tb=DO4Q4wcRvSwjufFUGs63FfSPa+vj8xIimBKAILLOCxevgevsPM2lFwoZo/zUBLs5uc\n\tVmETXEU7nq0+ca7ns8krK7AHtl2dbTSigPkCkZrAgEU79Op1Q3nBEDhU89uvmUrfH4Qi\n\tPjw5YbAQ0chfFbfwEyfrXVDrEqobZHIMt7YDJN/mOM7rd9mdsrfU38/fC/1yGBY3CjmD\n\tWpZDIIwDfipDmL35WdM8vZgq1dg0FM89gSXcwOjazlIlPc9Pr6p8fA4q1DzbeuXjIyV9\n\thNx87J/fJgMilCnb1C7p6+/xCFM2LXD/5784eQ4nCOlROzPeCihAIUXda2Y9xogXUHTz\n\tL//g==","X-Gm-Message-State":"AHQUAuacOCInh/Uty+13RnqywhALfq9H5jo1BdCtpzGyrWyETUC/ZyE/\n\tmhn5Rs7rDn2tKi6s6wlgcoXd6oahFk8=","X-Google-Smtp-Source":"AHgI3IYic4Uj484ulYemtbhh5UdukCU1ePbw3FuekKszHJo6emQysntG06u26f648cxQl+CJ7IwWSg==","X-Received":"by 2002:a19:5205:: with SMTP id m5mr11588665lfb.61.1551134117974;\n\tMon, 25 Feb 2019 14:35:17 -0800 (PST)","Date":"Mon, 25 Feb 2019 23:35:16 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"Jacopo Mondi , libcamera-devel@lists.libcamera.org","Message-ID":"<20190225223516.GG899@bigcity.dyn.berto.se>","References":"<20190219165620.2385-1-jacopo@jmondi.org>\n\t<20190219165620.2385-2-jacopo@jmondi.org>\n\t<20190221153156.GD11484@bigcity.dyn.berto.se>\n\t<20190225092833.x64hqbmkjkm7ukjw@uno.localdomain>\n\t<20190225094435.diy2qjlhi5f4qx7e@uno.localdomain>\n\t<20190225195055.GE4756@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":"<20190225195055.GE4756@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 1/3] libcamera: v4l2_subdevice:\n\tImplement ENUM_FRAME_SIZES","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, 25 Feb 2019 22:35:19 -0000"}}]