[v2,14/22] libcamera: software_isp: Move isStandardBayerOrder to base class
diff mbox series

Message ID 20251127022256.178929-15-bryan.odonoghue@linaro.org
State Superseded
Headers show
Series
  • GPUISP precursor series
Related show

Commit Message

Bryan O'Donoghue Nov. 27, 2025, 2:22 a.m. UTC 
isStandardBayerOrder is useful to both CPU and GPU debayer logic and
reusable as-is for both.

Move to shared location in base class.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 src/libcamera/software_isp/debayer.cpp     | 10 ++++++++++
 src/libcamera/software_isp/debayer.h       |  2 ++
 src/libcamera/software_isp/debayer_cpu.cpp |  6 ------
 3 files changed, 12 insertions(+), 6 deletions(-)

Comments

Barnabás Pőcze Dec. 1, 2025, 12:20 p.m. UTC | #1 
Hi

2025. 11. 27. 3:22 keltezéssel, Bryan O'Donoghue írta:
> isStandardBayerOrder is useful to both CPU and GPU debayer logic and
> reusable as-is for both.
> 
> Move to shared location in base class.
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> ---
>   src/libcamera/software_isp/debayer.cpp     | 10 ++++++++++
>   src/libcamera/software_isp/debayer.h       |  2 ++
>   src/libcamera/software_isp/debayer_cpu.cpp |  6 ------
>   3 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
> index 158128f30..1d135b278 100644
> --- a/src/libcamera/software_isp/debayer.cpp
> +++ b/src/libcamera/software_isp/debayer.cpp
> @@ -379,4 +379,14 @@ void Debayer::dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *inpu
>   	}
>   }
>   
> +/**
> + * \fn void Debayer::isStandardBayerOrder(BayerFormat::Order order)
> + * \brief Common method to validate standard 2x2 Bayer pattern of 2 Green, 1 Blue, 1 Red pixels.
> + */
> +bool Debayer::isStandardBayerOrder(BayerFormat::Order order)
> +{
> +	return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
> +	       order == BayerFormat::GRBG || order == BayerFormat::RGGB;
> +}
> +
>   } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
> index d2893d81b..451c1c9ac 100644
> --- a/src/libcamera/software_isp/debayer.h
> +++ b/src/libcamera/software_isp/debayer.h
> @@ -20,6 +20,7 @@
>   #include <libcamera/geometry.h>
>   #include <libcamera/stream.h>
>   
> +#include "libcamera/internal/bayer_format.h"
>   #include "libcamera/internal/dma_buf_allocator.h"
>   #include "libcamera/internal/global_configuration.h"
>   #include "libcamera/internal/software_isp/benchmark.h"
> @@ -88,6 +89,7 @@ private:
>   protected:
>   	void setParams(DebayerParams &params);
>   	void dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *input, FrameBuffer *output);
> +	bool isStandardBayerOrder(BayerFormat::Order order);

This does not need to be a member function, so at least make it `static`.


Regards,
Barnabás Pőcze


