[{"id":15976,"web_url":"https://patchwork.libcamera.org/comment/15976/","msgid":"","date":"2021-03-28T22:26:51","subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Always add\n\tmandatory metadata entries","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Phi-Bang,\n\nThank you for the patch.\n\nOn Sun, Mar 28, 2021 at 10:17:15PM +0200, Phi-Bang Nguyen wrote:\n> The following static metadata entries are mandatory. Without them,\n> the Android camera service cannot work properly and application will\n> fail to start (error: \"This device doesn't support camera2 API\"):\n> \n> - ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES\n> - ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS\n> \n> Fixes: edd4b1dab26b (\"android: camera_device: Compute frame durations\")\n> \n> Signed-off-by: Phi-Bang Nguyen \n> ---\n> src/android/camera_device.cpp | 32 ++++++++++++++++++--------------\n> 1 file changed, 18 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index a8108e3a..4b5d8f97 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -769,6 +769,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \n> \tint64_t minFrameDurationNsec = -1;\n> \tint64_t maxFrameDurationNsec = -1;\n> +\tstd::vector availableAeFpsTarget = {\n> +\t\t15, 30, 30, 30,\n> +\t};\n\nI agree that these tags need to be provided, as they're mandatory.\nHowever, I don't think providing a default value is the right way to go.\nWe should instead add support for the FrameDurations controls in the\nsimple pipeline handler. The HAL should also likely fail to initialize\n(or open the camera device) if the pipeline handler doesn't support the\nrequired controls, with a big warning message to make it clear what\nneeds to be fixed.\n\n> \tconst auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurations);\n> \tif (frameDurationsInfo != controlsInfo.end()) {\n> \t\tminFrameDurationNsec = frameDurationsInfo->second.min().get() * 1000;\n> @@ -803,12 +806,12 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\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\tavailableAeFpsTarget = {\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> +\tstaticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,\n> +\t\t\t\t availableAeFpsTarget.data(), availableAeFpsTarget.size());\n> \n> \tstd::vector aeCompensationRange = {\n> \t\t0, 0,\n> @@ -1140,19 +1143,20 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\t\t\t availableStallDurations.size());\n> \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> +\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\tif (minFrameDurationNsec > 0)\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\telse\n> +\t\t\tminFrameDurations.push_back(33333333);\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);","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 1C947C32ED\n\tfor ;\n\tSun, 28 Mar 2021 22:27:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81F3A68784;\n\tMon, 29 Mar 2021 00:27:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 16DCD602D7\n\tfor ;\n\tMon, 29 Mar 2021 00:27:36 +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 658658C4;\n\tMon, 29 Mar 2021 00:27:35 +0200 (CEST)"],"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=\"n9jxHz5w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616970455;\n\tbh=tEZ+rHG2Es4f2JlMpqvTeTc+dp9JkqVoFonFyHlBYGg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=n9jxHz5w1qGTzGbQYQhIpc9RvHwmrNNzGis6XHn52iRyybaY02i5TDHAkVLLqYuUu\n\tEYVraZdunNtkwNKcYNr2S6ighEsx50UYSKNNeydH24mNpcjHnEcSZHPVjBhlaUoGV0\n\tLi0bChwWLTG8Qmy2dynJkwL1wbl+rVbpjNS1U3Ms=","Date":"Mon, 29 Mar 2021 01:26:51 +0300","From":"Laurent Pinchart ","To":"Phi-Bang Nguyen ","Message-ID":"","References":"<20210328201715.51816-1-pnguyen@baylibre.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210328201715.51816-1-pnguyen@baylibre.com>","Subject":"Re: [libcamera-devel] [PATCH] android: camera_device: Always add\n\tmandatory metadata entries","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\" "}}]