[{"id":15543,"web_url":"https://patchwork.libcamera.org/comment/15543/","msgid":"","date":"2021-03-08T23:47:45","subject":"Re: [libcamera-devel] [PATCH v3 3/5] android: camera_device:\n\tCompute frame durations","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 Mon, Feb 22, 2021 at 11:52:20AM +0100, Jacopo Mondi wrote:\n> Use the FrameDuration control reported by pipeline handlers to register\n> the Auto-Exposure routine FPS range, the minimum stream frame durations\n> and the sensor maximum frame duration.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/android/camera_device.cpp | 84 ++++++++++++++++++++++++++---------\n> 1 file changed, 64 insertions(+), 20 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 1e2a5b5f91da..1658e4a4dd9f 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -9,6 +9,7 @@\n> #include \"camera_ops.h\"\n> #include \"post_processor.h\"\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -708,10 +709,10 @@ std::tuple CameraDevice::calculateStaticMetadataSize()\n> {\n> \t/*\n> \t * \\todo Keep this in sync with the actual number of entries.\n> -\t * Currently: 53 entries, 854 bytes of static metadata\n> +\t * Currently: 54 entries, 874 bytes of static metadata\n> \t */\n> -\tuint32_t numEntries = 53;\n> -\tuint32_t byteSize = 854;\n> +\tuint32_t numEntries = 54;\n> +\tuint32_t byteSize = 874;\n> \n> \t/*\n> \t * Calculate space occupation in bytes for dynamically built metadata\n> @@ -801,12 +802,48 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\t\t\t aeAvailableModes.data(),\n> \t\t\t\t aeAvailableModes.size());\n> \n> -\tstd::vector availableAeFpsTarget = {\n> -\t\t15, 30,\n> -\t};\n> -\tstaticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,\n> -\t\t\t\t availableAeFpsTarget.data(),\n> -\t\t\t\t availableAeFpsTarget.size());\n> +\tint64_t minFrameDurationNsec = -1;\n> +\tint64_t maxFrameDurationNsec = -1;\n> +\tconst auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurations);\n> +\tif (frameDurationsInfo != controlsInfo.end()) {\n> +\t\tminFrameDurationNsec = frameDurationsInfo->second.min().get() * 1000;\n> +\t\tmaxFrameDurationNsec = frameDurationsInfo->second.max().get() * 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> +\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 = static_cast(std::round(1e9 / minFrameDurationNsec));\n> +\t\tint32_t minFps = static_cast(std::round(1e9 / maxFrameDurationNsec));\n\nAre the explicit casts needed ? I don't get any compilation failure in\nmy tests, but maybe the runtime behaviour isn't correct ?\n\nReviewed-by: Laurent Pinchart \n\n> +\t\tminFps = std::max(1, 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, 4);\n> +\t}\n> \n> \tstd::vector aeCompensationRange = {\n> \t\t0, 0,\n> @@ -996,6 +1033,10 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_TIMESTAMP_SOURCE,\n> \t\t\t\t ×tampSource, 1);\n> \n> +\tif (maxFrameDurationNsec > 0)\n> +\t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_MAX_FRAME_DURATION,\n> +\t\t\t\t\t &maxFrameDurationNsec, 1);\n> +\n> \t/* Statistics static metadata. */\n> \tuint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> \tstaticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,\n> @@ -1130,18 +1171,20 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\t\t\t availableStallDurations.data(),\n> \t\t\t\t availableStallDurations.size());\n> \n> -\t/* \\todo Collect the minimum frame duration from the camera. */\n> -\tstd::vector minFrameDurations;\n> -\tminFrameDurations.reserve(streamConfigurations_.size() * 4);\n> -\tfor (const auto &entry : streamConfigurations_) {\n> -\t\tminFrameDurations.push_back(entry.androidFormat);\n> -\t\tminFrameDurations.push_back(entry.resolution.width);\n> -\t\tminFrameDurations.push_back(entry.resolution.height);\n> -\t\tminFrameDurations.push_back(33333333);\n> +\t/* Use the minimum frame duration for all the YUV/RGB formats. */\n> +\tif (minFrameDurationNsec > 0) {\n> +\t\tstd::vector minFrameDurations;\n> +\t\tminFrameDurations.reserve(streamConfigurations_.size() * 4);\n> +\t\tfor (const auto &entry : streamConfigurations_) {\n> +\t\t\tminFrameDurations.push_back(entry.androidFormat);\n> +\t\t\tminFrameDurations.push_back(entry.resolution.width);\n> +\t\t\tminFrameDurations.push_back(entry.resolution.height);\n> +\t\t\tminFrameDurations.push_back(minFrameDurationNsec);\n> +\t\t}\n> +\t\tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n> +\t\t\t\t\t minFrameDurations.data(),\n> +\t\t\t\t\t minFrameDurations.size());\n> \t}\n> -\tstaticMetadata_->addEntry(ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS,\n> -\t\t\t\t minFrameDurations.data(),\n> -\t\t\t\t minFrameDurations.size());\n> \n> \tuint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;\n> \tstaticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, &croppingType, 1);\n> @@ -1241,6 +1284,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\tANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n> \t\tANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> \t\tANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,\n> +\t\tANDROID_SENSOR_INFO_MAX_FRAME_DURATION,\n> \t\tANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n> \t\tANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n> \t\tANDROID_SENSOR_INFO_SENSITIVITY_RANGE,","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 D7004BD80C\n\tfor ;\n\tMon, 8 Mar 2021 23:48:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9566668A9F;\n\tTue, 9 Mar 2021 00:48:20 +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 345BD6051F\n\tfor ;\n\tTue, 9 Mar 2021 00:48:18 +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 581AF99;\n\tTue, 9 Mar 2021 00:48:17 +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=\"NOWvyLxq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1615247297;\n\tbh=RJ1pCAqCBSuQcVQzysAyLeJOjqVaB4F3K/ZR06/Gfh8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NOWvyLxqSt3vLf/VrSaeUBzpOX/6Io2Cnwxp2zj7cSH+gt+nXpMrPfqw9/NDuzWeo\n\t3Jsjn80XFcx0LH3fVjV748Xc1rQJoEAqWMYBRC37s+XIiccRLX+zR3XtSmWjTlGUGK\n\tXgeT4+o7u/NV7W/xIHvBnXz/PQ35s1jhWnRDdqOI=","Date":"Tue, 9 Mar 2021 01:47:45 +0200","From":"Laurent Pinchart ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210222105222.393677-1-jacopo@jmondi.org>\n\t<20210222105222.393677-4-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210222105222.393677-4-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v3 3/5] android: camera_device:\n\tCompute frame durations","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\" "}}]