@@ -46,6 +46,33 @@ constexpr float kDefaultSaturation = 1.0f;
*/
constexpr float kHueScale = -90.0f;
+void applyRequestedBrightness(IPAActiveState &activeState)
+{
+ auto &cproc = activeState.cproc;
+
+ /*
+ * The CPROC module applies the transfer function
+ *
+ * Yout = Yin * contrast + brightness/2
+ *
+ * The calculations are done one bit wider than the input data to
+ * account for the maximum contrast of 2x without clamping. Brightness
+ * is applied after and therefore affects the output only with half the
+ * value.
+ *
+ * Most users expect that changing contrast doesn't change the middle
+ * gray and that the brightness value is normalized to one. So they
+ * expect a transfer function of
+ *
+ * Yout = (Yin - 0.5) * contrast + 0.5 + brightness
+ *
+ * To implement above formula with the given hardware we need adjust the
+ * hardware brightness value depending on the contrast.
+ */
+ cproc.brightness = (1 - cproc.contrast.value()) + 2 * cproc.requestedBrightness;
+ cproc.actualBrightness = (cproc.brightness.value() - (1 - cproc.contrast.value())) / 2;
+}
+
} /* namespace */
/**
@@ -76,8 +103,10 @@ int ColorProcessing::configure(IPAContext &context,
{
auto &cproc = context.activeState.cproc;
- cproc.brightness = BrightnessQ(kDefaultBrightness);
+ cproc.requestedBrightness = kDefaultBrightness;
cproc.contrast = ContrastQ(kDefaultContrast);
+ applyRequestedBrightness(context.activeState);
+
cproc.hue = HueQ(kDefaultHue);
cproc.saturation = SaturationQ(kDefaultSaturation);
@@ -98,17 +127,6 @@ void ColorProcessing::queueRequest(IPAContext &context,
if (frame == 0)
update = true;
- const auto &brightness = controls.get(controls::Brightness);
- if (brightness) {
- BrightnessQ value = *brightness;
- if (cproc.brightness != value) {
- cproc.brightness = value;
- update = true;
- }
-
- LOG(RkISP1CProc, Debug) << "Set brightness to " << value;
- }
-
const auto &contrast = controls.get(controls::Contrast);
if (contrast) {
ContrastQ value = *contrast;
@@ -120,6 +138,19 @@ void ColorProcessing::queueRequest(IPAContext &context,
LOG(RkISP1CProc, Debug) << "Set contrast to " << value;
}
+ const auto &brightness = controls.get(controls::Brightness);
+ if (brightness)
+ cproc.requestedBrightness = *brightness;
+
+ if (update || brightness) {
+ BrightnessQ old = cproc.brightness;
+ applyRequestedBrightness(context.activeState);
+ if (cproc.brightness != old)
+ update = true;
+
+ LOG(RkISP1CProc, Debug) << "Set brightness to " << cproc.actualBrightness;
+ }
+
const auto &hue = controls.get(controls::Hue);
if (hue) {
/* Scale the Hue from ]-90, +90] */
@@ -144,6 +175,7 @@ void ColorProcessing::queueRequest(IPAContext &context,
}
frameContext.cproc.brightness = cproc.brightness;
+ frameContext.cproc.actualBrightness = cproc.actualBrightness;
frameContext.cproc.contrast = cproc.contrast;
frameContext.cproc.hue = cproc.hue;
frameContext.cproc.saturation = cproc.saturation;
@@ -179,7 +211,7 @@ void ColorProcessing::process([[maybe_unused]] IPAContext &context,
[[maybe_unused]] const rkisp1_stat_buffer *stats,
ControlList &metadata)
{
- metadata.set(controls::Brightness, frameContext.cproc.brightness.value());
+ metadata.set(controls::Brightness, frameContext.cproc.actualBrightness);
metadata.set(controls::Contrast, frameContext.cproc.contrast.value());
metadata.set(controls::Hue, frameContext.cproc.hue.value() * kHueScale);
metadata.set(controls::Saturation, frameContext.cproc.saturation.value());
@@ -232,9 +232,15 @@ namespace libcamera::ipa::rkisp1 {
/**
* \var IPAActiveState::cproc
* \brief State for the Color Processing algorithm
+ *
+ * \var IPAActiveState::cproc.requestedBrightness
+ * \brief Brightness level requested by the user
+ *
+ * \var IPAActiveState::cproc.actualBrightness
+ * \brief Brightness level after contrast compensation
*
- * \struct IPAActiveState::cproc.brightness
- * \brief Brightness level
+ * \var IPAActiveState::cproc.brightness
+ * \brief Hardware brightness level
*
* \var IPAActiveState::cproc.contrast
* \brief Contrast level
@@ -400,8 +406,11 @@ namespace libcamera::ipa::rkisp1 {
* \var IPAFrameContext::cproc
* \brief Color Processing parameters for this frame
*
- * \struct IPAFrameContext::cproc.brightness
- * \brief Brightness level
+ * \var IPAFrameContext::cproc.actualBrightness
+ * \brief Brightness level after contrast compensation
+ *
+ * \var IPAFrameContext::cproc.brightness
+ * \brief Hardware brightness level
*
* \var IPAFrameContext::cproc.contrast
* \brief Contrast level
@@ -122,6 +122,8 @@ struct IPAActiveState {
} ccm;
struct {
+ float requestedBrightness;
+ float actualBrightness;
BrightnessQ brightness;
ContrastQ contrast;
HueQ hue;
@@ -181,6 +183,7 @@ struct IPAFrameContext : public FrameContext {
} awb;
struct {
+ float actualBrightness;
BrightnessQ brightness;
ContrastQ contrast;
HueQ hue;
By reverse engineering it was discovered that the transfer curve applied by the CPROC module is Yout = Yin * contrast + brightness/2. However the most common expectation on contrast is to not change the middle gray of an image. So the expected curve is: Yout = (Yin - 0.5) * contrast + 0.5 + brightness Map the user expectation to the hardware by changing the brightness value written to the hardware. This also includes multiplying the requested brightness value by two. Note: The limits of the brightness control are left as is even though they can not be reached without changing the contrast. But limiting them to [0.5, 0.5] would cut off parts of the combined control space of contrast + brightness. Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- This patch needs to be applied on top of the quantization series: https://patchwork.libcamera.org/project/libcamera/list/?series=5708 Changes in v2: - Moved calculation of brightness into helper function, to be able to reuse it inside configure() - Moved calculation of actualBrightness from process() to the helper function - Added documentation for the new context variables - Removed unnecessary curly braces - Improved comment --- src/ipa/rkisp1/algorithms/cproc.cpp | 58 ++++++++++++++++++++++------- src/ipa/rkisp1/ipa_context.cpp | 17 +++++++-- src/ipa/rkisp1/ipa_context.h | 3 ++ 3 files changed, 61 insertions(+), 17 deletions(-)