[{"id":13463,"web_url":"https://patchwork.libcamera.org/comment/13463/","msgid":"<20201023233758.GO5979@pendragon.ideasonboard.com>","date":"2020-10-23T23:37:58","subject":"Re: [libcamera-devel] [PATCH v5 0/5] Digital zoom","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nOn Fri, Oct 23, 2020 at 11:21:54AM +0100, David Plowman wrote:\n> Hi everyone\n> \n> Thanks for the reviews and comments, latest version of digital zoom\n> patches attached. I reordered the first couple because the ScalerCrop\n> control seems more fundamental now, so it seemed slightly better\n> before the ScalerCropMaximum one. Don't think it matters much.\n> \n> The main differences are:\n> \n> 1 and 2. These are now the control then the property, as described.\n> \n> 3. Initialisation of the ScalerCropMaximum property is now in the RPi\n> pipeline handler, and to zero values (isNull is our friend).\n> \n> 4. Geometry helpers - a bit more renaming, as usual. I found I had to\n> keep some of the casts because widths and heights tend to be unsigned\n> ints (not just ints). There's a Rectangle::topLeft() function and you\n> now translate by a Point (I'll live with my discomfort on that one).\n> \n> That Rectangle scaling function is now described as a \"non-uniform\n> scaling\", I think that's something familiar to most people. I agree it\n> was confusing before (I'd described it by the way I used it, rather\n> than what it was), but I think a non-unifrom scaling is clearer than a\n> homothety (actually I had some difficulties with that too).\n> \n> 5. RPi implementation. I fill in the ScalerCrop in the metadata\n> directly now, when the metadata returns from the IPA (which just\n> ignores it).\n\nAll this looks really good to me ! I've reviewed all patches, there are\na fair number of comments, but they're getting really minor. I think the\nnext iteration could be the final one.\n\n> I thought I'd get this lot out there first. In the meantime I'll work\n> on some geometry tests and include them either in the next round or as\n> a separate patch.\n\nThank you for that. The tests should be split to a separate patch, but\nif it can be included in the series, that would be best.\n\n> David Plowman (5):\n> libcamera: Add ScalerCrop control\n> libcamera: Add SensorCropMaximum property\n> libcamera: raspberrypi: Initialise the SensorCropMaximum property\n> libcamera: Add geometry helper functions\n> libcamera: pipeline: raspberrypi: Implementation of digital zoom\n> \n> include/libcamera/geometry.h | 52 +++\n> include/libcamera/ipa/raspberrypi.h | 1 +\n> src/ipa/raspberrypi/raspberrypi.cpp | 5 +\n> src/libcamera/control_ids.yaml | 13 +\n> src/libcamera/geometry.cpp | 307 ++++++++++++++++++\n> .../pipeline/raspberrypi/raspberrypi.cpp | 89 ++++-\n> src/libcamera/property_ids.yaml | 14 +\n> 7 files changed, 465 insertions(+), 16 deletions(-)","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D9066C3D3C\n\tfor ;\n\tFri, 23 Oct 2020 23:38:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3D48619AF;\n\tSat, 24 Oct 2020 01:38:46 +0200 (CEST)","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 66A38615D2\n\tfor ;\n\tSat, 24 Oct 2020 01:38:45 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D87AD9BF;\n\tSat, 24 Oct 2020 01:38:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GdhMWXuo\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1603496325;\n\tbh=hbXOwadmsgkbwWgeB7G0fmUhX8OpQ9rwsfklIJxMtvw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GdhMWXuoC/rRHSNsPsBVzsBx16hA/I3LQTXFF/StKUZyUYXskJc/chaXtuZP+SH78\n\tgVAMOwo3VhiWstnqFNG8umaTkw4AGoGF7aJYjaVXn1ZhqRWOhWlKELU2lhkxUc29fz\n\tWBD7dogwG6QXK35ui6Gf1zCiAkbQ5VhcVF9W02nA=","Date":"Sat, 24 Oct 2020 02:37:58 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20201023233758.GO5979@pendragon.ideasonboard.com>","References":"<20201023102159.26274-1-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201023102159.26274-1-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v5 0/5] Digital zoom","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]