[v2,3/3] libcamera: android: Add face detection control support
diff mbox series

Message ID 20240823143205.2668765-4-chenghaoyang@google.com
State Superseded
Headers show
Series
  • Add Face Detection Controls
Related show

Commit Message

Cheng-Hao Yang Aug. 23, 2024, 2:29 p.m. UTC 
From: Yudhistira Erlandinata <yerlandinata@chromium.org>

Allow Android HAL adapter to pass the face detection metadata control to
the pipeline and also send face detection metadata to the camera client
if the pipeline generates it.

Squashed CL:
https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5065292

Don't use ControlInfoMap::count() and ControlInfoMap::at() because
its internal assumption is wrong: The ControlIdMap and its idmap have a
1:1 mapping between their entries.

BUG=b:308713855
TEST=emerge-geralt libcamera-mtkisp7

Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
---
 src/android/camera_capabilities.cpp | 41 ++++++++++++++++--
 src/android/camera_device.cpp       | 64 +++++++++++++++++++++++++++--
 2 files changed, 98 insertions(+), 7 deletions(-)

Comments

Jacopo Mondi Aug. 28, 2024, 1:10 p.m. UTC | #1 
Hello

On Fri, Aug 23, 2024 at 02:29:10PM GMT, Harvey Yang wrote:
> From: Yudhistira Erlandinata <yerlandinata@chromium.org>
>
> Allow Android HAL adapter to pass the face detection metadata control to
> the pipeline and also send face detection metadata to the camera client
> if the pipeline generates it.
>
> Squashed CL:
> https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5065292

Please drop, unrelated to libcamera

>
> Don't use ControlInfoMap::count() and ControlInfoMap::at() because
> its internal assumption is wrong: The ControlIdMap and its idmap have a
> 1:1 mapping between their entries.

Not sure I got this :)

>
> BUG=b:308713855
> TEST=emerge-geralt libcamera-mtkisp7

ditto, please drop, it's your internal stuff.

>
> Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  src/android/camera_capabilities.cpp | 41 ++++++++++++++++--
>  src/android/camera_device.cpp       | 64 +++++++++++++++++++++++++++--
>  2 files changed, 98 insertions(+), 7 deletions(-)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 71043e127..31d113d51 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -11,6 +11,7 @@
>  #include <array>
>  #include <cmath>
>  #include <map>
> +#include <stdint.h>

ups, thanks

>  #include <type_traits>
>
>  #include <hardware/camera3.h>
> @@ -1176,11 +1177,43 @@ int CameraCapabilities::initializeStaticMetadata()
>  				  maxFrameDuration_);
>
>  	/* Statistics static metadata. */
> -	uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> -	staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> -				  faceDetectMode);
> -
>  	int32_t maxFaceCount = 0;
> +	auto iter = camera_->controls().find(controls::FaceDetectMode.id());
> +	if (iter != camera_->controls().end()) {
> +		const ControlInfo &faceDetectCtrlInfo = iter->second;
> +		std::vector<uint8_t> faceDetectModes;
> +		bool hasFaceDetection = false;

Maybe an empty line to give this code some space to breath

> +		for (const auto &value : faceDetectCtrlInfo.values()) {
> +			auto mode = value.get<uint8_t>();

Don't use auto when the type is trivial

> +			uint8_t androidMode = 0;

I would add an empty line here as well

> +			switch (mode) {
> +			case controls::FaceDetectModeOff:
> +				androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> +				break;
> +			case controls::FaceDetectModeSimple:
> +				androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_SIMPLE;
> +				hasFaceDetection = true;
> +				break;
> +			default:
> +				LOG(HAL, Fatal) << "Received invalid face detect mode: "
> +						<< static_cast<int>(mode);

do you need the case if you make mode an uint8_t ?

> +			}
> +			faceDetectModes.push_back(androidMode);
> +		}
> +		if (hasFaceDetection) {
> +			// todo(yerlandinata): Create new libcamera controls
> +			// to query max possible faces detected.

No C++ comments, please

Also, we don't doxygen the hal, but the rest of the code uses \todo

> +			maxFaceCount = 10;
> +			staticMetadata_->addEntry(
> +				ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> +				faceDetectModes.data(), faceDetectModes.size());
> +		}
> +	} else {
> +		uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> +		staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> +					  faceDetectMode);
> +	}
> +
>  	staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_MAX_FACE_COUNT,
>  				  maxFaceCount);
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 493f66e7b..cd600eade 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -9,12 +9,12 @@
>
>  #include <algorithm>
>  #include <fstream>
> -#include <set>

