[v7,2/3] libcamera: Add face detection controls

21344 diff mbox series

From: Yudhistira Erlandinata <yerlandinata@chromium.org>

Add FaceDetectMode, FaceDetectFaceRectangles, FaceDetectFaceScores,
and FaceDetectFaceLandmark. Also add ControlTypePoint for supporting
FaceDetectFaceLandmark.

Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
Co-developed-by: becker hsieh <beckerh@chromium.org>
Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/controls.h           |  6 ++
 include/libcamera/meson.build          |  3 +-
 src/libcamera/control_ids_android.yaml | 98 ++++++++++++++++++++++++++
 src/libcamera/control_ranges.yaml      |  4 +-
 src/libcamera/controls.cpp             |  6 ++
 5 files changed, 115 insertions(+), 2 deletions(-)
 create mode 100644 src/libcamera/control_ids_android.yaml

Hi Harvey

On Tue, Sep 24, 2024 at 11:02:45AM GMT, Harvey Yang wrote:
> From: Yudhistira Erlandinata <yerlandinata@chromium.org>
>
> Add FaceDetectMode, FaceDetectFaceRectangles, FaceDetectFaceScores,
> and FaceDetectFaceLandmark. Also add ControlTypePoint for supporting
> FaceDetectFaceLandmark.
>
> Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> Co-developed-by: becker hsieh <beckerh@chromium.org>
> Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>  include/libcamera/controls.h           |  6 ++
>  include/libcamera/meson.build          |  3 +-
>  src/libcamera/control_ids_android.yaml | 98 ++++++++++++++++++++++++++
>  src/libcamera/control_ranges.yaml      |  4 +-
>  src/libcamera/controls.cpp             |  6 ++
>  5 files changed, 115 insertions(+), 2 deletions(-)
>  create mode 100644 src/libcamera/control_ids_android.yaml
>
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 7c2bb287..bf1b8609 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -34,6 +34,7 @@ enum ControlType {
>  	ControlTypeString,
>  	ControlTypeRectangle,
>  	ControlTypeSize,
> +	ControlTypePoint,
>  };
>
>  namespace details {
> @@ -87,6 +88,11 @@ struct control_type<Size> {
>  	static constexpr ControlType value = ControlTypeSize;
>  };
>
> +template<>
> +struct control_type<Point> {
> +	static constexpr ControlType value = ControlTypePoint;
> +};
> +
>  template<typename T, std::size_t N>
>  struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> {
>  };
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index a969a95d..4fff14d2 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -34,6 +34,7 @@ libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir
>
>  controls_map = {
>      'controls': {
> +        'android': 'control_ids_android.yaml',
>          'draft': 'control_ids_draft.yaml',
>          'core': 'control_ids_core.yaml',
>          'rpi/vc4': 'control_ids_rpi.yaml',
> @@ -55,7 +56,7 @@ foreach mode, entry : controls_map
>      files_list = []
>      input_files = []
>      foreach vendor, header : entry
> -        if vendor != 'core' and vendor != 'draft'
> +        if vendor != 'core' and vendor != 'draft' and vendor != 'android'
>              if vendor not in pipelines
>                  continue
>              endif
> diff --git a/src/libcamera/control_ids_android.yaml b/src/libcamera/control_ids_android.yaml
> new file mode 100644
> index 00000000..8b0d624f
> --- /dev/null
> +++ b/src/libcamera/control_ids_android.yaml

As Kieran said, this should be moved back to draft.
I can do that if you want, no need to resend.

> @@ -0,0 +1,98 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +#
> +# Copyright (C) 2024, Google Inc.
> +#
> +%YAML 1.1
> +---
> +# Unless otherwise stated, all controls are bi-directional, i.e. they can be
> +# set through Request::controls() and returned out through Request::metadata().
> +vendor: android
> +controls:
> +  - FaceDetectMode:
> +      type: uint8_t
> +      description: |
> +        Reporting mode of face detection.
> +
> +        Currently identical to ANDROID_STATISTICS_FACE_DETECT_MODE.
> +
> +        \sa FaceDetectFaceRectangles
> +        \sa FaceDetectFaceScores
> +        \sa FaceDetectFaceLandmarks
> +        \sa FaceDetectFaceIds
> +
> +      enum:
> +        - name: FaceDetectModeOff
> +          value: 0
> +          description: |
> +            Pipeline should not report face detection result.

Blank line since you have it in the other block

> +        - name: FaceDetectModeSimple
> +          value: 1
> +          description: |
> +            Pipeline should at least report FaceDetectFaceRectangles and
> +            FaceDetectFaceScores for each detected faces.

s/faces/face

> +            FaceDetectFaceLandmarks and FaceDetectFaceIds is optional.

s/is/are

> +
> +        - name: FaceDetectModeFull
> +          value: 2
> +          description: |
> +            Pipeline should report all face controls, including
> +            FaceDetectFaceRectangles, FaceDetectFaceScores,
> +            FaceDetectFaceLandmarks, and FaceDeteceFaceIds.
> +
> +  - FaceDetectFaceRectangles:
> +      type: Rectangle
> +      description: |
> +        Boundary rectangles of the detected faces.
> +        The number of values should be the number of faces reported in
> +        FaceDetectFaceRectangles.
> +
> +        The FaceDetectFaceRectangles control can only be returned in metadata.
> +
> +        Currently identical to ANDROID_STATISTICS_FACE_RECTANGLES.
> +
> +      size: [n]
> +
> +  - FaceDetectFaceScores:
> +      type: uint8_t
> +      description: |
> +        Confidence score of each of the detected faces by face detector.
> +        The range of score is [0, 100].
> +        The FaceDetectFaceScores control can only be returned in metadata.
> +        The number of values should be the number of faces reported in
> +        FaceDetectFaceRectangles.
> +
> +        Currently identical to ANDROID_STATISTICS_FACE_SCORES.
> +
> +      size: [n]
> +
> +  - FaceDetectFaceLandmarks:
> +      type: Point
> +      description: |
> +        Array of human face landmark coordinates in format:
> +        [..., left_eye_i, right_eye_i, mouth_i, left_eye_i+1, ...],
> +        with i = index of face.
> +        The number of values should be 3 * the number of faces reported in
> +        FaceDetectFaceRectangles.
> +
> +        The FaceDetectFaceLandmarks control can only be returned in metadata.
> +
> +        Currently identical to ANDROID_STATISTICS_FACE_LANDMARKS.
> +
> +      size: [n]
> +
> +  - FaceDetectFaceIds:
> +      type: int32_t
> +      description: |
> +        Each detected face is given a unique ID that is valid for as long as
> +        the face is visible to the camera device. A face that leaves the field
> +        of view and later returns may be assigned a new ID.
> +        The number of values should be the number of faces reported in
> +        FaceDetectFaceRectangles.
> +
> +        The FaceDetectFaceIds control can only be returned in metadata.
> +
> +        Currently identical to ANDROID_STATISTICS_FACE_IDS.
> +
> +      size: [n]
> +
> +...
> diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml
> index d42447d0..9d406f10 100644
> --- a/src/libcamera/control_ranges.yaml
> +++ b/src/libcamera/control_ranges.yaml
> @@ -13,6 +13,8 @@ ranges:
>    draft: 10000
>    # Raspberry Pi vendor controls
>    rpi: 20000
> -  # Next range starts at 30000
> +  # Android metadata related controls
> +  android: 30000
> +  # Next range starts at 40000

And I will also drop this change when moving controls back to draft

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j
>
>  ...
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index dba74404..c452e607 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -60,6 +60,7 @@ static constexpr size_t ControlValueSize[] = {
>  	[ControlTypeString]		= sizeof(char),
>  	[ControlTypeRectangle]		= sizeof(Rectangle),
>  	[ControlTypeSize]		= sizeof(Size),
> +	[ControlTypePoint]		= sizeof(Point),
>  };
>
>  } /* namespace */
> @@ -254,6 +255,11 @@ std::string ControlValue::toString() const
>  			str += value->toString();
>  			break;
>  		}
> +		case ControlTypePoint: {
> +			const Point *value = reinterpret_cast<const Point *>(data);
> +			str += value->toString();
> +			break;
> +		}
>  		case ControlTypeNone:
>  		case ControlTypeString:
>  			break;
> --
> 2.46.0.792.g87dc391469-goog
>

Hi Jacopo,

On Tue, Sep 24, 2024 at 8:27 PM Jacopo Mondi
<jacopo.mondi@ideasonboard.com> wrote:
>
> Hi Harvey
>
> On Tue, Sep 24, 2024 at 11:02:45AM GMT, Harvey Yang wrote:
> > From: Yudhistira Erlandinata <yerlandinata@chromium.org>
> >
> > Add FaceDetectMode, FaceDetectFaceRectangles, FaceDetectFaceScores,
> > and FaceDetectFaceLandmark. Also add ControlTypePoint for supporting
> > FaceDetectFaceLandmark.
> >
> > Signed-off-by: Yudhistira Erlandinata <yerlandinata@chromium.org>
> > Co-developed-by: becker hsieh <beckerh@chromium.org>
> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >  include/libcamera/controls.h           |  6 ++
> >  include/libcamera/meson.build          |  3 +-
> >  src/libcamera/control_ids_android.yaml | 98 ++++++++++++++++++++++++++
> >  src/libcamera/control_ranges.yaml      |  4 +-
> >  src/libcamera/controls.cpp             |  6 ++
> >  5 files changed, 115 insertions(+), 2 deletions(-)
> >  create mode 100644 src/libcamera/control_ids_android.yaml
> >
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 7c2bb287..bf1b8609 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -34,6 +34,7 @@ enum ControlType {
> >       ControlTypeString,
> >       ControlTypeRectangle,
> >       ControlTypeSize,
> > +     ControlTypePoint,
> >  };
> >
> >  namespace details {
> > @@ -87,6 +88,11 @@ struct control_type<Size> {
> >       static constexpr ControlType value = ControlTypeSize;
> >  };
> >
> > +template<>
> > +struct control_type<Point> {
> > +     static constexpr ControlType value = ControlTypePoint;
> > +};
> > +
> >  template<typename T, std::size_t N>
> >  struct control_type<Span<T, N>> : public control_type<std::remove_cv_t<T>> {
> >  };
> > diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> > index a969a95d..4fff14d2 100644
> > --- a/include/libcamera/meson.build
> > +++ b/include/libcamera/meson.build
> > @@ -34,6 +34,7 @@ libcamera_headers_install_dir = get_option('includedir') / libcamera_include_dir
> >
> >  controls_map = {
> >      'controls': {
> > +        'android': 'control_ids_android.yaml',
> >          'draft': 'control_ids_draft.yaml',
> >          'core': 'control_ids_core.yaml',
> >          'rpi/vc4': 'control_ids_rpi.yaml',
> > @@ -55,7 +56,7 @@ foreach mode, entry : controls_map
> >      files_list = []
> >      input_files = []
> >      foreach vendor, header : entry
> > -        if vendor != 'core' and vendor != 'draft'
> > +        if vendor != 'core' and vendor != 'draft' and vendor != 'android'
> >              if vendor not in pipelines
> >                  continue
> >              endif
> > diff --git a/src/libcamera/control_ids_android.yaml b/src/libcamera/control_ids_android.yaml
> > new file mode 100644
> > index 00000000..8b0d624f
> > --- /dev/null
> > +++ b/src/libcamera/control_ids_android.yaml
>
> As Kieran said, this should be moved back to draft.
> I can do that if you want, no need to resend.

No worries. I'll upload another version.

>
> > @@ -0,0 +1,98 @@
> > +# SPDX-License-Identifier: LGPL-2.1-or-later
> > +#
> > +# Copyright (C) 2024, Google Inc.
> > +#
> > +%YAML 1.1
> > +---
> > +# Unless otherwise stated, all controls are bi-directional, i.e. they can be
> > +# set through Request::controls() and returned out through Request::metadata().
> > +vendor: android
> > +controls:
> > +  - FaceDetectMode:
> > +      type: uint8_t
> > +      description: |
> > +        Reporting mode of face detection.
> > +
> > +        Currently identical to ANDROID_STATISTICS_FACE_DETECT_MODE.
> > +
> > +        \sa FaceDetectFaceRectangles
> > +        \sa FaceDetectFaceScores
> > +        \sa FaceDetectFaceLandmarks
> > +        \sa FaceDetectFaceIds
> > +
> > +      enum:
> > +        - name: FaceDetectModeOff
> > +          value: 0
> > +          description: |
> > +            Pipeline should not report face detection result.
>
> Blank line since you have it in the other block

Done

>
> > +        - name: FaceDetectModeSimple
> > +          value: 1
> > +          description: |
> > +            Pipeline should at least report FaceDetectFaceRectangles and
> > +            FaceDetectFaceScores for each detected faces.
>
> s/faces/face

Right, updated.

>
> > +            FaceDetectFaceLandmarks and FaceDetectFaceIds is optional.
>
> s/is/are

Thanks!

>
> > +
> > +        - name: FaceDetectModeFull
> > +          value: 2
> > +          description: |
> > +            Pipeline should report all face controls, including
> > +            FaceDetectFaceRectangles, FaceDetectFaceScores,
> > +            FaceDetectFaceLandmarks, and FaceDeteceFaceIds.
> > +
> > +  - FaceDetectFaceRectangles:
> > +      type: Rectangle
> > +      description: |
> > +        Boundary rectangles of the detected faces.
> > +        The number of values should be the number of faces reported in
> > +        FaceDetectFaceRectangles.
> > +
> > +        The FaceDetectFaceRectangles control can only be returned in metadata.
> > +
> > +        Currently identical to ANDROID_STATISTICS_FACE_RECTANGLES.
> > +
> > +      size: [n]
> > +
> > +  - FaceDetectFaceScores:
> > +      type: uint8_t
> > +      description: |
> > +        Confidence score of each of the detected faces by face detector.
> > +        The range of score is [0, 100].
> > +        The FaceDetectFaceScores control can only be returned in metadata.
> > +        The number of values should be the number of faces reported in
> > +        FaceDetectFaceRectangles.
> > +
> > +        Currently identical to ANDROID_STATISTICS_FACE_SCORES.
> > +
> > +      size: [n]
> > +
> > +  - FaceDetectFaceLandmarks:
> > +      type: Point
> > +      description: |
> > +        Array of human face landmark coordinates in format:
> > +        [..., left_eye_i, right_eye_i, mouth_i, left_eye_i+1, ...],
> > +        with i = index of face.
> > +        The number of values should be 3 * the number of faces reported in
> > +        FaceDetectFaceRectangles.
> > +
> > +        The FaceDetectFaceLandmarks control can only be returned in metadata.
> > +
> > +        Currently identical to ANDROID_STATISTICS_FACE_LANDMARKS.
> > +
> > +      size: [n]
> > +
> > +  - FaceDetectFaceIds:
> > +      type: int32_t
> > +      description: |
> > +        Each detected face is given a unique ID that is valid for as long as
> > +        the face is visible to the camera device. A face that leaves the field
> > +        of view and later returns may be assigned a new ID.
> > +        The number of values should be the number of faces reported in
> > +        FaceDetectFaceRectangles.
> > +
> > +        The FaceDetectFaceIds control can only be returned in metadata.
> > +
> > +        Currently identical to ANDROID_STATISTICS_FACE_IDS.
> > +
> > +      size: [n]
> > +
> > +...
> > diff --git a/src/libcamera/control_ranges.yaml b/src/libcamera/control_ranges.yaml
> > index d42447d0..9d406f10 100644
> > --- a/src/libcamera/control_ranges.yaml
> > +++ b/src/libcamera/control_ranges.yaml
> > @@ -13,6 +13,8 @@ ranges:
> >    draft: 10000
> >    # Raspberry Pi vendor controls
> >    rpi: 20000
> > -  # Next range starts at 30000
> > +  # Android metadata related controls
> > +  android: 30000
> > +  # Next range starts at 40000
>
> And I will also drop this change when moving controls back to draft

Sure :)


>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
> >
> >  ...
> > diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> > index dba74404..c452e607 100644
> > --- a/src/libcamera/controls.cpp
> > +++ b/src/libcamera/controls.cpp
> > @@ -60,6 +60,7 @@ static constexpr size_t ControlValueSize[] = {
> >       [ControlTypeString]             = sizeof(char),
> >       [ControlTypeRectangle]          = sizeof(Rectangle),
> >       [ControlTypeSize]               = sizeof(Size),
> > +     [ControlTypePoint]              = sizeof(Point),
> >  };
> >
> >  } /* namespace */
> > @@ -254,6 +255,11 @@ std::string ControlValue::toString() const
> >                       str += value->toString();
> >                       break;
> >               }
> > +             case ControlTypePoint: {
> > +                     const Point *value = reinterpret_cast<const Point *>(data);
> > +                     str += value->toString();
> > +                     break;
> > +             }
> >               case ControlTypeNone:
> >               case ControlTypeString:
> >                       break;
> > --
> > 2.46.0.792.g87dc391469-goog
> >



--
BR,
Harvey Yang