[{"id":21804,"web_url":"https://patchwork.libcamera.org/comment/21804/","msgid":"<CAJAuwMmpNx==HfYcN-WKs7P6Eh-=s809G-p5QYKgfGD_gmagrQ@mail.gmail.com>","date":"2021-12-16T10:51:02","subject":"Re: [libcamera-devel] [RFC PATCH 0/1] Autofocus controls","submitter":{"id":98,"url":"https://patchwork.libcamera.org/api/people/98/","name":"Hanlin Chen","email":"hanlinchen@chromium.org"},"content":"Hi David, Thanks for the work!\n\n\nOn Mon, Dec 13, 2021 at 11:22 PM David Plowman\n<david.plowman@raspberrypi.com> wrote:\n>\n> Hi everyone\n>\n> I thought it was time for a slightly more formal API proposal for AF\n> (autofocus).\n>\n> I've sent a patch listing all the new controls that I think we would\n> need. These controls cover both the use-cases that I can imagine we\n> (Raspberry Pi) would expect, and I think it covers all the Android\n> use-cases too. Nonetheless, I think a few words of additional\n> discussion and questions are warranted.\n>\n>\n> 1. AfState\n>\n> This closely follows what Android does, only Android duplicates them\n> all, once for \"passive\" (aka. continuous) AF and once for \"active\"\n> (user-triggered) AF. I can't really see why duplicating them would be\n> necessary, it seems pretty clear to me what mode we're in, but of\n> course we could if there is a good reason.\nI think it's related to the \"pause\" control mentioned below.\n\n>\n> I wondered a bit about having an \"Error\" state. Maybe that would get\n> reported if the AfWindow rectangles are ill-defined?\n>\n> 2. AfWindow\n>\n> Allowing applications to set the AF windows seems required. For\n> example, you might be running some kind of object or face detector,\n> and then you want to feed those results back to the AF algorithm.\n>\n> I was unsure what units to use for the Rectangles. It seemed to me\n> that you'd want the rectangles automatically to follow whatever\n> ScalerCrop you are using. For example, doing digital zoom and leaving\n> the focus windows in a part of the image you can't see seems\n> particularly unhelpful!\n>\n> So anyway, I went with the units being \"a proportion of the ScalerCrop\n> region\". Can we think of something better? Would floating point\n> Rectangles be less annoying?\n>\n> 3. AfMode\n>\n> As you probably already know, I'm not really seeing the need for an\n> \"off\" mode as it seems the same as \"auto\" to me - so I didn't define\n> one.\n>\n> One thing I wondered about was \"pausing\" CAF (continuous AF). You\n> might want to let it run, then \"pause\" for a moment to take some\n> captures, and then let it re-start from where it was previously. How\n> to accomplish this?\n>\n> We could have a separate \"pause\" control, just for CAF. Or I've\n> suggested that you could set the mode back to \"auto\" which would have\n> the same effect. But then how would you go back to CAF \"from where it\n> left off\" as opposed to letting it re-start as though it wasn't\n> running before? Maybe the \"pause\" control isn't such a bad idea.\n\nI would vote for the \"pause\" control. I tried to map the combination\nof pause and AfState with Android's definition.\nIt describes the \"passive\" cases.\n\nPASSIVE_SCAN = CAF + AfStateScanning\nPASSIVE_FOCUSED = CAF + AfStateFocused\nPASSIVE_UNFOCUSED = CAF + AfStateFailed\n\nACTIVE_SCAN = CAF + AfStateScanning + pause on\nFOCUSED_LOCKED = CAF + AfStateFocused + paused\nNOT_FOCUSED_LOCKED = CAF + AfStateFailed + paused\n\nThe difference between PASSIVE_FOCUSED and FOCUSED_LOCKED are:\nPASSIVE_FOCUSED tells the user \"Although it's focused, it may change\nanytime due to CAF\".\nFOCUSED_LOCKED tell the user \"It's focused, and won't change unless\nthe user stop pausing even in CAF mode\"\n\nAndroid use the combination to describe \"pause\":\nCAF + TriggerStart = pause on\nCAF + TriggerCancel = pause off\n\nI notice that the TriggerStart is forbidden for CAF in the Patch, but\nI think maybe \"pause\" is a more clear definition for CAF.\nAnd the user doesn't need to do the trick by changing modes.\n\nAnother thing I'd like to notice is that:\n\"CAF + TriggerStart\" in Android is not going to abruptly \"pause\" if\nCAF is already scanning, but wait for the scan to finish, and then\npause.\n\n>\n> 4. LensPosition\n>\n> I've proposed using the lens driver's units for controlling the lens\n> position. It's easy and naturally mimics what we tend to do for\n> exposure/gain.\n>\n> Another option would be to go a bit more Android-y and insist that\n> zero means \"infinity\". Android seems to categorise lenses into\n> \"calibrated\" and \"uncalibrated\" types. For the latter type, it looks\n> like pretty much the only stipulation is that zero means\n> \"infinity\". For \"calibrated\" lenses the units are formally defined to\n> be dioptres, but I'm thinking that we don't need to go there right\n> now.\n>\n> There's also the question of exactly where we get the lens range from.\n> It will be different for each module, even ones using the same lens\n> and/or driver - it will in each case be a different subset of the\n> range that the driver advertises. But I think that's not a question\n> for this control API.\n>\n>\n> Hopefully all that's enough to start taking this forward. There are of\n> course lots of points of detail to think about here, but clearly the\n> \"big picture\" is most important in the first instance.\n>\n> Thanks!\n>\n> David\n>\n> David Plowman (1):\n>   libcamera: controls: Controls for driving AF (autofocus) algorithms\n>\n>  src/libcamera/control_ids.yaml | 227 ++++++++++++++++++++++++---------\n>  1 file changed, 167 insertions(+), 60 deletions(-)\n>\n> --\n> 2.30.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 41FA6BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 16 Dec 2021 10:51:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 400A5608A5;\n\tThu, 16 Dec 2021 11:51:16 +0100 (CET)","from mail-oi1-x235.google.com (mail-oi1-x235.google.com\n\t[IPv6:2607:f8b0:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7086460115\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Dec 2021 11:51:14 +0100 (CET)","by mail-oi1-x235.google.com with SMTP id bk14so35931192oib.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 16 Dec 2021 02:51:14 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"c49oJqVP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=isHzGwiBDO0vRbboGOZUOuTyMFmkgO2CUpsVNhOvgZs=;\n\tb=c49oJqVPLwUf/KyQdZQ5Ji6F0Sbo8E7GzBfHJjld9ZauGSsZWSPMQdJOcAbJ2vsuR6\n\t9IG1fUl/jrfW0fj17x1FdijjaZ5O1xCtc+r5dHS21sfYA5RTZuQg1RRGBzkU0qSJ2uqF\n\tFJqQRdyT3n+ZwZJyLGCr6+C8XSySmMPFcL9kk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=isHzGwiBDO0vRbboGOZUOuTyMFmkgO2CUpsVNhOvgZs=;\n\tb=X/MO7sLXNjVZ2SRr0mElL+6p26+1Joe1v8wk3q9BmGDaYYxfs29qBUmvipd+lYNGbI\n\tH46xo1KY+/57KCYZcRuVWXKtkYb3cQpXn8wXFB/xhFkslnL85Bgi6UMdG/0oZApkvid9\n\txXp5fuc6dbfDLCAdNHJnBWct/CZ1NLZPtyGPrtUhuV/D+opEdwZRfogrLiV0eS+mLm+t\n\t/KCUCt5+7v8HmffWqCSwCV+aaKHbmdiopYIh0X4iAFGjbfLuqU5jv8kL6Hp2NIe8f183\n\tZL5mi8gk31+WU52uFpv3ZmMBGUS+BhUtiqFrgIybpdw3ZsN/lsIVR/96tLLYaT3ukiCy\n\tb0dA==","X-Gm-Message-State":"AOAM533iy7oVN2Xrc6zo7AxDGMEwgI6ClcpUzI28GC0A4QUSHD7HFF4I\n\tJYxFeZvj4Rr3Vz08+DLnnoc2mVCJag9Tsu5d0w1V8Q==","X-Google-Smtp-Source":"ABdhPJzUXl9vhxnc8rcal2iPQ7qlenaq/O+22gcm7YmLkmM4Ydb8Bu/dS32EzJKLU4CTvhqYckf9g/RnQ9FhXW8eqyI=","X-Received":"by 2002:a54:4401:: with SMTP id k1mr3635194oiw.143.1639651872945;\n\tThu, 16 Dec 2021 02:51:12 -0800 (PST)","MIME-Version":"1.0","References":"<20211213152215.17584-1-david.plowman@raspberrypi.com>","In-Reply-To":"<20211213152215.17584-1-david.plowman@raspberrypi.com>","From":"Hanlin Chen <hanlinchen@chromium.org>","Date":"Thu, 16 Dec 2021 18:51:02 +0800","Message-ID":"<CAJAuwMmpNx==HfYcN-WKs7P6Eh-=s809G-p5QYKgfGD_gmagrQ@mail.gmail.com>","To":"David Plowman <david.plowman@raspberrypi.com>","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [RFC PATCH 0/1] Autofocus controls","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","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>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]