[v3,5/9] libcamera: software_isp: Use a macro to assign debayering methods

22281 diff mbox series

Assignments of the debayering methods to be used is a repetitive pattern
that can be (arguably) better expressed by using a macro.  This removes
some duplication and also makes easier to introduce more complex
assignment patterns.  This will be useful once color correction matrix
support is added.

Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
---
 src/libcamera/software_isp/debayer_cpu.cpp | 25 ++++++++++------------
 1 file changed, 11 insertions(+), 14 deletions(-)

Quoting Milan Zamazal (2024-12-10 15:34:35)
> Assignments of the debayering methods to be used is a repetitive pattern
> that can be (arguably) better expressed by using a macro.  This removes
> some duplication and also makes easier to introduce more complex
> assignment patterns.  This will be useful once color correction matrix
> support is added.
> 

Being concise here will definitely help readability IMO of the lines. It
does mean what it's setting and how is a bit more hidden, but the
/intent/ is much more clear and I think that's beneficial.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
> ---
>  src/libcamera/software_isp/debayer_cpu.cpp | 25 ++++++++++------------
>  1 file changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 0eabced2..01cfb36b 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -368,6 +368,10 @@ int DebayerCpu::setupStandardBayerOrder(BayerFormat::Order order)
>         return 0;
>  }
>  
> +#define SET_DEBAYER_METHODS(method0, method1)                                                \
> +       debayer0_ = addAlphaByte ? &DebayerCpu::method0<true> : &DebayerCpu::method0<false>; \
> +       debayer1_ = addAlphaByte ? &DebayerCpu::method1<true> : &DebayerCpu::method1<false>;
> +
>  int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputFormat)
>  {
>         BayerFormat bayerFormat =
> @@ -423,16 +427,13 @@ int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputF
>             isStandardBayerOrder(bayerFormat.order)) {
>                 switch (bayerFormat.bitDepth) {
>                 case 8:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer8_BGBG_BGR888<true> : &DebayerCpu::debayer8_BGBG_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer8_GRGR_BGR888<true> : &DebayerCpu::debayer8_GRGR_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer8_BGBG_BGR888, debayer8_GRGR_BGR888)
>                         break;
>                 case 10:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer10_BGBG_BGR888<true> : &DebayerCpu::debayer10_BGBG_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer10_GRGR_BGR888<true> : &DebayerCpu::debayer10_GRGR_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer10_BGBG_BGR888, debayer10_GRGR_BGR888)
>                         break;
>                 case 12:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer12_BGBG_BGR888<true> : &DebayerCpu::debayer12_BGBG_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer12_GRGR_BGR888<true> : &DebayerCpu::debayer12_GRGR_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer12_BGBG_BGR888, debayer12_GRGR_BGR888)
>                         break;
>                 }
>                 setupStandardBayerOrder(bayerFormat.order);
> @@ -443,20 +444,16 @@ int DebayerCpu::setDebayerFunctions(PixelFormat inputFormat, PixelFormat outputF
>             bayerFormat.packing == BayerFormat::Packing::CSI2) {
>                 switch (bayerFormat.order) {
>                 case BayerFormat::BGGR:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer10P_BGBG_BGR888<true> : &DebayerCpu::debayer10P_BGBG_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer10P_GRGR_BGR888<true> : &DebayerCpu::debayer10P_GRGR_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer10P_BGBG_BGR888, debayer10P_GRGR_BGR888)
>                         return 0;
>                 case BayerFormat::GBRG:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer10P_GBGB_BGR888<true> : &DebayerCpu::debayer10P_GBGB_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer10P_RGRG_BGR888<true> : &DebayerCpu::debayer10P_RGRG_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer10P_GBGB_BGR888, debayer10P_RGRG_BGR888)
>                         return 0;
>                 case BayerFormat::GRBG:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer10P_GRGR_BGR888<true> : &DebayerCpu::debayer10P_GRGR_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer10P_BGBG_BGR888<true> : &DebayerCpu::debayer10P_BGBG_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer10P_GRGR_BGR888, debayer10P_BGBG_BGR888)
>                         return 0;
>                 case BayerFormat::RGGB:
> -                       debayer0_ = addAlphaByte ? &DebayerCpu::debayer10P_RGRG_BGR888<true> : &DebayerCpu::debayer10P_RGRG_BGR888<false>;
> -                       debayer1_ = addAlphaByte ? &DebayerCpu::debayer10P_GBGB_BGR888<true> : &DebayerCpu::debayer10P_GBGB_BGR888<false>;
> +                       SET_DEBAYER_METHODS(debayer10P_RGRG_BGR888, debayer10P_GBGB_BGR888)
>                         return 0;
>                 default:
>                         break;
> -- 
> 2.44.2
>