why ?

>  #include <sys/mman.h>
>  #include <unistd.h>
>  #include <vector>
>
>  #include <libcamera/base/log.h>
> +#include <libcamera/base/span.h>
>  #include <libcamera/base/unique_fd.h>
>  #include <libcamera/base/utils.h>
>
> @@ -22,6 +22,7 @@
>  #include <libcamera/controls.h>
>  #include <libcamera/fence.h>
>  #include <libcamera/formats.h>
> +#include <libcamera/geometry.h>
>  #include <libcamera/property_ids.h>
>
>  #include "system/graphics.h"
> @@ -813,6 +814,14 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  		controls.set(controls::ScalerCrop, cropRegion);
>  	}
>
> +	if (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {
> +		const uint8_t *data = entry.data.u8;
> +		controls.set(controls::FaceDetectMode, data[0]);
> +		if (!controls.get(controls::FaceDetectMode)) {

I don't get this: are you checking if the face detect mode is set to off ?
Does this deserves a Warning ?

> +			LOG(HAL, Warning) << "Pipeline doesn't support controls::FaceDetectMode";
> +		}

no {}  for single line statements, please

> +	}
> +
>  	if (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {
>  		const int32_t data = *entry.data.i32;
>  		int32_t testPatternMode = controls::draft::TestPatternModeOff;
> @@ -1540,8 +1549,9 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  	value32 = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;
>  	resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, value32);
>
> -	value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> -	resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, value);
> +	settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry);
> +	resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
> +				 entry.data.u8, 1);
>
>  	value = ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF;
>  	resultMetadata->addEntry(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE,
> @@ -1580,6 +1590,54 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
>  					 *frameDuration * 1000);
>
> +	const auto &faceDetectRectangles =
> +		metadata.get(controls::FaceDetectFaceRectangles);
> +	if (faceDetectRectangles) {
> +		const Span<const Rectangle> rectangles = *faceDetectRectangles;
> +		std::vector<int32_t> flatRectangles;
> +		for (const Rectangle &rect : rectangles) {
> +			flatRectangles.push_back(rect.x);
> +			flatRectangles.push_back(rect.y);
> +			flatRectangles.push_back(rect.x + rect.width);
> +			flatRectangles.push_back(rect.y + rect.height);

Why x + width and y + height to express width and height ?
The control description should probably also specify what the
rectangles reference system is.

> +		}
> +		resultMetadata->addEntry(
> +			ANDROID_STATISTICS_FACE_RECTANGLES, flatRectangles);
> +	}
> +
> +	const auto &faceDetectFaceScores =
> +		metadata.get(controls::FaceDetectFaceScores);
> +	if (faceDetectRectangles && faceDetectFaceScores) {
> +		const Span<const uint8_t> &scores = *faceDetectFaceScores;
> +		if (scores.size() != faceDetectRectangles->size()) {
> +			LOG(HAL, Error) << "Pipeline returned wrong number of face scores; "
> +					<< "Expected: "
> +					<< faceDetectRectangles->size()
> +					<< ", got:" << scores.size();
> +		}
> +		resultMetadata->addEntry(ANDROID_STATISTICS_FACE_SCORES, scores);
> +	}
> +
> +	const auto &faceDetectFaceLandmarks =
> +		metadata.get(controls::FaceDetectFaceLandmark);
> +	if (faceDetectFaceScores && faceDetectRectangles &&
> +	    faceDetectFaceLandmarks) {
> +		const auto &landmarks = *faceDetectFaceLandmarks;
> +		size_t expectedLandmarks = faceDetectRectangles->size() * 3;

As said, the control definition should express all these requirements.

> +		std::vector<int32_t> androidLandmarks;
> +		for (const auto &landmark : landmarks) {
> +			androidLandmarks.push_back(landmark.x);
> +			androidLandmarks.push_back(landmark.y);
> +		}
> +		resultMetadata->addEntry(
> +			ANDROID_STATISTICS_FACE_LANDMARKS, androidLandmarks);
> +		if (landmarks.size() != expectedLandmarks) {

You can check this before populating the vector

> +			LOG(HAL, Error) << "Pipeline returned wrong number of face landmarks; "
> +					<< "Expected: " << expectedLandmarks
> +					<< ", got: " << landmarks.size();
> +		}
> +	}
> +
>  	const auto &scalerCrop = metadata.get(controls::ScalerCrop);
>  	if (scalerCrop) {
>  		const Rectangle &crop = *scalerCrop;
> --
> 2.46.0.295.g3b9ea8a38a-goog
>
Cheng-Hao Yang Aug. 30, 2024, 9:03 p.m. UTC | #2 
Thanks Jacopo,

On Wed, Aug 28, 2024 at 3:10 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com>
wrote:

> Hello
>
> On Fri, Aug 23, 2024 at 02:29:10PM GMT, Harvey Yang wrote:
> > From: Yudhistira Erlandinata <yerlandinata@chromium.org>
> >
> > Allow Android HAL adapter to pass the face detection metadata control to
> > the pipeline and also send face detection metadata to the camera client
> > if the pipeline generates it.
> >
> > Squashed CL:
> >
> https://chromium-review.googlesource.com/c/chromiumos/third_party/libcamera/+/5065292
>
> Please drop, unrelated to libcamera
>
Dropped.


>
> >
> > Don't use ControlInfoMap::count() and ControlInfoMap::at() because
> > its internal assumption is wrong: The ControlIdMap and its idmap have a
> > 1:1 mapping between their entries.
>
> Not sure I got this :)
>
In the previous CL, we were using count() & at(), while the assumption [1]
was
wrong for mtkisp7's CameraData [2], as it includes other controls::draft
entires,
like [3].

