<div dir="ltr"><div dir="ltr">Thanks Jacopo!<br><br>v6 will be uploaded.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 23, 2024 at 6:21 PM Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Harvey<br>
<br>
On Mon, Sep 23, 2024 at 09:30:46AM GMT, Harvey Yang wrote:<br>
> From: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
><br>
> Allow Android HAL adapter to pass the face detection metadata control to<br>
> the pipeline and also send face detection metadata to the camera client<br>
> if the pipeline generates it.<br>
><br>
> Signed-off-by: Yudhistira Erlandinata <<a href="mailto:yerlandinata@chromium.org" target="_blank">yerlandinata@chromium.org</a>><br>
> Co-developed-by: Harvey Yang <<a href="mailto:chenghaoyang@chromium.org" target="_blank">chenghaoyang@chromium.org</a>><br>
> ---<br>
>  src/android/camera_capabilities.cpp | 45 +++++++++++++++--<br>
>  src/android/camera_device.cpp       | 75 ++++++++++++++++++++++++++++-<br>
>  2 files changed, 114 insertions(+), 6 deletions(-)<br>
><br>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp<br>
> index 71043e12..a89a3115 100644<br>
> --- a/src/android/camera_capabilities.cpp<br>
> +++ b/src/android/camera_capabilities.cpp<br>
> @@ -7,6 +7,8 @@<br>
><br>
>  #include "camera_capabilities.h"<br>
><br>
> +#include <stdint.h><br>
> +<br>
>  #include <algorithm><br>
>  #include <array><br>
>  #include <cmath><br>
> @@ -1176,11 +1178,46 @@ int CameraCapabilities::initializeStaticMetadata()<br>
>                                 maxFrameDuration_);<br>
><br>
>       /* Statistics static metadata. */<br>
> -     uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;<br>
> -     staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,<br>
> -                               faceDetectMode);<br>
> -<br>
>       int32_t maxFaceCount = 0;<br>
> +     auto iter = camera_->controls().find(controls::draft::FaceDetectMode.id());<br>
> +     if (iter != camera_->controls().end()) {<br>
> +             const ControlInfo &faceDetectCtrlInfo = iter->second;<br>
> +             std::vector<uint8_t> faceDetectModes;<br>
> +             bool hasFaceDetection = false;<br>
> +<br>
> +             for (const auto &value : faceDetectCtrlInfo.values()) {<br>
> +                     uint8_t mode = value.get<uint8_t>();<br>
> +                     uint8_t androidMode = 0;<br>
> +<br>
> +                     switch (mode) {<br>
> +                     case controls::draft::FaceDetectModeOff:<br>
> +                             androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;<br>
> +                             break;<br>
> +                     case controls::draft::FaceDetectModeSimple:<br>
> +                             androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_SIMPLE;<br>
> +                             hasFaceDetection = true;<br>
> +                             break;<br>
> +                     default:<br>
> +                             LOG(HAL, Fatal) << "Received invalid face detect mode: " << mode;<br>
> +                     }<br>
> +                     faceDetectModes.push_back(androidMode);<br>
> +             }<br>
> +             if (hasFaceDetection) {<br>
> +                     /*<br>
> +                      * \todo Create new libcamera controls to query max<br>
> +                      * possible faces detected.<br>
> +                      */<br>
> +                     maxFaceCount = 10;<br>
> +                     staticMetadata_->addEntry(<br>
> +                             ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,<br>
> +                             faceDetectModes.data(), faceDetectModes.size());<br>
> +             }<br>
> +     } else {<br>
> +             uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;<br>
> +             staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,<br>
> +                                       faceDetectMode);<br>
> +     }<br>
> +<br>
>       staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_MAX_FACE_COUNT,<br>
>                                 maxFaceCount);<br>
><br>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp<br>
> index 493f66e7..50caa14a 100644<br>
> --- a/src/android/camera_device.cpp<br>
> +++ b/src/android/camera_device.cpp<br>
> @@ -15,6 +15,7 @@<br>
>  #include <vector><br>
><br>
>  #include <libcamera/base/log.h><br>
> +#include <libcamera/base/span.h><br>
>  #include <libcamera/base/unique_fd.h><br>
>  #include <libcamera/base/utils.h><br>
><br>
> @@ -22,6 +23,7 @@<br>
>  #include <libcamera/controls.h><br>
>  #include <libcamera/fence.h><br>
>  #include <libcamera/formats.h><br>
> +#include <libcamera/geometry.h><br>
>  #include <libcamera/property_ids.h><br>
><br>
>  #include "system/graphics.h"<br>
> @@ -813,6 +815,11 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)<br>
>               controls.set(controls::ScalerCrop, cropRegion);<br>
>       }<br>
><br>
> +     if (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {<br>
> +             const uint8_t *data = entry.data.u8;<br>
> +             controls.set(controls::draft::FaceDetectMode, data[0]);<br>
> +     }<br>
> +<br>
>       if (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {<br>
>               const int32_t data = *entry.data.i32;<br>
>               int32_t testPatternMode = controls::draft::TestPatternModeOff;<br>
> @@ -1540,8 +1547,9 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons<br>
>       value32 = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;<br>
>       resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, value32);<br>
><br>
> -     value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;<br>
> -     resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, value);<br>
> +     settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry);<br>
> +     resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,<br>
> +                              entry.data.u8, 1);<br>
><br>
>       value = ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF;<br>
>       resultMetadata->addEntry(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE,<br>
> @@ -1580,6 +1588,69 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons<br>
>               resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,<br>
>                                        *frameDuration * 1000);<br>
><br>
> +     const auto &faceDetectRectangles =<br>
> +             metadata.get(controls::draft::FaceDetectFaceRectangles);<br>
> +     if (faceDetectRectangles) {<br>
> +             const Span<const Rectangle> rectangles = *faceDetectRectangles;<br>
> +             std::vector<int32_t> flatRectangles;<br>
> +             for (const Rectangle &rect : rectangles) {<br>
> +                     flatRectangles.push_back(rect.x);<br>
> +                     flatRectangles.push_back(rect.y);<br>
> +                     flatRectangles.push_back(rect.x + rect.width);<br>
> +                     flatRectangles.push_back(rect.y + rect.height);<br>
> +             }<br>
> +             resultMetadata->addEntry(<br>
> +                     ANDROID_STATISTICS_FACE_RECTANGLES, flatRectangles);<br>
> +     }<br>
> +<br>
> +     const auto &faceDetectFaceScores =<br>
> +             metadata.get(controls::draft::FaceDetectFaceScores);<br>
> +     if (faceDetectRectangles && faceDetectFaceScores) {<br>
> +             const Span<const uint8_t> &scores = *faceDetectFaceScores;<br>
> +             if (scores.size() != faceDetectRectangles->size()) {<br>
> +                     LOG(HAL, Error) << "Pipeline returned wrong number of face scores; "<br>
> +                                     << "Expected: "<br>
> +                                     << faceDetectRectangles->size()<br>
> +                                     << ", got:" << scores.size();<br>
<br>
missing space after ", got:"<br></blockquote><div><br></div><div>Right, sorry. Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             }<br>
> +             resultMetadata->addEntry(ANDROID_STATISTICS_FACE_SCORES, scores);<br>
> +     }<br>
> +<br>
> +     const auto &faceDetectFaceLandmarks =<br>
> +             metadata.get(controls::draft::FaceDetectFaceLandmarks);<br>
> +     if (faceDetectRectangles && faceDetectFaceScores &&<br>
> +         faceDetectFaceLandmarks) {<br>
<br>
Do you need to accomulate the checks ? can't you just prefix only it<br>
with<br>
        if (faceDetectRectangles && ...) ?<br></blockquote><div><br></div><div>Ah good point. Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             const auto &landmarks = *faceDetectFaceLandmarks;<br>
> +             size_t expectedLandmarks = faceDetectRectangles->size() * 3;<br>
> +             if (landmarks.size() != expectedLandmarks) {<br>
> +                     LOG(HAL, Error) << "Pipeline returned wrong number of face landmarks; "<br>
> +                                     << "Expected: " << expectedLandmarks<br>
> +                                     << ", got: " << landmarks.size();<br>
> +             }<br>
> +<br>
> +             std::vector<int32_t> androidLandmarks;<br>
> +             for (const auto &landmark : landmarks) {<br>
> +                     androidLandmarks.push_back(landmark.x);<br>
> +                     androidLandmarks.push_back(landmark.y);<br>
> +             }<br>
> +             resultMetadata->addEntry(<br>
> +                     ANDROID_STATISTICS_FACE_LANDMARKS, androidLandmarks);<br>
> +     }<br>
> +<br>
> +     const auto &faceDetectFaceIds =<br>
> +             metadata.get(controls::draft::FaceDetectFaceIds);<br>
> +     if (faceDetectRectangles && faceDetectFaceScores &&<br>
> +         faceDetectFaceLandmarks && faceDetectFaceIds) {<br>
> +             const Span<const int32_t> &ids = *faceDetectFaceIds;<br>
> +             if (ids.size() != faceDetectRectangles->size()) {<br>
> +                     LOG(HAL, Error) << "Pipeline returned wrong number of face ids; "<br>
> +                                     << "Expected: "<br>
> +                                     << faceDetectRectangles->size()<br>
> +                                     << ", got:" << ids.size();<br>
<br>
missing space after ", got:"<br></blockquote><div><br></div><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +             }<br>
> +             resultMetadata->addEntry(ANDROID_STATISTICS_FACE_IDS, ids);<br>
> +     }<br>
> +<br>
<br>
I can fix the above comments when applying if it's fine with you<br>
<br>
Reviewed-by: Jacopo Mondi <<a href="mailto:jacopo.mondi@ideasonboard.com" target="_blank">jacopo.mondi@ideasonboard.com</a>><br>
<br>
Thanks<br>
  j<br>
<br>
>       const auto &scalerCrop = metadata.get(controls::ScalerCrop);<br>
>       if (scalerCrop) {<br>
>               const Rectangle &crop = *scalerCrop;<br>
> --<br>
> 2.46.0.792.g87dc391469-goog<br>
><br>
</blockquote></div><br clear="all"><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div>BR,</div>Harvey Yang</div></div></div>