>   };
>   
>   } /* namespace libcamera */
> diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
> index 8f1b4e53d..00738c56b 100644
> --- a/src/libcamera/software_isp/debayer_cpu.cpp
> +++ b/src/libcamera/software_isp/debayer_cpu.cpp
> @@ -288,12 +288,6 @@ void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[])
>   	}
>   }
>   
> -static bool isStandardBayerOrder(BayerFormat::Order order)
> -{
> -	return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
> -	       order == BayerFormat::GRBG || order == BayerFormat::RGGB;
> -}
> -
>   /*
>    * Setup the Debayer object according to the passed in parameters.
>    * Return 0 on success, a negative errno value on failure
Bryan O'Donoghue Dec. 1, 2025, 11 p.m. UTC | #2 
On 01/12/2025 12:20, Barnabás Pőcze wrote:
> Hi
> 
> 2025. 11. 27. 3:22 keltezéssel, Bryan O'Donoghue írta:
>> isStandardBayerOrder is useful to both CPU and GPU debayer logic and
>> reusable as-is for both.
>>
>> Move to shared location in base class.
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>> ---
>>   src/libcamera/software_isp/debayer.cpp     | 10 ++++++++++
>>   src/libcamera/software_isp/debayer.h       |  2 ++
>>   src/libcamera/software_isp/debayer_cpu.cpp |  6 ------
>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/ 
>> software_isp/debayer.cpp
>> index 158128f30..1d135b278 100644
>> --- a/src/libcamera/software_isp/debayer.cpp
>> +++ b/src/libcamera/software_isp/debayer.cpp
>> @@ -379,4 +379,14 @@ void Debayer::dmaSyncBegin(std::vector<DmaSyncer> 
>> &dmaSyncers, FrameBuffer *inpu
>>       }
>>   }
>> +/**
>> + * \fn void Debayer::isStandardBayerOrder(BayerFormat::Order order)
>> + * \brief Common method to validate standard 2x2 Bayer pattern of 2 
>> Green, 1 Blue, 1 Red pixels.
>> + */
>> +bool Debayer::isStandardBayerOrder(BayerFormat::Order order)
>> +{
>> +    return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
>> +           order == BayerFormat::GRBG || order == BayerFormat::RGGB;
>> +}
>> +
>>   } /* namespace libcamera */
>> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/ 
>> software_isp/debayer.h
>> index d2893d81b..451c1c9ac 100644
>> --- a/src/libcamera/software_isp/debayer.h
>> +++ b/src/libcamera/software_isp/debayer.h
>> @@ -20,6 +20,7 @@
>>   #include <libcamera/geometry.h>
>>   #include <libcamera/stream.h>
>> +#include "libcamera/internal/bayer_format.h"
>>   #include "libcamera/internal/dma_buf_allocator.h"
>>   #include "libcamera/internal/global_configuration.h"
>>   #include "libcamera/internal/software_isp/benchmark.h"
>> @@ -88,6 +89,7 @@ private:
>>   protected:
>>       void setParams(DebayerParams &params);
>>       void dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, 
>> FrameBuffer *input, FrameBuffer *output);
>> +    bool isStandardBayerOrder(BayerFormat::Order order);
> 
> This does not need to be a member function, so at least make it `static`.

lol

[PATCH v4 02/23] libcamera: software_isp: Make isStandardBayerOrder static

np so

---
bod
Barnabás Pőcze Dec. 2, 2025, 8:04 a.m. UTC | #3 
2025. 12. 02. 0:00 keltezéssel, Bryan O'Donoghue írta:
> On 01/12/2025 12:20, Barnabás Pőcze wrote:
>> Hi
>>
>> 2025. 11. 27. 3:22 keltezéssel, Bryan O'Donoghue írta:
>>> isStandardBayerOrder is useful to both CPU and GPU debayer logic and
>>> reusable as-is for both.
>>>
>>> Move to shared location in base class.
>>>
>>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>> Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
>>> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
>>> ---
>>>   src/libcamera/software_isp/debayer.cpp     | 10 ++++++++++
>>>   src/libcamera/software_isp/debayer.h       |  2 ++
>>>   src/libcamera/software_isp/debayer_cpu.cpp |  6 ------
>>>   3 files changed, 12 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/ software_isp/debayer.cpp
>>> index 158128f30..1d135b278 100644
>>> --- a/src/libcamera/software_isp/debayer.cpp
>>> +++ b/src/libcamera/software_isp/debayer.cpp
>>> @@ -379,4 +379,14 @@ void Debayer::dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *inpu
>>>       }
>>>   }
>>> +/**
>>> + * \fn void Debayer::isStandardBayerOrder(BayerFormat::Order order)
>>> + * \brief Common method to validate standard 2x2 Bayer pattern of 2 Green, 1 Blue, 1 Red pixels.
>>> + */
>>> +bool Debayer::isStandardBayerOrder(BayerFormat::Order order)
>>> +{
>>> +    return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
>>> +           order == BayerFormat::GRBG || order == BayerFormat::RGGB;
>>> +}
>>> +
>>>   } /* namespace libcamera */
>>> diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/ software_isp/debayer.h
>>> index d2893d81b..451c1c9ac 100644
>>> --- a/src/libcamera/software_isp/debayer.h
>>> +++ b/src/libcamera/software_isp/debayer.h
>>> @@ -20,6 +20,7 @@
>>>   #include <libcamera/geometry.h>
>>>   #include <libcamera/stream.h>
>>> +#include "libcamera/internal/bayer_format.h"
>>>   #include "libcamera/internal/dma_buf_allocator.h"
>>>   #include "libcamera/internal/global_configuration.h"
>>>   #include "libcamera/internal/software_isp/benchmark.h"
>>> @@ -88,6 +89,7 @@ private:
>>>   protected:
>>>       void setParams(DebayerParams &params);
>>>       void dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *input, FrameBuffer *output);
>>> +    bool isStandardBayerOrder(BayerFormat::Order order);
>>
>> This does not need to be a member function, so at least make it `static`.
> 
> lol
> 
> [PATCH v4 02/23] libcamera: software_isp: Make isStandardBayerOrder static

