diff --git a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp index b69d38c31..2b9cfee21 100644 --- a/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp +++ b/src/ipa/rpi/cam_helper/cam_helper_imx290.cpp @@ -14,7 +14,7 @@ using namespace RPiController; class CamHelperImx290 : public CamHelper { public: - CamHelperImx290(); + CamHelperImx290(unsigned int frameIntegrationDiff = kFrameIntegrationDiff); uint32_t gainCode(double gain) const override; double gain(uint32_t gainCode) const override; unsigned int hideFramesStartup() const override; @@ -25,10 +25,10 @@ class CamHelperImx290 : public CamHelper * Smallest difference between the frame length and integration time, * in units of lines. */ - static constexpr int frameIntegrationDiff = 2; + static constexpr unsigned int kFrameIntegrationDiff = 2; }; -CamHelperImx290::CamHelperImx290() +CamHelperImx290::CamHelperImx290(unsigned int frameIntegrationDiff) : CamHelper({}, frameIntegrationDiff) { } @@ -61,7 +61,18 @@ static CamHelper *create() return new CamHelperImx290(); } +static CamHelper *createImx662() +{ + /* + * The imx662 requires a frame integration diff of at least 4, + * according to the datasheet, but in practice this didn't prevent + * bad black level values in HCG (high conversion gain) mode. + * So instead, we go with 6 for "safety". + */ + return new CamHelperImx290(6); +} + static RegisterCamHelper reg("imx290", &create); static RegisterCamHelper reg327("imx327", &create); static RegisterCamHelper reg462("imx462", &create); -static RegisterCamHelper reg662("imx662", &create); +static RegisterCamHelper reg662("imx662", &createImx662); diff --git a/src/ipa/rpi/controller/rpi/black_level.cpp b/src/ipa/rpi/controller/rpi/black_level.cpp index 42ea15050..876bc661d 100644 --- a/src/ipa/rpi/controller/rpi/black_level.cpp +++ b/src/ipa/rpi/controller/rpi/black_level.cpp @@ -41,12 +41,29 @@ int BlackLevel::read(const libcamera::ValueNode ¶ms) << " Read black levels red " << blackLevelR_ << " green " << blackLevelG_ << " blue " << blackLevelB_; + + /* Allow "black_level_func" as a shorthand for all 3 colours. */ + libcamera::ipa::Pwl blackLevelFunc; + blackLevelFunc = params["black_level_func"].get(ipa::Pwl{}); + blackLevelFuncR_ = params["black_level_func_r"].get(blackLevelFunc); + blackLevelFuncG_ = params["black_level_func_g"].get(blackLevelFunc); + blackLevelFuncB_ = params["black_level_func_b"].get(blackLevelFunc); + return 0; } void BlackLevel::initialValues(uint16_t &blackLevelR, uint16_t &blackLevelG, uint16_t &blackLevelB) { + if (!blackLevelFuncR_.empty()) + blackLevelR_ = blackLevelFuncR_.eval(1.0); + + if (!blackLevelFuncG_.empty()) + blackLevelG_ = blackLevelFuncG_.eval(1.0); + + if (!blackLevelFuncB_.empty()) + blackLevelB_ = blackLevelFuncB_.eval(1.0); + blackLevelR = blackLevelR_; blackLevelG = blackLevelG_; blackLevelB = blackLevelB_; @@ -54,10 +71,18 @@ void BlackLevel::initialValues(uint16_t &blackLevelR, uint16_t &blackLevelG, void BlackLevel::prepare(Metadata *imageMetadata) { - /* - * Possibly we should think about doing this in a switchMode or - * something? - */ + DeviceStatus deviceStatus; + if (!imageMetadata->get("device.status", deviceStatus)) { + if (!blackLevelFuncR_.empty()) + blackLevelR_ = blackLevelFuncR_.eval(deviceStatus.analogueGain); + + if (!blackLevelFuncG_.empty()) + blackLevelG_ = blackLevelFuncG_.eval(deviceStatus.analogueGain); + + if (!blackLevelFuncB_.empty()) + blackLevelB_ = blackLevelFuncB_.eval(deviceStatus.analogueGain); + } + struct BlackLevelStatus status; status.blackLevelR = blackLevelR_; status.blackLevelG = blackLevelG_; diff --git a/src/ipa/rpi/controller/rpi/black_level.h b/src/ipa/rpi/controller/rpi/black_level.h index dbf29b282..34bb81548 100644 --- a/src/ipa/rpi/controller/rpi/black_level.h +++ b/src/ipa/rpi/controller/rpi/black_level.h @@ -6,6 +6,8 @@ */ #pragma once +#include + #include "../black_level_algorithm.h" #include "../black_level_status.h" @@ -27,6 +29,11 @@ class BlackLevel : public BlackLevelAlgorithm double blackLevelR_; double blackLevelG_; double blackLevelB_; + + /* Black levels can vary with analogue gain instead of being constant. */ + libcamera::ipa::Pwl blackLevelFuncR_; + libcamera::ipa::Pwl blackLevelFuncG_; + libcamera::ipa::Pwl blackLevelFuncB_; }; } /* namespace RPiController */ diff --git a/src/ipa/rpi/pisp/data/imx662.json b/src/ipa/rpi/pisp/data/imx662.json index b146d6b59..8491452b1 100644 --- a/src/ipa/rpi/pisp/data/imx662.json +++ b/src/ipa/rpi/pisp/data/imx662.json @@ -5,7 +5,16 @@ { "rpi.black_level": { - "black_level": 3200 + "black_level_func": + [ + 1.0, 3200, + 192.0, 3200, + 256.0, 3245, + 320.0, 3331, + 384.0, 3443, + 480.0, 3606, + 512.0, 3623 + ] } }, { @@ -146,7 +155,7 @@ { "auto": { - "lo": 2910.0, + "lo": 2500.0, "hi": 5535.0 }, "incandescent": @@ -183,6 +192,7 @@ "bayes": 1, "ct_curve": [ + 2500.0, 0.6632, 0.2893, 2910.0, 0.5579, 0.3553, 3600.0, 0.4693, 0.4364, 4490.0, 0.3855, 0.5469, @@ -241,6 +251,7 @@ "enable_nn": 1, "ct_curve": [ + 2500.0, 0.6632, 0.2893, 2910.0, 0.5579, 0.3553, 3600.0, 0.4693, 0.4364, 4490.0, 0.3855, 0.5469, diff --git a/src/ipa/rpi/vc4/data/imx662.json b/src/ipa/rpi/vc4/data/imx662.json index cec937d62..aafccaced 100644 --- a/src/ipa/rpi/vc4/data/imx662.json +++ b/src/ipa/rpi/vc4/data/imx662.json @@ -5,7 +5,16 @@ { "rpi.black_level": { - "black_level": 3200 + "black_level_func": + [ + 1.0, 3200, + 192.0, 3200, + 256.0, 3245, + 320.0, 3331, + 384.0, 3443, + 480.0, 3606, + 512.0, 3623 + ] } }, { @@ -77,7 +86,7 @@ { "auto": { - "lo": 2910.0, + "lo": 2500.0, "hi": 5535.0 }, "incandescent": @@ -114,6 +123,7 @@ "bayes": 1, "ct_curve": [ + 2500.0, 0.6632, 0.2893, 2910.0, 0.5583, 0.3541, 3600.0, 0.4695, 0.4348, 4490.0, 0.3856, 0.5447,