Therefore, using find() is the safest.

[1]:
https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/controls.cpp#n753
[2]:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=881
[3]:
https://source.chromium.org/chromiumos/chromiumos/codesearch/+/main:src/third_party/libcamera/mtkisp7/src/libcamera/pipeline/mtkisp7/mtkisp7.cpp;l=769


> >
> > BUG=b:308713855
> > TEST=emerge-geralt libcamera-mtkisp7
>
> ditto, please drop, it's your internal stuff.
>
Done


>
> >
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  src/android/camera_capabilities.cpp | 41 ++++++++++++++++--
> >  src/android/camera_device.cpp       | 64 +++++++++++++++++++++++++++--
> >  2 files changed, 98 insertions(+), 7 deletions(-)
> >
> > diff --git a/src/android/camera_capabilities.cpp
> b/src/android/camera_capabilities.cpp
> > index 71043e127..31d113d51 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -11,6 +11,7 @@
> >  #include <array>
> >  #include <cmath>
> >  #include <map>
> > +#include <stdint.h>
>
> ups, thanks
>
Above other includes? Done, while the linter disagrees...
https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/jobs/63003432


>
> >  #include <type_traits>
> >
> >  #include <hardware/camera3.h>
> > @@ -1176,11 +1177,43 @@ int
> CameraCapabilities::initializeStaticMetadata()
> >                                 maxFrameDuration_);
> >
> >       /* Statistics static metadata. */
> > -     uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> > -
>  staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> > -                               faceDetectMode);
> > -
> >       int32_t maxFaceCount = 0;
> > +     auto iter =
> camera_->controls().find(controls::FaceDetectMode.id());
> > +     if (iter != camera_->controls().end()) {
> > +             const ControlInfo &faceDetectCtrlInfo = iter->second;
> > +             std::vector<uint8_t> faceDetectModes;
> > +             bool hasFaceDetection = false;
>
> Maybe an empty line to give this code some space to breath
>
Done


>
> > +             for (const auto &value : faceDetectCtrlInfo.values()) {
> > +                     auto mode = value.get<uint8_t>();
>
> Don't use auto when the type is trivial
>
Done


>
> > +                     uint8_t androidMode = 0;
>
> I would add an empty line here as well
>
Done


>
> > +                     switch (mode) {
> > +                     case controls::FaceDetectModeOff:
> > +                             androidMode =
> ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> > +                             break;
> > +                     case controls::FaceDetectModeSimple:
> > +                             androidMode =
> ANDROID_STATISTICS_FACE_DETECT_MODE_SIMPLE;
> > +                             hasFaceDetection = true;
> > +                             break;
> > +                     default:
> > +                             LOG(HAL, Fatal) << "Received invalid face
> detect mode: "
> > +                                             << static_cast<int>(mode);
>
> do you need the case if you make mode an uint8_t ?
>
Removed the cast.


>
> > +                     }
> > +                     faceDetectModes.push_back(androidMode);
> > +             }
> > +             if (hasFaceDetection) {
> > +                     // todo(yerlandinata): Create new libcamera
> controls
> > +                     // to query max possible faces detected.
>
> No C++ comments, please
>
> Also, we don't doxygen the hal, but the rest of the code uses \todo
>
Updated.


>
> > +                     maxFaceCount = 10;
> > +                     staticMetadata_->addEntry(
> > +
>  ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> > +                             faceDetectModes.data(),
> faceDetectModes.size());
> > +             }
> > +     } else {
> > +             uint8_t faceDetectMode =
> ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> > +
>  staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
> > +                                       faceDetectMode);
> > +     }
> > +
> >       staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_MAX_FACE_COUNT,
> >                                 maxFaceCount);
> >
> > diff --git a/src/android/camera_device.cpp
> b/src/android/camera_device.cpp
> > index 493f66e7b..cd600eade 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -9,12 +9,12 @@
> >
> >  #include <algorithm>
> >  #include <fstream>
> > -#include <set>
>
> why ?
>
Added it back.


