[{"id":20078,"web_url":"https://patchwork.libcamera.org/comment/20078/","msgid":"<YVz9lY3wD3Lz4uW3@pendragon.ideasonboard.com>","date":"2021-10-06T01:36:21","subject":"Re: [libcamera-devel] [PATCH v2 07/17] android: capabilties: Assume\n\tcontrols::FrameDurationLimits is supported","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 Tue, Sep 07, 2021 at 09:40:57PM +0200, Jacopo Mondi wrote:\n> As we now collect the per-stream frame durations at\n> initializeStreamConfigurations() times, the Camera is now guaranteed to\n> support the controls::FrameDurationLimits control.\n> \n> Remove the check for its presence when populating the\n> ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES static metadata.\n> \n> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>\n> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/android/camera_capabilities.cpp | 85 ++++++++++++++---------------\n>  1 file changed, 41 insertions(+), 44 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 9bfc7ebc6334..ee2883ab2821 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -862,55 +862,52 @@ int CameraCapabilities::initializeStaticMetadata()\n>  \tstaticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n>  \t\t\t\t  aeAvailableModes);\n>  \n> -\tint64_t minFrameDurationNsec = -1;\n> -\tint64_t maxFrameDurationNsec = -1;\n> +\t/* Initialize the AE frame duration limits. */\n>  \tconst auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);\n> -\tif (frameDurationsInfo != controlsInfo.end()) {\n> -\t\tminFrameDurationNsec = frameDurationsInfo->second.min().get<int64_t>() * 1000;\n> -\t\tmaxFrameDurationNsec = frameDurationsInfo->second.max().get<int64_t>() * 1000;\n> +\tint64_t minFrameDurationNsec = frameDurationsInfo->second.min().get<int64_t>() * 1000;\n> +\tint64_t maxFrameDurationNsec = frameDurationsInfo->second.max().get<int64_t>() * 1000;\n>  \n> -\t\t/*\n> -\t\t * Adjust the minimum frame duration to comply with Android\n> -\t\t * requirements. The camera service mandates all preview/record\n> -\t\t * streams to have a minimum frame duration < 33,366 milliseconds\n> -\t\t * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n> -\t\t * implementation).\n> -\t\t *\n> -\t\t * If we're close enough (+ 500 useconds) to that value, round\n> -\t\t * the minimum frame duration of the camera to an accepted\n> -\t\t * value.\n> -\t\t */\n> -\t\tstatic constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n> -\t\tif (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n> -\t\t    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n> -\t\t\tminFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n> +\t/*\n> +\t * Adjust the minimum frame duration to comply with Android\n> +\t * requirements. The camera service mandates all preview/record\n> +\t * streams to have a minimum frame duration < 33,366 milliseconds\n> +\t * (see MAX_PREVIEW_RECORD_DURATION_NS in the camera service\n> +\t * implementation).\n> +\t *\n> +\t * If we're close enough (+ 500 useconds) to that value, round\n> +\t * the minimum frame duration of the camera to an accepted\n> +\t * value.\n> +\t */\n> +\tstatic constexpr int64_t MAX_PREVIEW_RECORD_DURATION_NS = 1e9 / 29.97;\n> +\tif (minFrameDurationNsec > MAX_PREVIEW_RECORD_DURATION_NS &&\n> +\t    minFrameDurationNsec < MAX_PREVIEW_RECORD_DURATION_NS + 500000)\n> +\t\tminFrameDurationNsec = MAX_PREVIEW_RECORD_DURATION_NS - 1000;\n>  \n> -\t\t/*\n> -\t\t * The AE routine frame rate limits are computed using the frame\n> -\t\t * duration limits, as libcamera clips the AE routine to the\n> -\t\t * frame durations.\n> -\t\t */\n> -\t\tint32_t maxFps = std::round(1e9 / minFrameDurationNsec);\n> -\t\tint32_t minFps = std::round(1e9 / maxFrameDurationNsec);\n> -\t\tminFps = std::max(1, minFps);\n> +\t/*\n> +\t * The AE routine frame rate limits are computed using the frame\n> +\t * duration limits, as libcamera clips the AE routine to the\n> +\t * frame durations.\n> +\t */\n> +\tint32_t maxFps = std::round(1e9 / minFrameDurationNsec);\n> +\tint32_t minFps = std::round(1e9 / maxFrameDurationNsec);\n> +\tminFps = std::max(1, minFps);\n>  \n> -\t\t/*\n> -\t\t * Force rounding errors so that we have the proper frame\n> -\t\t * durations for when we reuse these variables later\n> -\t\t */\n> -\t\tminFrameDurationNsec = 1e9 / maxFps;\n> -\t\tmaxFrameDurationNsec = 1e9 / minFps;\n> +\t/*\n> +\t * Force rounding errors so that we have the proper frame\n> +\t * durations for when we reuse these variables later\n> +\t */\n> +\tminFrameDurationNsec = 1e9 / maxFps;\n> +\tmaxFrameDurationNsec = 1e9 / minFps;\n>  \n> -\t\t/*\n> -\t\t * Register to the camera service {min, max} and {max, max}\n> -\t\t * intervals as requested by the metadata documentation.\n> -\t\t */\n> -\t\tint32_t availableAeFpsTarget[] = {\n> -\t\t\tminFps, maxFps, maxFps, maxFps\n> -\t\t};\n> -\t\tstaticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,\n> -\t\t\t\t\t  availableAeFpsTarget);\n> -\t}\n> +\t/*\n> +\t * Register to the camera service {min, max} and {max, max}\n> +\t * intervals as requested by the metadata documentation.\n> +\t */\n> +\tint32_t availableAeFpsTarget[] = {\n> +\t\tminFps, maxFps, maxFps, maxFps\n> +\t};\n> +\tstaticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,\n> +\t\t\t\t  availableAeFpsTarget);\n>  \n>  \tstd::vector<int32_t> aeCompensationRange = {\n>  \t\t0, 0,","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 7D41AC3243\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  6 Oct 2021 01:36:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3AC68691BC;\n\tWed,  6 Oct 2021 03:36:31 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B0EAE602DC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  6 Oct 2021 03:36:29 +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 53F29581;\n\tWed,  6 Oct 2021 03:36:29 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"fQrV78jE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1633484189;\n\tbh=fZ0BUynw3hLjd54cBtCxdM56Mp92+8FU1zBcEap9ELQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fQrV78jEW9BqXrKqT3/Aa9f0rXEzwC2lQI05MgxP6mhXxXJaAAp6M3a+DPv9rvBw7\n\tEgcNn1BihxzL+DUB1XSzomFsEAcG8z8ZdcGugYXCyp8SSBE1o8SG4HFVvKzbziN+Nb\n\tCRhVNlRhlEZk2JEt9DmzJ++AYNDODQiBm5MiJK+M=","Date":"Wed, 6 Oct 2021 04:36:21 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Jacopo Mondi <jacopo@jmondi.org>","Message-ID":"<YVz9lY3wD3Lz4uW3@pendragon.ideasonboard.com>","References":"<20210907194107.803730-1-jacopo@jmondi.org>\n\t<20210907194107.803730-8-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210907194107.803730-8-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v2 07/17] android: capabilties: Assume\n\tcontrols::FrameDurationLimits is supported","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>"}}]