[{"id":883,"web_url":"https://patchwork.libcamera.org/comment/883/","msgid":"<20190226023942.GI899@bigcity.dyn.berto.se>","date":"2019-02-26T02:39:42","subject":"Re: [libcamera-devel] [PATCH v2 2/6] 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-25 13:10:33 +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 <jacopo@jmondi.org>\n> ---\n>  src/libcamera/include/v4l2_subdevice.h |  8 +++\n>  src/libcamera/v4l2_subdevice.cpp       | 93 ++++++++++++++++++++++++++\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 fcfbee5af106..cb033a76933c 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -7,7 +7,9 @@\n>  #ifndef __LIBCAMERA_V4L2_SUBDEVICE_H__\n>  #define __LIBCAMERA_V4L2_SUBDEVICE_H__\n>  \n> +#include <map>\n>  #include <string>\n> +#include <vector>\n>  \n>  #include \"media_object.h\"\n>  \n> @@ -38,16 +40,22 @@ public:\n>  \tint setCrop(unsigned int pad, Rectangle *rect);\n>  \tint setCompose(unsigned int pad, Rectangle *rect);\n>  \n> +\tstd::vector<V4L2SubdeviceFormat> &formats(unsigned int pad);\n\nAs you return a referent to the internal data structure should it not \nat lest be const?\n\n>  \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n>  \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n>  \n>  private:\n> +\tint listPadFormats(unsigned int pad,\n> +\t\t\t   std::vector<V4L2SubdeviceFormat> *formats);\n> +\tvoid listFormats();\n>  \tint setSelection(unsigned int pad, unsigned int target,\n>  \t\t\t Rectangle *rect);\n>  \n>  \tconst MediaEntity *entity_;\n>  \tstd::string deviceNode_;\n>  \tint fd_;\n> +\n> +\tstd::map<unsigned int, std::vector<V4L2SubdeviceFormat>> formats_;\n>  };\n>  \n>  } /* namespace libcamera */\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index ebf87f0124cb..0e9c654579dc 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -5,6 +5,10 @@\n>   * v4l2_subdevice.cpp - V4L2 Subdevice\n>   */\n>  \n> +#include <map>\n> +#include <string>\n> +#include <vector>\n> +\n>  #include <fcntl.h>\n>  #include <string.h>\n>  #include <sys/ioctl.h>\n> @@ -116,6 +120,8 @@ int V4L2Subdevice::open()\n>  \t}\n>  \tfd_ = ret;\n>  \n> +\tlistFormats();\n> +\n\nI would inline the function here, but I won't push it as its mostly \ntaste.\n\n>  \treturn 0;\n>  }\n>  \n> @@ -178,6 +184,17 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n>  \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n>  }\n>  \n> +/**\n> + * \\brief List the sub-device image resolutions and formats on \\a pad\n> + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> + *\n> + * \\return A vector of image formats, or an empty vector on error\n> + */\n> +std::vector<V4L2SubdeviceFormat> &V4L2Subdevice::formats(unsigned int pad)\n> +{\n> +\treturn formats_[pad];\n\nMaybe add a bounds checking on pad here? What would happen if a pad that \ndon't exists is requested?\n\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> @@ -242,6 +259,82 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n>  \treturn 0;\n>  }\n>  \n> +int V4L2Subdevice::listPadFormats(unsigned int pad,\n> +\t\t\t\t  std::vector<V4L2SubdeviceFormat> *formats)\n> +{\n> +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> +\tstruct v4l2_subdev_mbus_code_enum mbusEnum = {};\n> +\tint ret;\n> +\n> +\tmbusEnum.index = 0;\n> +\tmbusEnum.pad = pad;\n> +\tmbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> +\n> +\twhile (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum))) {\n> +\t\tsizeEnum.index = 0;\n> +\t\tsizeEnum.code = mbusEnum.code;\n> +\t\tsizeEnum.pad = pad;\n> +\t\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> +\n> +\t\twhile (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE,\n> +\t\t\t\t     &sizeEnum))) {\n> +\n> +\t\t\t/* Store the minimum and maximum reported sizes. */\n> +\t\t\tV4L2SubdeviceFormat minFormat = {\n> +\t\t\t\t.mbus_code = mbusEnum.code,\n> +\t\t\t\t.width = sizeEnum.min_width,\n> +\t\t\t\t.height = sizeEnum.min_height,\n> +\t\t\t};\n> +\t\t\tformats->push_back(minFormat);\n> +\n> +\t\t\tV4L2SubdeviceFormat maxFormat = {\n> +\t\t\t\t.mbus_code = mbusEnum.code,\n> +\t\t\t\t.width = sizeEnum.max_width,\n> +\t\t\t\t.height = sizeEnum.max_height,\n> +\t\t\t};\n> +\t\t\tformats->push_back(maxFormat);\n> +\n> +\t\t\tsizeEnum.index++;\n> +\t\t}\n> +\n> +\t\tif (-errno != -EINVAL) {\n> +\t\t\tLOG(V4L2Subdev, Error)\n> +\t\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> +\t\t\t\t<< \" of \" << deviceNode_ << \": \"\n> +\t\t\t\t<< strerror(-ret);\n> +\t\t\treturn ret;\n> +\t\t}\n> +\n> +\t\tmbusEnum.index++;\n> +\t}\n> +\n> +\tif (-errno != -EINVAL) {\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> +\treturn 0;\n> +}\n> +\n> +void V4L2Subdevice::listFormats()\n> +{\n> +\tint ret;\n> +\n> +\tfor (MediaPad *pad : entity_->pads()) {\n> +\t\tstd::vector<V4L2SubdeviceFormat> formats = {};\n> +\t\tret = listPadFormats(pad->index(), &formats);\n\nCan't listPadFormats() return the vector instead of taking a pointer as \nan argument? With copy elision there would be no performance impact.\n\n> +\t\tif (ret) {\n> +\t\t\tformats = {};\n> +\t\t\tformats_[pad->index()] = formats;\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tformats_[pad->index()] = formats;\n\nThis can be simplified, how about\n\n    for (MediaPad *pad : entity_->pads()) {\n        std::vector<V4L2SubdeviceFormat> formats = {};\n        ret = listPadFormats(pad->index(), &formats);\n        if (ret)\n            formats = {}\n\n        formats_[pad->index()] = formats;\n    }\n\n\nOr with the return type of listPadFormats() changed\n\n    for (MediaPad *pad : entity_->pads())\n        formats_[pad->index()] = listPadFormats(pad->index());\n\n\n> +\t}\n> +}\n> +\n>  int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n>  \t\t\t\tRectangle *rect)\n>  {\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":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lf1-x133.google.com (mail-lf1-x133.google.com\n\t[IPv6:2a00:1450:4864:20::133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3141F601E2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Feb 2019 03:39:44 +0100 (CET)","by mail-lf1-x133.google.com with SMTP id g12so8443728lfb.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 25 Feb 2019 18:39:44 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tq12sm563371lfc.8.2019.02.25.18.39.42\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 25 Feb 2019 18:39:42 -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=GP21TtouSHz5D5a+2UvHRDFDm8bTWW+bRXR8H7GHLbw=;\n\tb=qCNEKjVxRpdo2eV7ajIMkcxPlzPEcUi4iBBo1/ddIUt6ROkg3FYjjfXM/MQWR27few\n\tgpxX6YiaYGVwdvHdA/rcsEmWEFygxio5PO5/kHdivtypJrFJiBsNyxBZNA64lI6Jjo0+\n\tZhfyx95gPNi5BTCuegW24cJPlqWkeR2k8TtcbouZveqCL8vZJstIPebOy90E4poAgx+b\n\t6qzx64Djv6g8D3d0sICd+Zyl6XQygl0ZwrRe3Lm5tuWLy5O3L2/nf5tWeGA9gAMueFfS\n\t5jkYqeenez9IopobpBqeQs3cB3XS++K+CNPjP+wamDKMxVDW4oZwODhnHY6bnHVyOXXU\n\tOl2g==","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=GP21TtouSHz5D5a+2UvHRDFDm8bTWW+bRXR8H7GHLbw=;\n\tb=WrrOVHTEh81/nebupL6eDlROqtJqF4bVOTwSRlBXaJbe7n6C7mvFxSHfsGuwzgIj1M\n\tv+LDqDaF9uzqBMPQ1D7Q6xSODXkucqDCWgGpfWC90fnjAwsCs+DNCEm9lr6ZeM5QVMKn\n\t2mH7/XmbEADsSLAEEpJMnDRHiM8Ph2xDMVoNyFNZrXd7UAfea8BMUQPLVivuor9hULkF\n\tyvFovaEbLzN8bwqdqCWKofbHMMb5q5YlYoRtB8snxKfOwIYxSKfG+b4srZz0afj9HR74\n\tpit+1a3dXCPs1QUOxV+hEKT8cWdWIhzmFA26UJYtAgTOafVCPh8eM0Jk8y6sEwovpKh/\n\t9bxQ==","X-Gm-Message-State":"AHQUAuYaxArxzIYzwjCxtt1eb6tb+rLumkWUofKzSrZP/kKk7qz+be5+\n\tSa63euaatB4TW+SbYDJNe2BjqQ==","X-Google-Smtp-Source":"AHgI3IZfQSc0FNr2VjyY+StN0qe/nVJ/A+qyWMZijX9Bhx/Irgre/5WLbLAd2UhGkRuj7P7wCVKTPA==","X-Received":"by 2002:a19:4f03:: with SMTP id d3mr10794951lfb.52.1551148783262;\n\tMon, 25 Feb 2019 18:39:43 -0800 (PST)","Date":"Tue, 26 Feb 2019 03:39:42 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Jacopo Mondi <jacopo@jmondi.org>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190226023942.GI899@bigcity.dyn.berto.se>","References":"<20190225121037.11415-1-jacopo@jmondi.org>\n\t<20190225121037.11415-3-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":"<20190225121037.11415-3-jacopo@jmondi.org>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 26 Feb 2019 02:39:44 -0000"}},{"id":886,"web_url":"https://patchwork.libcamera.org/comment/886/","msgid":"<20190226082736.fcfqhqx4hqlrsiig@uno.localdomain>","date":"2019-02-26T08:27:36","subject":"Re: [libcamera-devel] [PATCH v2 2/6] 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   thanks for review.\n\nOn Tue, Feb 26, 2019 at 03:39:42AM +0100, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your patch.\n>\n> On 2019-02-25 13:10:33 +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 <jacopo@jmondi.org>\n> > ---\n> >  src/libcamera/include/v4l2_subdevice.h |  8 +++\n> >  src/libcamera/v4l2_subdevice.cpp       | 93 ++++++++++++++++++++++++++\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 fcfbee5af106..cb033a76933c 100644\n> > --- a/src/libcamera/include/v4l2_subdevice.h\n> > +++ b/src/libcamera/include/v4l2_subdevice.h\n> > @@ -7,7 +7,9 @@\n> >  #ifndef __LIBCAMERA_V4L2_SUBDEVICE_H__\n> >  #define __LIBCAMERA_V4L2_SUBDEVICE_H__\n> >\n> > +#include <map>\n> >  #include <string>\n> > +#include <vector>\n> >\n> >  #include \"media_object.h\"\n> >\n> > @@ -38,16 +40,22 @@ public:\n> >  \tint setCrop(unsigned int pad, Rectangle *rect);\n> >  \tint setCompose(unsigned int pad, Rectangle *rect);\n> >\n> > +\tstd::vector<V4L2SubdeviceFormat> &formats(unsigned int pad);\n>\n> As you return a referent to the internal data structure should it not\n> at lest be const?\n\nPossibly, yes. I'll try to make it const.\n\n>\n> >  \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> >  \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> >\n> >  private:\n> > +\tint listPadFormats(unsigned int pad,\n> > +\t\t\t   std::vector<V4L2SubdeviceFormat> *formats);\n> > +\tvoid listFormats();\n> >  \tint setSelection(unsigned int pad, unsigned int target,\n> >  \t\t\t Rectangle *rect);\n> >\n> >  \tconst MediaEntity *entity_;\n> >  \tstd::string deviceNode_;\n> >  \tint fd_;\n> > +\n> > +\tstd::map<unsigned int, std::vector<V4L2SubdeviceFormat>> formats_;\n> >  };\n> >\n> >  } /* namespace libcamera */\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index ebf87f0124cb..0e9c654579dc 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -5,6 +5,10 @@\n> >   * v4l2_subdevice.cpp - V4L2 Subdevice\n> >   */\n> >\n> > +#include <map>\n> > +#include <string>\n> > +#include <vector>\n> > +\n> >  #include <fcntl.h>\n> >  #include <string.h>\n> >  #include <sys/ioctl.h>\n> > @@ -116,6 +120,8 @@ int V4L2Subdevice::open()\n> >  \t}\n> >  \tfd_ = ret;\n> >\n> > +\tlistFormats();\n> > +\n>\n> I would inline the function here, but I won't push it as its mostly\n> taste.\n>\n\nActually not sure I like it.\n\n> >  \treturn 0;\n> >  }\n> >\n> > @@ -178,6 +184,17 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> >  \treturn setSelection(pad, V4L2_SEL_TGT_COMPOSE, rect);\n> >  }\n> >\n> > +/**\n> > + * \\brief List the sub-device image resolutions and formats on \\a pad\n> > + * \\param[in] pad The 0-indexed pad number to enumerate formats on\n> > + *\n> > + * \\return A vector of image formats, or an empty vector on error\n> > + */\n> > +std::vector<V4L2SubdeviceFormat> &V4L2Subdevice::formats(unsigned int pad)\n> > +{\n> > +\treturn formats_[pad];\n>\n> Maybe add a bounds checking on pad here? What would happen if a pad that\n> don't exists is requested?\n>\n\nMy understanding is that an empty vector is constructed and returned\nin such a case (and gets stored in the formats map, this is a side\neffect we might want to avoid maybe).\n\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> > @@ -242,6 +259,82 @@ int V4L2Subdevice::setFormat(unsigned int pad, V4L2SubdeviceFormat *format)\n> >  \treturn 0;\n> >  }\n> >\n> > +int V4L2Subdevice::listPadFormats(unsigned int pad,\n> > +\t\t\t\t  std::vector<V4L2SubdeviceFormat> *formats)\n> > +{\n> > +\tstruct v4l2_subdev_frame_size_enum sizeEnum = {};\n> > +\tstruct v4l2_subdev_mbus_code_enum mbusEnum = {};\n> > +\tint ret;\n> > +\n> > +\tmbusEnum.index = 0;\n> > +\tmbusEnum.pad = pad;\n> > +\tmbusEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> > +\n> > +\twhile (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_MBUS_CODE, &mbusEnum))) {\n> > +\t\tsizeEnum.index = 0;\n> > +\t\tsizeEnum.code = mbusEnum.code;\n> > +\t\tsizeEnum.pad = pad;\n> > +\t\tsizeEnum.which = V4L2_SUBDEV_FORMAT_ACTIVE;\n> > +\n> > +\t\twhile (!(ret = ioctl(fd_, VIDIOC_SUBDEV_ENUM_FRAME_SIZE,\n> > +\t\t\t\t     &sizeEnum))) {\n> > +\n> > +\t\t\t/* Store the minimum and maximum reported sizes. */\n> > +\t\t\tV4L2SubdeviceFormat minFormat = {\n> > +\t\t\t\t.mbus_code = mbusEnum.code,\n> > +\t\t\t\t.width = sizeEnum.min_width,\n> > +\t\t\t\t.height = sizeEnum.min_height,\n> > +\t\t\t};\n> > +\t\t\tformats->push_back(minFormat);\n> > +\n> > +\t\t\tV4L2SubdeviceFormat maxFormat = {\n> > +\t\t\t\t.mbus_code = mbusEnum.code,\n> > +\t\t\t\t.width = sizeEnum.max_width,\n> > +\t\t\t\t.height = sizeEnum.max_height,\n> > +\t\t\t};\n> > +\t\t\tformats->push_back(maxFormat);\n> > +\n> > +\t\t\tsizeEnum.index++;\n> > +\t\t}\n> > +\n> > +\t\tif (-errno != -EINVAL) {\n> > +\t\t\tLOG(V4L2Subdev, Error)\n> > +\t\t\t\t<< \"Unable to enumerate format on pad \" << pad\n> > +\t\t\t\t<< \" of \" << deviceNode_ << \": \"\n> > +\t\t\t\t<< strerror(-ret);\n> > +\t\t\treturn ret;\n> > +\t\t}\n> > +\n> > +\t\tmbusEnum.index++;\n> > +\t}\n> > +\n> > +\tif (-errno != -EINVAL) {\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> > +\treturn 0;\n> > +}\n> > +\n> > +void V4L2Subdevice::listFormats()\n> > +{\n> > +\tint ret;\n> > +\n> > +\tfor (MediaPad *pad : entity_->pads()) {\n> > +\t\tstd::vector<V4L2SubdeviceFormat> formats = {};\n> > +\t\tret = listPadFormats(pad->index(), &formats);\n>\n> Can't listPadFormats() return the vector instead of taking a pointer as\n> an argument? With copy elision there would be no performance impact.\n>\n> > +\t\tif (ret) {\n> > +\t\t\tformats = {};\n> > +\t\t\tformats_[pad->index()] = formats;\n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > +\t\tformats_[pad->index()] = formats;\n>\n> This can be simplified, how about\n>\n>     for (MediaPad *pad : entity_->pads()) {\n>         std::vector<V4L2SubdeviceFormat> formats = {};\n>         ret = listPadFormats(pad->index(), &formats);\n>         if (ret)\n>             formats = {}\n>\n>         formats_[pad->index()] = formats;\n>     }\n>\n>\n> Or with the return type of listPadFormats() changed\n>\n>     for (MediaPad *pad : entity_->pads())\n>         formats_[pad->index()] = listPadFormats(pad->index());\n>\n>\n\nThanks for both suggestions, it looks better.\n\nAnyway, as soon as I've run this code on a 'real' device, I realized\nit needs to handle gracefully the case where that ioctl is not\nimplemented (-ENOTTY), so this code will likely be re-writted in v3.\n\nThanks\n  j\n\n> > +\t}\n> > +}\n> > +\n> >  int V4L2Subdevice::setSelection(unsigned int pad, unsigned int target,\n> >  \t\t\t\tRectangle *rect)\n> >  {\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":"<jacopo@jmondi.org>","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 835F8600FD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Feb 2019 09:27:08 +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 relay12.mail.gandi.net (Postfix) with ESMTPSA id 04035200006;\n\tTue, 26 Feb 2019 08:27:07 +0000 (UTC)"],"Date":"Tue, 26 Feb 2019 09:27:36 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190226082736.fcfqhqx4hqlrsiig@uno.localdomain>","References":"<20190225121037.11415-1-jacopo@jmondi.org>\n\t<20190225121037.11415-3-jacopo@jmondi.org>\n\t<20190226023942.GI899@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"2daomgmcxfekakzg\"","Content-Disposition":"inline","In-Reply-To":"<20190226023942.GI899@bigcity.dyn.berto.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 2/6] 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 26 Feb 2019 08:27:08 -0000"}}]