>
> >  #include <sys/mman.h>
> >  #include <unistd.h>
> >  #include <vector>
> >
> >  #include <libcamera/base/log.h>
> > +#include <libcamera/base/span.h>
> >  #include <libcamera/base/unique_fd.h>
> >  #include <libcamera/base/utils.h>
> >
> > @@ -22,6 +22,7 @@
> >  #include <libcamera/controls.h>
> >  #include <libcamera/fence.h>
> >  #include <libcamera/formats.h>
> > +#include <libcamera/geometry.h>
> >  #include <libcamera/property_ids.h>
> >
> >  #include "system/graphics.h"
> > @@ -813,6 +814,14 @@ int
> CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
> >               controls.set(controls::ScalerCrop, cropRegion);
> >       }
> >
> > +     if (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
> &entry)) {
> > +             const uint8_t *data = entry.data.u8;
> > +             controls.set(controls::FaceDetectMode, data[0]);
> > +             if (!controls.get(controls::FaceDetectMode)) {
>
> I don't get this: are you checking if the face detect mode is set to off ?
> Does this deserves a Warning ?
>
I think Yudhis was checking if the control is set properly, while it should
be removed after debugging.


>
> > +                     LOG(HAL, Warning) << "Pipeline doesn't support
> controls::FaceDetectMode";
> > +             }
>
> no {}  for single line statements, please
>
Removed.


>
> > +     }
> > +
> >       if (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {
> >               const int32_t data = *entry.data.i32;
> >               int32_t testPatternMode =
> controls::draft::TestPatternModeOff;
> > @@ -1540,8 +1549,9 @@ CameraDevice::getResultMetadata(const
> Camera3RequestDescriptor &descriptor) cons
> >       value32 = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;
> >       resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
> value32);
> >
> > -     value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
> > -     resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
> value);
> > +     settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry);
> > +     resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
> > +                              entry.data.u8, 1);
> >
> >       value = ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF;
> >       resultMetadata->addEntry(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE,
> > @@ -1580,6 +1590,54 @@ CameraDevice::getResultMetadata(const
> Camera3RequestDescriptor &descriptor) cons
> >               resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> >                                        *frameDuration * 1000);
> >
> > +     const auto &faceDetectRectangles =
> > +             metadata.get(controls::FaceDetectFaceRectangles);
> > +     if (faceDetectRectangles) {
> > +             const Span<const Rectangle> rectangles =
> *faceDetectRectangles;
> > +             std::vector<int32_t> flatRectangles;
> > +             for (const Rectangle &rect : rectangles) {
> > +                     flatRectangles.push_back(rect.x);
> > +                     flatRectangles.push_back(rect.y);
> > +                     flatRectangles.push_back(rect.x + rect.width);
> > +                     flatRectangles.push_back(rect.y + rect.height);
>
> Why x + width and y + height to express width and height ?
> The control description should probably also specify what the
> rectangles reference system is.
>
Updated in the yaml file: it follows the format of
ANDROID_STATISTICS_FACE_RECTANGLES.


>
> > +             }
> > +             resultMetadata->addEntry(
> > +                     ANDROID_STATISTICS_FACE_RECTANGLES,
> flatRectangles);
> > +     }
> > +
> > +     const auto &faceDetectFaceScores =
> > +             metadata.get(controls::FaceDetectFaceScores);
> > +     if (faceDetectRectangles && faceDetectFaceScores) {
> > +             const Span<const uint8_t> &scores = *faceDetectFaceScores;
> > +             if (scores.size() != faceDetectRectangles->size()) {
> > +                     LOG(HAL, Error) << "Pipeline returned wrong number
> of face scores; "
> > +                                     << "Expected: "
> > +                                     << faceDetectRectangles->size()
> > +                                     << ", got:" << scores.size();
> > +             }
> > +             resultMetadata->addEntry(ANDROID_STATISTICS_FACE_SCORES,
> scores);
> > +     }
> > +
> > +     const auto &faceDetectFaceLandmarks =
> > +             metadata.get(controls::FaceDetectFaceLandmark);
> > +     if (faceDetectFaceScores && faceDetectRectangles &&
> > +         faceDetectFaceLandmarks) {
> > +             const auto &landmarks = *faceDetectFaceLandmarks;
> > +             size_t expectedLandmarks = faceDetectRectangles->size() *
> 3;
>
> As said, the control definition should express all these requirements.
>
Updated.


>
> > +             std::vector<int32_t> androidLandmarks;
> > +             for (const auto &landmark : landmarks) {
> > +                     androidLandmarks.push_back(landmark.x);
> > +                     androidLandmarks.push_back(landmark.y);
> > +             }
> > +             resultMetadata->addEntry(
> > +                     ANDROID_STATISTICS_FACE_LANDMARKS,
> androidLandmarks);
> > +             if (landmarks.size() != expectedLandmarks) {
>
> You can check this before populating the vector
>
Done


>
> > +                     LOG(HAL, Error) << "Pipeline returned wrong number
> of face landmarks; "
> > +                                     << "Expected: " <<
> expectedLandmarks
> > +                                     << ", got: " << landmarks.size();
> > +             }
> > +     }
> > +
> >       const auto &scalerCrop = metadata.get(controls::ScalerCrop);
> >       if (scalerCrop) {
> >               const Rectangle &crop = *scalerCrop;
> > --
> > 2.46.0.295.g3b9ea8a38a-goog
> >
>

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 71043e127..31d113d51 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -11,6 +11,7 @@ 
 #include <array>
 #include <cmath>
 #include <map>
+#include <stdint.h>
 #include <type_traits>
 
 #include <hardware/camera3.h>
@@ -1176,11 +1177,43 @@  int CameraCapabilities::initializeStaticMetadata()
 				  maxFrameDuration_);
 
 	/* Statistics static metadata. */
