[PATCH 5/5] libcamera: software_isp: Remove DebayerParams::kGain10
Milan Zamazal
mzamazal at redhat.com
Mon Apr 29 10:44:31 CEST 2024
Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
> Hi Milan,
>
> Thank you for the patch.
>
> On Tue, Apr 23, 2024 at 08:20:00PM +0200, Milan Zamazal wrote:
>> The constant is used in a single place internally and doesn't belong to
>> DebayerParams anymore. Let's use 256 directly.
>>
>> Completes software ISP TODO #4.
>
> The change doesn't address the TODO item.
Hi Laurent,
so is the remaining step to change the item type of ColorLookupTable's from
uint8_t (0..255) to float (0.0..1.0) and converting the tables just before
debayering to uint8_t for the sake of separation? Anything else?
Thanks,
Milan
>> Signed-off-by: Milan Zamazal <mzamazal at redhat.com>
>> ---
>> .../internal/software_isp/debayer_params.h | 1 -
>> src/ipa/simple/soft_simple.cpp | 3 +--
>> src/libcamera/software_isp/TODO | 13 -------------
>> src/libcamera/software_isp/debayer.cpp | 5 -----
>> 4 files changed, 1 insertion(+), 21 deletions(-)
>>
>> diff --git a/include/libcamera/internal/software_isp/debayer_params.h b/include/libcamera/internal/software_isp/debayer_params.h
>> index 09f4ff00..6aaa7d4c 100644
>> --- a/include/libcamera/internal/software_isp/debayer_params.h
>> +++ b/include/libcamera/internal/software_isp/debayer_params.h
>> @@ -16,7 +16,6 @@
>> namespace libcamera {
>>
>> struct DebayerParams {
>> - static constexpr unsigned int kGain10 = 256;
>> static constexpr unsigned int kRGBLookupSize = 256;
>>
>> using ColorLookupTable = std::array<uint8_t, kRGBLookupSize>;
>> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp
>> index b2f4d308..5298836c 100644
>> --- a/src/ipa/simple/soft_simple.cpp
>> +++ b/src/ipa/simple/soft_simple.cpp
>> @@ -300,8 +300,7 @@ void IPASoftSimple::processStats(const ControlList &sensorControls)
>>
>> for (unsigned int i = 0; i < DebayerParams::kRGBLookupSize; i++) {
>> constexpr unsigned int div =
>> - DebayerParams::kRGBLookupSize * DebayerParams::kGain10 /
>> - kGammaLookupSize;
>> + DebayerParams::kRGBLookupSize * 256 / kGammaLookupSize;
>> unsigned int idx;
>>
>> /* Apply gamma after gain! */
>> diff --git a/src/libcamera/software_isp/TODO b/src/libcamera/software_isp/TODO
>> index fcb02588..f8f00139 100644
>> --- a/src/libcamera/software_isp/TODO
>> +++ b/src/libcamera/software_isp/TODO
>> @@ -72,19 +72,6 @@ stats in hardware, such as the i.MX7), but please keep it on your radar.
>>
>> ---
>>
>> -4. Hide internal representation of gains from callers
>> -
>> -> struct DebayerParams {
>> -> static constexpr unsigned int kGain10 = 256;
>> -
>> -Forcing the caller to deal with the internal representation of gains
>> -isn't nice, especially given that it precludes implementing gains of
>> -different precisions in different backend. Wouldn't it be better to pass
>> -the values as floating point numbers, and convert them to the internal
>> -representation in the implementation of process() before using them ?
>> -
>> ----
>> -
>> 5. Store ISP parameters in per-frame buffers
>>
>> > /**
>> diff --git a/src/libcamera/software_isp/debayer.cpp b/src/libcamera/software_isp/debayer.cpp
>> index ac438a33..548c2ccd 100644
>> --- a/src/libcamera/software_isp/debayer.cpp
>> +++ b/src/libcamera/software_isp/debayer.cpp
>> @@ -18,11 +18,6 @@ namespace libcamera {
>> * \brief Struct to hold the debayer parameters.
>> */
>>
>> -/**
>> - * \var DebayerParams::kGain10
>> - * \brief const value for 1.0 gain
>> - */
>> -
>> /**
>> * \var DebayerParams::kRGBLookupSize
>> * \brief Size of a color lookup table
More information about the libcamera-devel
mailing list