Oops, I didn't notice that. In any case I think it probably make sense to squash it into this change.


> 
> np so
> 
> ---
> bod

Patch
diff mbox series 

diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
index 158128f30..1d135b278 100644
--- a/src/libcamera/software_isp/debayer.cpp
+++ b/src/libcamera/software_isp/debayer.cpp
@@ -379,4 +379,14 @@  void Debayer::dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *inpu
 	}
 }
 
+/**
+ * \fn void Debayer::isStandardBayerOrder(BayerFormat::Order order)
+ * \brief Common method to validate standard 2x2 Bayer pattern of 2 Green, 1 Blue, 1 Red pixels.
+ */
+bool Debayer::isStandardBayerOrder(BayerFormat::Order order)
+{
+	return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
+	       order == BayerFormat::GRBG || order == BayerFormat::RGGB;
+}
+
 } /* namespace libcamera */
diff --git a/src/libcamera/software_isp/debayer.h b/src/libcamera/software_isp/debayer.h
index d2893d81b..451c1c9ac 100644
--- a/src/libcamera/software_isp/debayer.h
+++ b/src/libcamera/software_isp/debayer.h
@@ -20,6 +20,7 @@ 
 #include <libcamera/geometry.h>
 #include <libcamera/stream.h>
 
+#include "libcamera/internal/bayer_format.h"
 #include "libcamera/internal/dma_buf_allocator.h"
 #include "libcamera/internal/global_configuration.h"
 #include "libcamera/internal/software_isp/benchmark.h"
@@ -88,6 +89,7 @@  private:
 protected:
 	void setParams(DebayerParams &params);
 	void dmaSyncBegin(std::vector<DmaSyncer> &dmaSyncers, FrameBuffer *input, FrameBuffer *output);
+	bool isStandardBayerOrder(BayerFormat::Order order);
 };
 
 } /* namespace libcamera */
diff --git a/src/libcamera/software_isp/debayer_cpu.cpp b/src/libcamera/software_isp/debayer_cpu.cpp
index 8f1b4e53d..00738c56b 100644
--- a/src/libcamera/software_isp/debayer_cpu.cpp
+++ b/src/libcamera/software_isp/debayer_cpu.cpp
@@ -288,12 +288,6 @@  void DebayerCpu::debayer10P_RGRG_BGR888(uint8_t *dst, const uint8_t *src[])
 	}
 }
 
-static bool isStandardBayerOrder(BayerFormat::Order order)
-{
-	return order == BayerFormat::BGGR || order == BayerFormat::GBRG ||
-	       order == BayerFormat::GRBG || order == BayerFormat::RGGB;
-}
-
 /*
  * Setup the Debayer object according to the passed in parameters.
  * Return 0 on success, a negative errno value on failure