-	uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
-	staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
-				  faceDetectMode);
-
 	int32_t maxFaceCount = 0;
+	auto iter = camera_->controls().find(controls::FaceDetectMode.id());
+	if (iter != camera_->controls().end()) {
+		const ControlInfo &faceDetectCtrlInfo = iter->second;
+		std::vector<uint8_t> faceDetectModes;
+		bool hasFaceDetection = false;
+		for (const auto &value : faceDetectCtrlInfo.values()) {
+			auto mode = value.get<uint8_t>();
+			uint8_t androidMode = 0;
+			switch (mode) {
+			case controls::FaceDetectModeOff:
+				androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
+				break;
+			case controls::FaceDetectModeSimple:
+				androidMode = ANDROID_STATISTICS_FACE_DETECT_MODE_SIMPLE;
+				hasFaceDetection = true;
+				break;
+			default:
+				LOG(HAL, Fatal) << "Received invalid face detect mode: "
+						<< static_cast<int>(mode);
+			}
+			faceDetectModes.push_back(androidMode);
+		}
+		if (hasFaceDetection) {
+			// todo(yerlandinata): Create new libcamera controls
+			// to query max possible faces detected.
+			maxFaceCount = 10;
+			staticMetadata_->addEntry(
+				ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
+				faceDetectModes.data(), faceDetectModes.size());
+		}
+	} else {
+		uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
+		staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_AVAILABLE_FACE_DETECT_MODES,
+					  faceDetectMode);
+	}
+
 	staticMetadata_->addEntry(ANDROID_STATISTICS_INFO_MAX_FACE_COUNT,
 				  maxFaceCount);
 
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 493f66e7b..cd600eade 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -9,12 +9,12 @@ 
 
 #include <algorithm>
 #include <fstream>
