[{"id":5075,"web_url":"https://patchwork.libcamera.org/comment/5075/","msgid":"<20200605230418.GO26752@pendragon.ideasonboard.com>","date":"2020-06-05T23:04:18","subject":"Re: [libcamera-devel] [PATCH 0/5] ImageFormats' not dead","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 Fri, May 29, 2020 at 01:03:30PM +0200, Jacopo Mondi wrote:\n> Hello, this might be controversial, as ImageFormats has been a target for\n> deprecation since a long time. But as of now, it's still alive and kicking, and\n> it has a few merits in my opinion, so this series tries to refactor it and its\n> users to make it nicer to work with.\n\nI still think (hope ?) we'll come up with something better in the long\nterm, but if the class has value for now, I certainly welcome a good\nrefactoring :-)\n\n> First and foremost, ImageFormats was designed to be used by both video devices\n> and video subdevices, but since the introduction of V4L2PixelFormat the formats\n> map cannot be represented anymore with ImageFormats, as it indexes the image\n> resolutions with a numerical index. As a consequence, pipeline handlers and\n> V4L2VideoDevice fell back on using a raw map in place of ImageFormats.\n> \n> To recover the situation, ImageFormats is turned into a templated class, capable\n> of indexing image resolutions with keys of variadic type.\n> \n> This allows reintroducing ImageFormats in V4L2VideoDevice, with its\n> specialization ImageFormats.\n> \n> On top of this the last three patches renames ImageFormats, provides typedef\n> for V4L2VideoDevice and V4L2Subdevice formats and renames the formats() method\n> in those classes whose name collides with ImageFormats::formats.\n> \n> The new usage pattern looks like\n> \n> V4L2VideoDevice::formatsMap = device_->imageFormats();\n> for (const V4L2PixelFormat &v4lFormat : formatsMaps().formats()) {\n> \tfor (const SizeRange &sizes : formatsMap.sizes(v4l2Format) {\n> \n> \t}\n> }\n> \n> compared to\n> \n> std::map> formats = device_->formats();\n> for (const auto it: formats) {\n> \tV4L2PixelFormat &v4l2Format = it.first;\n> \tfor (const SizeRange &sizes : it.second) {\n> \n> \t}\n> }\n> \n> While for subdevice it stays similar\n> \n> V4L2Subdevice::formatsMap formatsMap = subdev_->imageFormats();\n> for (const uint32_t code : formatsMap().formats()) {\n> \tfor (const SizeRange &sizes : formatsMap.sizes(v4l2Format) {\n> \n> \t}\n> }\n> \n> Jacopo Mondi (5):\n> libcamera: formats: Make ImageFormats a templated class\n> libcamera: v4l2_videodevice: Use ImageFormats\n> libcamera: Rename ImageFormats\n> libcamera: v4l2_device: Add formatsMap typedef\n> libcamera: v4l2_device: Rename formats() method\n> \n> include/libcamera/internal/camera_sensor.h | 6 +-\n> include/libcamera/internal/formats.h | 66 ++++++++--\n> include/libcamera/internal/v4l2_subdevice.h | 4 +-\n> include/libcamera/internal/v4l2_videodevice.h | 4 +-\n> src/libcamera/camera_sensor.cpp | 2 +-\n> src/libcamera/formats.cpp | 119 +++++++++++-------\n> .../pipeline/raspberrypi/raspberrypi.cpp | 24 ++--\n> src/libcamera/pipeline/simple/simple.cpp | 3 +-\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 3 +-\n> src/libcamera/v4l2_subdevice.cpp | 11 +-\n> src/libcamera/v4l2_videodevice.cpp | 9 +-\n> test/v4l2_subdevice/list_formats.cpp | 8 +-\n> 12 files changed, 175 insertions(+), 84 deletions(-)\n>","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 C309B603C8\n\tfor ;\n\tSat, 6 Jun 2020 01:04:40 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C47CC27C;\n\tSat, 6 Jun 2020 01:04:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rfexV5nv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591398280;\n\tbh=hbmu0u3UMb+eWZiPkxketHxbvypfbFWq2BAdiT8Jwig=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=rfexV5nvHXerq+VLu6D2bdvjx1X6edwvYXX9iQBecXNCkuoIvuiKGbt39WQyGlnmI\n\tx76AYxD62PtbfB9rl4n28gNGugwdpzBhRMVIPtfYRGWMAYoFfW/39KdU+iUddcgAYi\n\t1akHflnGwjrqw/QccleVDt41nyC8mvJisPny0Qdk=","Date":"Sat, 6 Jun 2020 02:04:18 +0300","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200605230418.GO26752@pendragon.ideasonboard.com>","References":"<20200529110335.620503-1-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200529110335.620503-1-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH 0/5] ImageFormats' not dead","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 05 Jun 2020 23:04:41 -0000"}}]