[{"id":14839,"web_url":"https://patchwork.libcamera.org/comment/14839/","msgid":"<20210128102941.GA2167@pyrite.rasen.tech>","date":"2021-01-28T10:29:41","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi me,\n\nOn Thu, Jan 28, 2021 at 07:21:40PM +0900, Paul Elder wrote:\n> Set the AE precapture triggler tag in the android result metadata\n> according to what was passed in the request metadata.\n> \n> This allows the following CTS test to pass:\n> - android.hardware.camera2.cts.StillCaptureTest#testAePrecaptureTriggerCancelJpegCapture\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> \n> ---\n> Changes in v2:\n> - move setting the tag to getResultMetadata()\n> ---\n>  src/android/camera_device.cpp | 10 ++++++----\n>  1 file changed, 6 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 82bf0d3a..f5066709 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1965,8 +1965,11 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n>  \t\t\t\t aeFpsTarget.data(), aeFpsTarget.size());\n>  \n>  \tvalue = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> -\t\t\t\t &value, 1);\n> +\tcamera_metadata_ro_entry_t entry;\n> +\t/* \\todo Handle IPA appropriately */\n> +\tbool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);\n> +\tresultMetadata->updateEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n\nThis should be addEntry.\n\nPaul\n\n> +\t\t\t\t    ret ? entry.data.u8 : &value, 1);\n>  \n>  \tvalue = ANDROID_CONTROL_AE_STATE_CONVERGED;\n>  \tresultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);\n> @@ -2010,8 +2013,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n>  \tvalue = ANDROID_FLASH_STATE_UNAVAILABLE;\n>  \tresultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);\n>  \n> -\tcamera_metadata_ro_entry_t entry;\n> -\tint ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> +\tret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n>  \tif (ret)\n>  \t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\n>  \n> -- \n> 2.27.0\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 5F687C33BB\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jan 2021 10:29:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E47D468383;\n\tThu, 28 Jan 2021 11:29:48 +0100 (CET)","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 5640B6030A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 11:29:48 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2174D331\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 11:29:46 +0100 (CET)"],"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=\"P9W3eADB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611829788;\n\tbh=SUE7CUVhX7UqNBUSz65Wu2XMZBQ6Q/LvkBs2ps1fegg=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=P9W3eADBXg/gckI+KJxjn+IpoXnL1I3VXSKG+Fns+w92pOKoUScBNjUIun36nID2G\n\tT81kqA1wbSWcGaFk9aBO7QVwEkS/J4rB2Uj4VmMoejyZ2U5dzowaKHX0d9S8hdaAF4\n\tDhTd442i4aIHimuyclNQtl4VBMMBHuzAvFulroPo=","Date":"Thu, 28 Jan 2021 19:29:41 +0900","From":"paul.elder@ideasonboard.com","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20210128102941.GA2167@pyrite.rasen.tech>","References":"<20210128102140.160134-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210128102140.160134-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","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>","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14841,"web_url":"https://patchwork.libcamera.org/comment/14841/","msgid":"<20210128110303.7gz4lbvduufsfcbq@uno.localdomain>","date":"2021-01-28T11:03:03","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n\nOn Thu, Jan 28, 2021 at 07:21:40PM +0900, Paul Elder wrote:\n> Set the AE precapture triggler tag in the android result metadata\n> according to what was passed in the request metadata.\n>\n> This allows the following CTS test to pass:\n> - android.hardware.camera2.cts.StillCaptureTest#testAePrecaptureTriggerCancelJpegCapture\n>\n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n>\n> ---\n> Changes in v2:\n> - move setting the tag to getResultMetadata()\n> ---\n>  src/android/camera_device.cpp | 10 ++++++----\n>  1 file changed, 6 insertions(+), 4 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 82bf0d3a..f5066709 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -1965,8 +1965,11 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n>  \t\t\t\t aeFpsTarget.data(), aeFpsTarget.size());\n>\n>  \tvalue = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;\n> -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> -\t\t\t\t &value, 1);\n> +\tcamera_metadata_ro_entry_t entry;\n\nYou can move this one at the beginning of the function. It is used in\nmany places, and we'll continue moving it otherwise.\n\n> +\t/* \\todo Handle IPA appropriately */\n> +\tbool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);\n> +\tresultMetadata->updateEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> +\t\t\t\t    ret ? entry.data.u8 : &value, 1);\n\nidk... we're blindly taking this from from the settings, while I think\nit would be worth plumbing it into the pipeline handler already,\neven if it will be functionally equivalent to what you have here.\n\nThere's also a draft control already defined for this.\n\nTo close the loop this would require to:\n- register the control in the pipeline handler (for IPU3 at initControls() time)\n- translate the android metadata into a libcamera control in\n  CameraDevice::processControls()\n- Return in the request complete handler of the pipeline handler the\n  value passed in the request settings (see how ScalerCrop is handled\n  in the IPU3 imguOutputBufferReady() slot)\n- Translate back the libcamera control from the request to the android\n  metadata (look again at how ScalerCrop is handled in\n  getResultMetadata())\n\nThis might seems like a useless exercize, but once the IPA needs to\nhandle this control for real, this will have to be done and I would\nnot delay it.\n\nLaurent, do you think this is worth going to the pipeline handler loop\nalready ?\n\nThanks\n  j\n\n>\n>  \tvalue = ANDROID_CONTROL_AE_STATE_CONVERGED;\n>  \tresultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);\n> @@ -2010,8 +2013,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n>  \tvalue = ANDROID_FLASH_STATE_UNAVAILABLE;\n>  \tresultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);\n>\n> -\tcamera_metadata_ro_entry_t entry;\n> -\tint ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> +\tret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n>  \tif (ret)\n>  \t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\n>\n> --\n> 2.27.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 97DF8BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jan 2021 11:02:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2716868389;\n\tThu, 28 Jan 2021 12:02:44 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6F785682A0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 12:02:43 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 082CBFF816;\n\tThu, 28 Jan 2021 11:02:42 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Thu, 28 Jan 2021 12:03:03 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Paul Elder <paul.elder@ideasonboard.com>","Message-ID":"<20210128110303.7gz4lbvduufsfcbq@uno.localdomain>","References":"<20210128102140.160134-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210128102140.160134-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14842,"web_url":"https://patchwork.libcamera.org/comment/14842/","msgid":"<YBKdAKmHj2MSSaeo@pendragon.ideasonboard.com>","date":"2021-01-28T11:16:16","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo and Paul,\n\nOn Thu, Jan 28, 2021 at 12:03:03PM +0100, Jacopo Mondi wrote:\n> On Thu, Jan 28, 2021 at 07:21:40PM +0900, Paul Elder wrote:\n> > Set the AE precapture triggler tag in the android result metadata\n> > according to what was passed in the request metadata.\n> >\n> > This allows the following CTS test to pass:\n> > - android.hardware.camera2.cts.StillCaptureTest#testAePrecaptureTriggerCancelJpegCapture\n> >\n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> >\n> > ---\n> > Changes in v2:\n> > - move setting the tag to getResultMetadata()\n> > ---\n> >  src/android/camera_device.cpp | 10 ++++++----\n> >  1 file changed, 6 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 82bf0d3a..f5066709 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -1965,8 +1965,11 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> >  \t\t\t\t aeFpsTarget.data(), aeFpsTarget.size());\n> >\n> >  \tvalue = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;\n> > -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> > -\t\t\t\t &value, 1);\n> > +\tcamera_metadata_ro_entry_t entry;\n> \n> You can move this one at the beginning of the function. It is used in\n> many places, and we'll continue moving it otherwise.\n> \n> > +\t/* \\todo Handle IPA appropriately */\n> > +\tbool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);\n> > +\tresultMetadata->updateEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> > +\t\t\t\t    ret ? entry.data.u8 : &value, 1);\n> \n> idk... we're blindly taking this from from the settings, while I think\n> it would be worth plumbing it into the pipeline handler already,\n> even if it will be functionally equivalent to what you have here.\n> \n> There's also a draft control already defined for this.\n> \n> To close the loop this would require to:\n> - register the control in the pipeline handler (for IPU3 at initControls() time)\n> - translate the android metadata into a libcamera control in\n>   CameraDevice::processControls()\n> - Return in the request complete handler of the pipeline handler the\n>   value passed in the request settings (see how ScalerCrop is handled\n>   in the IPU3 imguOutputBufferReady() slot)\n> - Translate back the libcamera control from the request to the android\n>   metadata (look again at how ScalerCrop is handled in\n>   getResultMetadata())\n> \n> This might seems like a useless exercize, but once the IPA needs to\n> handle this control for real, this will have to be done and I would\n> not delay it.\n> \n> Laurent, do you think this is worth going to the pipeline handler loop\n> already ?\n\nThere's a state machine to we need to implement to handle the AE. It\ninvolves multiple controls. As we don't support still image capture yet\nin the libcamera core I think it could be too early to plumb this\nthrough, I'm be tempted to just go with this shortcut for the very short\nterm.\n\n> >  \tvalue = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> >  \tresultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);\n> > @@ -2010,8 +2013,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> >  \tvalue = ANDROID_FLASH_STATE_UNAVAILABLE;\n> >  \tresultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);\n> >\n> > -\tcamera_metadata_ro_entry_t entry;\n> > -\tint ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> > +\tret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> >  \tif (ret)\n> >  \t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\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 E21BBC33BB\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jan 2021 11:16:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 581A46838A;\n\tThu, 28 Jan 2021 12:16:37 +0100 (CET)","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 7B7616030A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 12:16:36 +0100 (CET)","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 ECC5B331;\n\tThu, 28 Jan 2021 12:16:35 +0100 (CET)"],"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=\"ZUCuqIZd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1611832596;\n\tbh=R4VDTLdv1g33EfIrhx6i3n0gGAaPQgRGz3uERR0NZio=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZUCuqIZdYxqLnB0NEYYv+x+z1UxOi0ZX8aAFMy88hqGSmgWBuMXV2VV7wUvgwRRuY\n\teWKm+OcPFl1BO7gsNiw0VD94q1fGri4p0VJeiYMEhDEQbph5LrQZHw0nK/3wdPPSVT\n\tShkH5imSXEcE6MBbObrvbP08BPe/b5TzZK+/hvWw=","Date":"Thu, 28 Jan 2021 13:16:16 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YBKdAKmHj2MSSaeo@pendragon.ideasonboard.com>","References":"<20210128102140.160134-1-paul.elder@ideasonboard.com>\n\t<20210128110303.7gz4lbvduufsfcbq@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210128110303.7gz4lbvduufsfcbq@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":14843,"web_url":"https://patchwork.libcamera.org/comment/14843/","msgid":"<20210128125201.oka526t6ffzyigld@uno.localdomain>","date":"2021-01-28T12:52:01","subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi\n\nOn Thu, Jan 28, 2021 at 01:16:16PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo and Paul,\n>\n> On Thu, Jan 28, 2021 at 12:03:03PM +0100, Jacopo Mondi wrote:\n> > On Thu, Jan 28, 2021 at 07:21:40PM +0900, Paul Elder wrote:\n> > > Set the AE precapture triggler tag in the android result metadata\n> > > according to what was passed in the request metadata.\n> > >\n> > > This allows the following CTS test to pass:\n> > > - android.hardware.camera2.cts.StillCaptureTest#testAePrecaptureTriggerCancelJpegCapture\n> > >\n> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > >\n> > > ---\n> > > Changes in v2:\n> > > - move setting the tag to getResultMetadata()\n> > > ---\n> > >  src/android/camera_device.cpp | 10 ++++++----\n> > >  1 file changed, 6 insertions(+), 4 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > > index 82bf0d3a..f5066709 100644\n> > > --- a/src/android/camera_device.cpp\n> > > +++ b/src/android/camera_device.cpp\n> > > @@ -1965,8 +1965,11 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> > >  \t\t\t\t aeFpsTarget.data(), aeFpsTarget.size());\n> > >\n> > >  \tvalue = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE;\n> > > -\tresultMetadata->addEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> > > -\t\t\t\t &value, 1);\n> > > +\tcamera_metadata_ro_entry_t entry;\n> >\n> > You can move this one at the beginning of the function. It is used in\n> > many places, and we'll continue moving it otherwise.\n> >\n> > > +\t/* \\todo Handle IPA appropriately */\n> > > +\tbool ret = descriptor->settings_.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry);\n> > > +\tresultMetadata->updateEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER,\n> > > +\t\t\t\t    ret ? entry.data.u8 : &value, 1);\n> >\n> > idk... we're blindly taking this from from the settings, while I think\n> > it would be worth plumbing it into the pipeline handler already,\n> > even if it will be functionally equivalent to what you have here.\n> >\n> > There's also a draft control already defined for this.\n> >\n> > To close the loop this would require to:\n> > - register the control in the pipeline handler (for IPU3 at initControls() time)\n> > - translate the android metadata into a libcamera control in\n> >   CameraDevice::processControls()\n> > - Return in the request complete handler of the pipeline handler the\n> >   value passed in the request settings (see how ScalerCrop is handled\n> >   in the IPU3 imguOutputBufferReady() slot)\n> > - Translate back the libcamera control from the request to the android\n> >   metadata (look again at how ScalerCrop is handled in\n> >   getResultMetadata())\n> >\n> > This might seems like a useless exercize, but once the IPA needs to\n> > handle this control for real, this will have to be done and I would\n> > not delay it.\n> >\n> > Laurent, do you think this is worth going to the pipeline handler loop\n> > already ?\n>\n> There's a state machine to we need to implement to handle the AE. It\n> involves multiple controls. As we don't support still image capture yet\n> in the libcamera core I think it could be too early to plumb this\n> through, I'm be tempted to just go with this shortcut for the very short\n> term.\n>\n\nI see, makes sense.\n\nFor this patch then\nReviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n> > >  \tvalue = ANDROID_CONTROL_AE_STATE_CONVERGED;\n> > >  \tresultMetadata->addEntry(ANDROID_CONTROL_AE_STATE, &value, 1);\n> > > @@ -2010,8 +2013,7 @@ CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor,\n> > >  \tvalue = ANDROID_FLASH_STATE_UNAVAILABLE;\n> > >  \tresultMetadata->addEntry(ANDROID_FLASH_STATE, &value, 1);\n> > >\n> > > -\tcamera_metadata_ro_entry_t entry;\n> > > -\tint ret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> > > +\tret = descriptor->settings_.getEntry(ANDROID_LENS_APERTURE, &entry);\n> > >  \tif (ret)\n> > >  \t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\n> > >\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 6D420BD808\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 28 Jan 2021 12:51:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E4B636838B;\n\tThu, 28 Jan 2021 13:51:42 +0100 (CET)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B0006030A\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 28 Jan 2021 13:51:41 +0100 (CET)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id D6ED5200006;\n\tThu, 28 Jan 2021 12:51:40 +0000 (UTC)"],"Date":"Thu, 28 Jan 2021 13:52:01 +0100","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20210128125201.oka526t6ffzyigld@uno.localdomain>","References":"<20210128102140.160134-1-paul.elder@ideasonboard.com>\n\t<20210128110303.7gz4lbvduufsfcbq@uno.localdomain>\n\t<YBKdAKmHj2MSSaeo@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<YBKdAKmHj2MSSaeo@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: camera_device: Set AE\n\tprecapture trigger according to request","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]