[{"id":31467,"web_url":"https://patchwork.libcamera.org/comment/31467/","msgid":"","date":"2024-09-30T13:49:00","subject":"Re: [PATCH v8 3/3] libcamera: android: Add face detection control\n\tsupport","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Harvey\n\n While rebasing this patch I noticed something\n\nOn Wed, Sep 25, 2024 at 08:12:26AM GMT, Harvey Yang wrote:\n> From: Harvey Yang \n>\n> Allow Android HAL adapter to pass the face detection metadata control to\n> the pipeline and also send face detection metadata to the camera client\n> if the pipeline generates it.\n>\n> Signed-off-by: Yudhistira Erlandinata \n> Co-developed-by: Harvey Yang \n> Reviewed-by: Jacopo Mondi \n> ---\n> src/android/camera_capabilities.cpp | 45 +++++++++++++++++--\n> src/android/camera_device.cpp | 70 ++++++++++++++++++++++++++++-\n> 2 files changed, 109 insertions(+), 6 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 71043e12..a89a3115 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -7,6 +7,8 @@\n>\n> #include \"camera_capabilities.h\"\n>\n> +#include \n> +\n> #include \n> #include \n> #include \n> @@ -1176,11 +1178,46 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t\t\t\t maxFrameDuration_);\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> -\t\t\t\t faceDetectMode);\n> -\n> \tint32_t maxFaceCount = 0;\n> +\tauto iter = camera_->controls().find(controls::draft::FaceDetectMode.id());\n> +\tif (iter != camera_->controls().end()) {\n> +\t\tconst ControlInfo &faceDetectCtrlInfo = iter->second;\n> +\t\tstd::vector faceDetectModes;\n> +\t\tbool hasFaceDetection = false;\n> +\n> +\t\tfor (const auto &value : faceDetectCtrlInfo.values()) {\n> +\t\t\tuint8_t mode = value.get();\n> +\t\t\tuint8_t androidMode = 0;\n> +\n> +\t\t\tswitch (mode) {\n> +\t\t\tcase controls::draft::FaceDetectModeOff:\n> +\t\t\t\tandroidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> +\t\t\t\tbreak;\n> +\t\t\tcase controls::draft::FaceDetectModeSimple:\n> +\t\t\t\tandroidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_SIMPLE;\n> +\t\t\t\thasFaceDetection = true;\n> +\t\t\t\tbreak;\n> +\t\t\tdefault:\n> +\t\t\t\tLOG(HAL, Fatal) << \"Received invalid face detect mode: \" << mode;\n> +\t\t\t}\n> +\t\t\tfaceDetectModes.push_back(androidMode);\n> +\t\t}\n> +\t\tif (hasFaceDetection) {\n> +\t\t\t/*\n> +\t\t\t * \\todo Create new libcamera controls to query max\n> +\t\t\t * possible faces detected.\n> +\t\t\t */\n> +\t\t\tmaxFaceCount = 10;\n> +\t\t\tstaticMetadata_->addEntry(\n> +\t\t\t\tANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,\n> +\t\t\t\tfaceDetectModes.data(), faceDetectModes.size());\n> +\t\t}\n> +\t} else {\n> +\t\tuint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> +\t\tstaticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,\n> +\t\t\t\t\t faceDetectMode);\n> +\t}\n> +\n> \tstaticMetadata_->addEntry(ANDROID_STATISTICS_INFO_MAX_FACE_COUNT,\n> \t\t\t\t maxFaceCount);\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 493f66e7..e80b9e17 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -15,6 +15,7 @@\n> #include \n>\n> #include \n> +#include \n> #include \n> #include \n>\n> @@ -22,6 +23,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n>\n> #include \"system/graphics.h\"\n> @@ -813,6 +815,11 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> \t\tcontrols.set(controls::ScalerCrop, cropRegion);\n> \t}\n>\n> +\tif (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {\n> +\t\tconst uint8_t *data = entry.data.u8;\n> +\t\tcontrols.set(controls::draft::FaceDetectMode, data[0]);\n> +\t}\n> +\n> \tif (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {\n> \t\tconst int32_t data = *entry.data.i32;\n> \t\tint32_t testPatternMode = controls::draft::TestPatternModeOff;\n> @@ -1540,8 +1547,9 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \tvalue32 = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, value32);\n>\n> -\tvalue = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;\n> -\tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, value);\n> +\tsettings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry);\n> +\tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,\n> +\t\t\t\t entry.data.u8, 1);\n\nShould this be unconditionally populated or should we rather\n\n\tif (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {\n \tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,\n\t\t\t\t entry.data.u8, 1);\n }\n\nlike it is done for other metadata populated directly with the value\nreceived from controls ?\n\nin example:\n\tif (settings.getEntry(ANDROID_LENS_APERTURE, &entry))\n\t\tresultMetadata->addEntry(ANDROID_LENS_APERTURE, entry.data.f, 1);\n\na few lines above\n\n>\n> \tvalue = ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF;\n> \tresultMetadata->addEntry(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE,\n> @@ -1580,6 +1588,64 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> \t\t\t\t\t *frameDuration * 1000);\n>\n> +\tconst auto &faceDetectRectangles =\n> +\t\tmetadata.get(controls::draft::FaceDetectFaceRectangles);\n> +\tif (faceDetectRectangles) {\n> +\t\tstd::vector flatRectangles;\n> +\t\tfor (const Rectangle &rect : *faceDetectRectangles) {\n> +\t\t\tflatRectangles.push_back(rect.x);\n> +\t\t\tflatRectangles.push_back(rect.y);\n> +\t\t\tflatRectangles.push_back(rect.x + rect.width);\n> +\t\t\tflatRectangles.push_back(rect.y + rect.height);\n> +\t\t}\n> +\t\tresultMetadata->addEntry(\n> +\t\t\tANDROID_STATISTICS_FACE_RECTANGLES, flatRectangles);\n> +\t}\n> +\n> +\tconst auto &faceDetectFaceScores =\n> +\t\tmetadata.get(controls::draft::FaceDetectFaceScores);\n> +\tif (faceDetectRectangles && faceDetectFaceScores) {\n> +\t\tif (faceDetectFaceScores->size() != faceDetectRectangles->size()) {\n> +\t\t\tLOG(HAL, Error) << \"Pipeline returned wrong number of face scores; \"\n> +\t\t\t\t\t<< \"Expected: \"\n> +\t\t\t\t\t<< faceDetectRectangles->size()\n> +\t\t\t\t\t<< \", got: \" << faceDetectFaceScores->size();\n> +\t\t}\n> +\t\tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_SCORES,\n> +\t\t\t\t\t *faceDetectRectangles);\n\ns/faceDetectRectangles/faceDetectFaceScores\n\nThis seems a bug: can you see scores in the application you're testing\nwith ?\n\nWith your ack, I'll fix the above in the series I'm about to send.\n\nThanks\n j\n\n\n> +\t}\n> +\n> +\tconst auto &faceDetectFaceLandmarks =\n> +\t\tmetadata.get(controls::draft::FaceDetectFaceLandmarks);\n> +\tif (faceDetectRectangles && faceDetectFaceLandmarks) {\n> +\t\tsize_t expectedLandmarks = faceDetectRectangles->size() * 3;\n> +\t\tif (faceDetectFaceLandmarks->size() != expectedLandmarks) {\n> +\t\t\tLOG(HAL, Error) << \"Pipeline returned wrong number of face landmarks; \"\n> +\t\t\t\t\t<< \"Expected: \" << expectedLandmarks\n> +\t\t\t\t\t<< \", got: \" << faceDetectFaceLandmarks->size();\n> +\t\t}\n> +\n> +\t\tstd::vector androidLandmarks;\n> +\t\tfor (const auto &landmark : *faceDetectFaceLandmarks) {\n> +\t\t\tandroidLandmarks.push_back(landmark.x);\n> +\t\t\tandroidLandmarks.push_back(landmark.y);\n> +\t\t}\n> +\t\tresultMetadata->addEntry(\n> +\t\t\tANDROID_STATISTICS_FACE_LANDMARKS, androidLandmarks);\n> +\t}\n> +\n> +\tconst auto &faceDetectFaceIds =\n> +\t\tmetadata.get(controls::draft::FaceDetectFaceIds);\n> +\tif (faceDetectRectangles && faceDetectFaceIds) {\n> +\t\tif (faceDetectFaceIds->size() != faceDetectRectangles->size()) {\n> +\t\t\tLOG(HAL, Error) << \"Pipeline returned wrong number of face ids; \"\n> +\t\t\t\t\t<< \"Expected: \"\n> +\t\t\t\t\t<< faceDetectRectangles->size()\n> +\t\t\t\t\t<< \", got: \" << faceDetectFaceIds->size();\n> +\t\t}\n> +\t\tresultMetadata->addEntry(ANDROID_STATISTICS_FACE_IDS, *faceDetectFaceIds);\n> +\t}\n> +\n> \tconst auto &scalerCrop = metadata.get(controls::ScalerCrop);\n> \tif (scalerCrop) {\n> \t\tconst Rectangle &crop = *scalerCrop;\n> --\n> 2.46.0.792.g87dc391469-goog\n>","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 32257C0F1B\n\tfor ;\n\tMon, 30 Sep 2024 13:49:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0DC3D63512;\n\tMon, 30 Sep 2024 15:49:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 06A6063502\n\tfor ;\n\tMon, 30 Sep 2024 15:49:05 +0200 (CEST)","from ideasonboard.com (unknown [95.131.45.214])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73CD1280;\n\tMon, 30 Sep 2024 15:47:33 +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=\"ht9RQSw7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727704053;\n\tbh=bsiqzBaWB0N6VSxvY9vWp1AS8E+IogEzGwiBtcxCDJs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ht9RQSw7PAQUNLhirlmKDXOaGjs1Mp0UWl0PK6r/qLM6LB1AXbCBqDCS63MYwUh/u\n\tFuXIUK1iY9xESy/0h3bDaRTdo1iaom4noTEqgILszCPn1y0G5KPBxdwCbeOqauxyps\n\t50Xcaa4n5u/CxWakO8FTwkMTuLlME+NTo/lTwQNg=","Date":"Mon, 30 Sep 2024 15:49:00 +0200","From":"Jacopo Mondi ","To":"Harvey Yang ","Cc":"libcamera-devel@lists.libcamera.org, \n\tYudhistira Erlandinata ,\n\tJacopo Mondi ","Subject":"Re: [PATCH v8 3/3] libcamera: android: Add face detection control\n\tsupport","Message-ID":"","References":"<20240925081551.3265942-1-chenghaoyang@google.com>\n\t<20240925081551.3265942-4-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240925081551.3265942-4-chenghaoyang@google.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]