[{"id":21223,"web_url":"https://patchwork.libcamera.org/comment/21223/","msgid":"<163783810899.3059017.7580202108352413296@Monstersaurus>","date":"2021-11-25T11:01:48","subject":"Re: [libcamera-devel] [PATCH 1/7] android: camera_capabilities: Add\n\tmessages for lack of FULL support","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2021-11-23 10:40:36)\n> Print messages when some feature is missing that causes hardware level\n> FULL to not be supported.\n> \n> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> ---\n>  src/android/camera_capabilities.cpp | 7 ++++++-\n>  1 file changed, 6 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index f357902e..8c138df4 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -359,6 +359,9 @@ void CameraCapabilities::computeHwLevel(\n>  {\n>         camera_metadata_ro_entry_t entry;\n>         bool found;\n> +\n> +       const char *noFull = \"Hardware level FULL unavailable: \";\n> +\n>         camera_metadata_enum_android_info_supported_hardware_level\n>                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n>  \n> @@ -372,8 +375,10 @@ void CameraCapabilities::computeHwLevel(\n>                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n>  \n>         found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> -       if (!found || *entry.data.i32 != 0)\n> +       if (!found || *entry.data.i32 != 0) {\n> +               LOG(HAL, Info) << noFull << \"missing or invalid max sync latency\";\n>                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +       }\n\nAre the other settings that force _LIMITED affected? Or are the all the\n'real' times when _LIMITED is expected.\n\n\nShould it only print if hwLevel is not already set to\nANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED by one of the statements\nabove?\n\nOtherwise if it's a missing feature on a target that expects to be\nlimited it would be noisy.\n\nIf we always want to print it, I'd move it below the if (!found) to it's\nown statement otherwise.\n\n--\nKieran\n\n\n>  \n>         hwLevel_ = hwLevel;\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 8C970BDB13\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 25 Nov 2021 11:01:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CE4076036F;\n\tThu, 25 Nov 2021 12:01:52 +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 D84BA60231\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 25 Nov 2021 12:01:51 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A94A2C5;\n\tThu, 25 Nov 2021 12:01:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KJDRHwuL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637838111;\n\tbh=q7bKB86JVQG+Kmlucvcqi+457eFZMd4q5kJqq7S4RrA=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=KJDRHwuLQGDluMtbJY9PWLRTWwEG2Lqitp/6QsIKyTnVRIULeMeuFfowSzYdftJdk\n\tLFlGiIInvcCtXeytoHPscvh9GtZLpx3uvXDCs7WeA1fcWyXrBuP5oa2x9pm052gAp2\n\t4qXiWx+1ocslBQemRpwbsmMIWZ/H1/BvwVHZEWaA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211123104042.3100902-2-paul.elder@ideasonboard.com>","References":"<20211123104042.3100902-1-paul.elder@ideasonboard.com>\n\t<20211123104042.3100902-2-paul.elder@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","To":"Paul Elder <paul.elder@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 25 Nov 2021 11:01:48 +0000","Message-ID":"<163783810899.3059017.7580202108352413296@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/7] android: camera_capabilities: Add\n\tmessages for lack of FULL support","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>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":21831,"web_url":"https://patchwork.libcamera.org/comment/21831/","msgid":"<20211220224159.GB2742@pyrite.rasen.tech>","date":"2021-12-20T22:41:59","subject":"Re: [libcamera-devel] [PATCH 1/7] android: camera_capabilities: Add\n\tmessages for lack of FULL support","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Thu, Nov 25, 2021 at 11:01:48AM +0000, Kieran Bingham wrote:\n> Quoting Paul Elder (2021-11-23 10:40:36)\n> > Print messages when some feature is missing that causes hardware level\n> > FULL to not be supported.\n> > \n> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>\n> > ---\n> >  src/android/camera_capabilities.cpp | 7 ++++++-\n> >  1 file changed, 6 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index f357902e..8c138df4 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -359,6 +359,9 @@ void CameraCapabilities::computeHwLevel(\n> >  {\n> >         camera_metadata_ro_entry_t entry;\n> >         bool found;\n> > +\n> > +       const char *noFull = \"Hardware level FULL unavailable: \";\n> > +\n> >         camera_metadata_enum_android_info_supported_hardware_level\n> >                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> >  \n> > @@ -372,8 +375,10 @@ void CameraCapabilities::computeHwLevel(\n> >                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> >  \n> >         found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> > -       if (!found || *entry.data.i32 != 0)\n> > +       if (!found || *entry.data.i32 != 0) {\n> > +               LOG(HAL, Info) << noFull << \"missing or invalid max sync latency\";\n> >                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +       }\n> \n> Are the other settings that force _LIMITED affected? Or are the all the\n> 'real' times when _LIMITED is expected.\n\nThe other settings that force LIMITED, at the point in time of this\npatch, are checking capabilities. The capability validators already\nprint their own messages. In future patches, when we add more\nnon-capability but yes-hardware-level-full checks, they will add prints\nsimilar to this one.\n\n> \n> \n> Should it only print if hwLevel is not already set to\n> ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED by one of the statements\n> above?\n> \n> Otherwise if it's a missing feature on a target that expects to be\n> limited it would be noisy.\n\nYeah I realize it would be noisy on a device that expects to be limited.\nHowever, we don't have a way to declare that a device is limited. So for\nexample while trying to get a device to support full, one would have to\ndo many re-runs to get the full list of requirements.\n\n> \n> If we always want to print it, I'd move it below the if (!found) to it's\n> own statement otherwise.\n\nSo I don't think this, nor printing only if (!hwLevel = LIMITED) are\nsolutions. I think we need a way for a device to declare its desired\nhardware level. Perhaps the platform config? We can't put it in\nlibcamera.\n\nOtherwise we just leave the noisy messages. After all, this is exactly\nthe mechanism where we decide whether our camera is FULL or not.\n\n\nPaul\n\n> \n> \n> >  \n> >         hwLevel_ = hwLevel;\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 29904BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 20 Dec 2021 22:42:09 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1817B608E7;\n\tMon, 20 Dec 2021 23:42:08 +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 19AED60223\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 20 Dec 2021 23:42:06 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2604:2d80:ad90:fb00:96fd:8874:873:6c16])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1BE42B9C;\n\tMon, 20 Dec 2021 23:42:04 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Oi3pgO5M\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1640040125;\n\tbh=AUBSW9EUiZtd3NEYFyKW8VJA2aGlPD5939bwC0hgjDE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Oi3pgO5MbqrJLlQV8I2RcXzTVQSr1WZJMAc3GvZ+FYpW7XK3TbrAGY2nZwHpe8XmB\n\t82ADlPEIrgL3bY31hu6JJy8eWaMZkD8twJ4/9tI1GczlWcLWQWhl4CYrKP0FdgzwEO\n\tgj99bYOc3JI7tNlCtb8Goe7WdxqjEO4bHvpXg9PI=","Date":"Mon, 20 Dec 2021 16:41:59 -0600","From":"paul.elder@ideasonboard.com","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20211220224159.GB2742@pyrite.rasen.tech>","References":"<20211123104042.3100902-1-paul.elder@ideasonboard.com>\n\t<20211123104042.3100902-2-paul.elder@ideasonboard.com>\n\t<163783810899.3059017.7580202108352413296@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<163783810899.3059017.7580202108352413296@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 1/7] android: camera_capabilities: Add\n\tmessages for lack of FULL support","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>"}}]