-#include <set>
 #include <sys/mman.h>
 #include <unistd.h>
 #include <vector>
 
 #include <libcamera/base/log.h>
+#include <libcamera/base/span.h>
 #include <libcamera/base/unique_fd.h>
 #include <libcamera/base/utils.h>
 
@@ -22,6 +22,7 @@ 
 #include <libcamera/controls.h>
 #include <libcamera/fence.h>
 #include <libcamera/formats.h>
+#include <libcamera/geometry.h>
 #include <libcamera/property_ids.h>
 
 #include "system/graphics.h"
@@ -813,6 +814,14 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 		controls.set(controls::ScalerCrop, cropRegion);
 	}
 
+	if (settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry)) {
+		const uint8_t *data = entry.data.u8;
+		controls.set(controls::FaceDetectMode, data[0]);
+		if (!controls.get(controls::FaceDetectMode)) {
+			LOG(HAL, Warning) << "Pipeline doesn't support controls::FaceDetectMode";
+		}
+	}
+
 	if (settings.getEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, &entry)) {
 		const int32_t data = *entry.data.i32;
 		int32_t testPatternMode = controls::draft::TestPatternModeOff;
@@ -1540,8 +1549,9 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 	value32 = ANDROID_SENSOR_TEST_PATTERN_MODE_OFF;
 	resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE, value32);
 
-	value = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF;
-	resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, value);
+	settings.getEntry(ANDROID_STATISTICS_FACE_DETECT_MODE, &entry);
+	resultMetadata->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
+				 entry.data.u8, 1);
 
 	value = ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF;
 	resultMetadata->addEntry(ANDROID_STATISTICS_LENS_SHADING_MAP_MODE,
@@ -1580,6 +1590,54 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
 					 *frameDuration * 1000);
 
+	const auto &faceDetectRectangles =
+		metadata.get(controls::FaceDetectFaceRectangles);
+	if (faceDetectRectangles) {
+		const Span<const Rectangle> rectangles = *faceDetectRectangles;
+		std::vector<int32_t> flatRectangles;
+		for (const Rectangle &rect : rectangles) {
+			flatRectangles.push_back(rect.x);
+			flatRectangles.push_back(rect.y);
+			flatRectangles.push_back(rect.x + rect.width);
+			flatRectangles.push_back(rect.y + rect.height);
+		}
+		resultMetadata->addEntry(
+			ANDROID_STATISTICS_FACE_RECTANGLES, flatRectangles);
+	}
+
+	const auto &faceDetectFaceScores =
+		metadata.get(controls::FaceDetectFaceScores);
+	if (faceDetectRectangles && faceDetectFaceScores) {
+		const Span<const uint8_t> &scores = *faceDetectFaceScores;
+		if (scores.size() != faceDetectRectangles->size()) {
+			LOG(HAL, Error) << "Pipeline returned wrong number of face scores; "
+					<< "Expected: "
+					<< faceDetectRectangles->size()
+					<< ", got:" << scores.size();
+		}
+		resultMetadata->addEntry(ANDROID_STATISTICS_FACE_SCORES, scores);
+	}
+
+	const auto &faceDetectFaceLandmarks =
+		metadata.get(controls::FaceDetectFaceLandmark);
+	if (faceDetectFaceScores && faceDetectRectangles &&
+	    faceDetectFaceLandmarks) {
+		const auto &landmarks = *faceDetectFaceLandmarks;
+		size_t expectedLandmarks = faceDetectRectangles->size() * 3;
+		std::vector<int32_t> androidLandmarks;
+		for (const auto &landmark : landmarks) {
+			androidLandmarks.push_back(landmark.x);
+			androidLandmarks.push_back(landmark.y);
+		}
+		resultMetadata->addEntry(
+			ANDROID_STATISTICS_FACE_LANDMARKS, androidLandmarks);
+		if (landmarks.size() != expectedLandmarks) {
+			LOG(HAL, Error) << "Pipeline returned wrong number of face landmarks; "
+					<< "Expected: " << expectedLandmarks
+					<< ", got: " << landmarks.size();
+		}
+	}
+
 	const auto &scalerCrop = metadata.get(controls::ScalerCrop);
 	if (scalerCrop) {
 		const Rectangle &crop = *scalerCrop;