[libcamera-devel] [PATCH v3 1/5] ipa: ipu3: af: Move constants to implementation
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Mar 29 12:17:58 CEST 2022
Hi Kieran
Quoting Kieran Bingham (2022-03-25 09:25:51)
> A selection of constants are imported from ChromiumOS.
>
> Move these out of the header, and simplify their documentation. Further
> more, add a direct reference to the location they were obtained from.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
> src/ipa/ipu3/algorithms/af.cpp | 73 ++++++++++------------------------
> src/ipa/ipu3/algorithms/af.h | 11 -----
> 2 files changed, 22 insertions(+), 62 deletions(-)
>
> diff --git a/src/ipa/ipu3/algorithms/af.cpp b/src/ipa/ipu3/algorithms/af.cpp
> index 0170a3728892..634d0f2e3176 100644
> --- a/src/ipa/ipu3/algorithms/af.cpp
> +++ b/src/ipa/ipu3/algorithms/af.cpp
> @@ -29,59 +29,30 @@
> * \file af.h
> */
>
> -/**
> - * \var kAfMinGridWidth
> - * \brief the minimum width of AF grid.
> - * The minimum grid horizontal dimensions.
> - */
> -
> -/**
> - * \var kAfMinGridHeight
> - * \brief the minimum height of AF grid.
> - * The minimum grid vertical dimensions.
> - */
> -
> -/**
> - * \var kAfMaxGridWidth
> - * \brief the maximum width of AF grid.
> - * The maximum grid horizontal dimensions.
> - */
> -
> -/**
> - * \var kAfMaxGridHeight
> - * \brief The maximum height of AF grid.
> - * The maximum grid vertical dimensions.
> - */
> -
> -/**
> - * \var kAfMinGridBlockWidth
> - * \brief The minimum block size of the width.
> - * The minimum value of Log2 of the width of the grid cell.
> - */
> -
> -/**
> - * \var kAfMinGridBlockHeight
> - * \brief The minimum block size of the height.
> - * The minimum value of Log2 of the height of the grid cell.
> - */
> -
> -/**
> - * \def kAfMaxGridBlockWidth
> - * \brief The maximum block size of the width.
> - * The maximum value of Log2 of the width of the grid cell.
> - */
> -
> -/**
> - * \var kAfMaxGridBlockHeight
> - * \brief The maximum block size of the height.
> - * The maximum value of Log2 of the height of the grid cell.
> +/*
> + * Static variables from ChromiumOS Intel Camera HAL and ia_imaging library:
> + * - https://chromium.googlesource.com/chromiumos/platform/arc-camera/+/master/hal/intel/psl/ipu3/statsConverter/ipu3-stats.h
> + * - https://chromium.googlesource.com/chromiumos/platform/camera/+/refs/heads/main/hal/intel/ipu3/include/ia_imaging/af_public.h
> */
>
> -/**
> - * \var kAfDefaultHeightPerSlice
> - * \brief The default number of blocks in vertical axis per slice.
> - * The number of blocks in vertical axis per slice.
> - */
> +/** The minimum horizontal grid dimension. */
> +static constexpr uint8_t kAfMinGridWidth = 16;
> +/** The minimum vertical grid dimension. */
> +static constexpr uint8_t kAfMinGridHeight = 16;
> +/** The maximum horizontal grid dimension. */
> +static constexpr uint8_t kAfMaxGridWidth = 32;
> +/** The maximum vertical grid dimension. */
> +static constexpr uint8_t kAfMaxGridHeight = 24;
> +/** The minimum value of Log2 of the width of the grid cell. */
> +static constexpr uint16_t kAfMinGridBlockWidth = 4;
> +/** The minimum value of Log2 of the height of the grid cell. */
> +static constexpr uint16_t kAfMinGridBlockHeight = 3;
> +/** The maximum value of Log2 of the width of the grid cell. */
> +static constexpr uint16_t kAfMaxGridBlockWidth = 6;
> +/** The maximum value of Log2 of the height of the grid cell. */
> +static constexpr uint16_t kAfMaxGridBlockHeight = 6;
It turns out some of these are unused and that breaks Clang-12
compilation by default:
clang++-12 -Isrc/ipa/ipu3/ipa_ipu3.so.p -Isrc/ipa/ipu3 -I../../../src/libcamera/src/ipa/ipu3 -Iinclude -I../../../src/libcamera/include -Isrc/ipa -I../../../src/libcamera/src/ipa -Iinclude/libcamera/ipa -Iinclude/libcamera -fcolor-diagnostics -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -g -Wextra-semi -Wthread-safety -Wshadow -include config.h -Wno-c99-designator -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/ipa/ipu3/ipa_ipu3.so.p/algorithms_af.cpp.o -MF src/ipa/ipu3/ipa_ipu3.so.p/algorithms_af.cpp.o.d -o src/ipa/ipu3/ipa_ipu3.so.p/algorithms_af.cpp.o -c ../../../src/libcamera/src/ipa/ipu3/algorithms/af.cpp
../../../src/libcamera/src/ipa/ipu3/algorithms/af.cpp:43:26: error: unused variable 'kAfMaxGridWidth' [-Werror,-Wunused-const-variable]
static constexpr uint8_t kAfMaxGridWidth = 32;
^
../../../src/libcamera/src/ipa/ipu3/algorithms/af.cpp:45:26: error: unused variable 'kAfMaxGridHeight' [-Werror,-Wunused-const-variable]
static constexpr uint8_t kAfMaxGridHeight = 24;
^
../../../src/libcamera/src/ipa/ipu3/algorithms/af.cpp:51:27: error: unused variable 'kAfMaxGridBlockWidth' [-Werror,-Wunused-const-variable]
static constexpr uint16_t kAfMaxGridBlockWidth = 6;
^
../../../src/libcamera/src/ipa/ipu3/algorithms/af.cpp:53:27: error: unused variable 'kAfMaxGridBlockHeight' [-Werror,-Wunused-const-variable]
static constexpr uint16_t kAfMaxGridBlockHeight = 6;
^
4 errors generated.
Kate/JM -
Do you think we should keep these either commented out or in some form
of documentation?
Or should we add an assert that makes use of them so that they can be
kept?
Or should we just drop the unused constants altogether?
--
Kieran
> +/** The number of blocks in vertical axis per slice. */
> +static constexpr uint16_t kAfDefaultHeightPerSlice = 2;
>
> namespace libcamera {
>
> diff --git a/src/ipa/ipu3/algorithms/af.h b/src/ipa/ipu3/algorithms/af.h
> index 13c7e0e877fd..906f2843dd49 100644
> --- a/src/ipa/ipu3/algorithms/af.h
> +++ b/src/ipa/ipu3/algorithms/af.h
> @@ -15,17 +15,6 @@
>
> #include "algorithm.h"
>
> -/* Static variables from repo of chromium */
> -static constexpr uint8_t kAfMinGridWidth = 16;
> -static constexpr uint8_t kAfMinGridHeight = 16;
> -static constexpr uint8_t kAfMaxGridWidth = 32;
> -static constexpr uint8_t kAfMaxGridHeight = 24;
> -static constexpr uint16_t kAfMinGridBlockWidth = 4;
> -static constexpr uint16_t kAfMinGridBlockHeight = 3;
> -static constexpr uint16_t kAfMaxGridBlockWidth = 6;
> -static constexpr uint16_t kAfMaxGridBlockHeight = 6;
> -static constexpr uint16_t kAfDefaultHeightPerSlice = 2;
> -
> namespace libcamera {
>
> namespace ipa::ipu3::algorithms {
> --
> 2.32.0
>
More information about the libcamera-devel
mailing list