[libcamera-devel,v5,22/33] ipa: rkisp1: cproc: Store per-frame information in frame context
diff mbox series

Message ID 20220927023642.12341-23-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • ipa: Frame context queue, IPU3 & RkISP consolidation, and RkISP1 improvements
Related show

Commit Message

Laurent Pinchart Sept. 27, 2022, 2:36 a.m. UTC 
Rework the algorithm's usage of the active state, to store the value of
controls for the last queued request in the queueRequest() function, and
store a copy of the values in the corresponding frame context. The
latter is used in the prepare() function to populate the ISP parameters
with values corresponding to the right frame.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/ipa/rkisp1/algorithms/cproc.cpp | 51 ++++++++++++++++++-----------
 src/ipa/rkisp1/ipa_context.cpp      | 21 ++++++++++--
 src/ipa/rkisp1/ipa_context.h        |  8 ++++-
 3 files changed, 56 insertions(+), 24 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/algorithms/cproc.cpp b/src/ipa/rkisp1/algorithms/cproc.cpp
index ea819b2acfcb..eaa56c37f2bb 100644
--- a/src/ipa/rkisp1/algorithms/cproc.cpp
+++ b/src/ipa/rkisp1/algorithms/cproc.cpp
@@ -38,55 +38,66 @@  LOG_DEFINE_CATEGORY(RkISP1CProc)
  */
 void ColorProcessing::queueRequest(IPAContext &context,
 				   [[maybe_unused]] const uint32_t frame,
-				   [[maybe_unused]] IPAFrameContext &frameContext,
+				   IPAFrameContext &frameContext,
 				   const ControlList &controls)
 {
 	auto &cproc = context.activeState.cproc;
+	bool update = false;
 
 	const auto &brightness = controls.get(controls::Brightness);
 	if (brightness) {
-		cproc.brightness = std::clamp<int>(std::lround(*brightness * 128), -128, 127);
-		cproc.updateParams = true;
+		int value = std::clamp<int>(std::lround(*brightness * 128), -128, 127);
+		if (cproc.brightness != value) {
+			cproc.brightness = value;
+			update = true;
+		}
 
-		LOG(RkISP1CProc, Debug) << "Set brightness to " << *brightness;
+		LOG(RkISP1CProc, Debug) << "Set brightness to " << value;
 	}
 
 	const auto &contrast = controls.get(controls::Contrast);
 	if (contrast) {
-		cproc.contrast = std::clamp<int>(std::lround(*contrast * 128), 0, 255);
-		cproc.updateParams = true;
+		int value = std::clamp<int>(std::lround(*contrast * 128), 0, 255);
+		if (cproc.contrast != value) {
+			cproc.contrast = value;
+			update = true;
+		}
 
-		LOG(RkISP1CProc, Debug) << "Set contrast to " << *contrast;
+		LOG(RkISP1CProc, Debug) << "Set contrast to " << value;
 	}
 
 	const auto saturation = controls.get(controls::Saturation);
 	if (saturation) {
-		cproc.saturation = std::clamp<int>(std::lround(*saturation * 128), 0, 255);
-		cproc.updateParams = true;
+		int value = std::clamp<int>(std::lround(*saturation * 128), 0, 255);
+		if (cproc.saturation != value) {
+			cproc.saturation = value;
+			update = true;
+		}
 
-		LOG(RkISP1CProc, Debug) << "Set saturation to " << *saturation;
+		LOG(RkISP1CProc, Debug) << "Set saturation to " << value;
 	}
+
+	frameContext.cproc.brightness = cproc.brightness;
+	frameContext.cproc.contrast = cproc.contrast;
+	frameContext.cproc.saturation = cproc.saturation;
+	frameContext.cproc.update = update;
 }
 
 /**
  * \copydoc libcamera::ipa::Algorithm::prepare
  */
-void ColorProcessing::prepare(IPAContext &context,
+void ColorProcessing::prepare([[maybe_unused]] IPAContext &context,
 			      [[maybe_unused]] const uint32_t frame,
-			      [[maybe_unused]] IPAFrameContext &frameContext,
+			      IPAFrameContext &frameContext,
 			      rkisp1_params_cfg *params)
 {
-	auto &cproc = context.activeState.cproc;
-
 	/* Check if the algorithm configuration has been updated. */
-	if (!cproc.updateParams)
+	if (!frameContext.cproc.update)
 		return;
 
-	cproc.updateParams = false;
-
-	params->others.cproc_config.brightness = cproc.brightness;
-	params->others.cproc_config.contrast = cproc.contrast;
-	params->others.cproc_config.sat = cproc.saturation;
+	params->others.cproc_config.brightness = frameContext.cproc.brightness;
+	params->others.cproc_config.contrast = frameContext.cproc.contrast;
+	params->others.cproc_config.sat = frameContext.cproc.saturation;
 
 	params->module_en_update |= RKISP1_CIF_ISP_MODULE_CPROC;
 	params->module_ens |= RKISP1_CIF_ISP_MODULE_CPROC;
diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp
index ba80a0707ef7..fdc8e7df0b76 100644
--- a/src/ipa/rkisp1/ipa_context.cpp
+++ b/src/ipa/rkisp1/ipa_context.cpp
@@ -160,9 +160,6 @@  namespace libcamera::ipa::rkisp1 {
  *
  * \var IPAActiveState::cproc.saturation
  * \brief Saturation level
- *
- * \var IPAActiveState::cproc.updateParams
- * \brief Indicates if ISP parameters need to be updated
  */
 
 /**
@@ -236,6 +233,24 @@  namespace libcamera::ipa::rkisp1 {
  * \brief Whether the Auto White Balance algorithm is enabled
  */
 
+/**
+ * \var IPAFrameContext::cproc
+ * \brief Color Processing parameters for this frame
+ *
+ * \struct IPAFrameContext::cproc.brightness
+ * \brief Brightness level
+ *
+ * \var IPAFrameContext::cproc.contrast
+ * \brief Contrast level
+ *
+ * \var IPAFrameContext::cproc.saturation
+ * \brief Saturation level
+ *
+ * \var IPAFrameContext::cproc.update
+ * \brief Indicates if the color processing parameters have been updated
+ * compared to the previous frame
+ */
+
 /**
  * \var IPAFrameContext::sensor
  * \brief Sensor configuration that used been used for this frame
diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h
index f23ebb2c9004..cc217aec511a 100644
--- a/src/ipa/rkisp1/ipa_context.h
+++ b/src/ipa/rkisp1/ipa_context.h
@@ -76,7 +76,6 @@  struct IPAActiveState {
 		int8_t brightness;
 		uint8_t contrast;
 		uint8_t saturation;
-		bool updateParams;
 	} cproc;
 
 	struct {
@@ -108,6 +107,13 @@  struct IPAFrameContext : public FrameContext {
 		bool autoEnabled;
 	} awb;
 
+	struct {
+		int8_t brightness;
+		uint8_t contrast;
+		uint8_t saturation;
+		bool update;
+	} cproc;
+
 	struct {
 		uint32_t exposure;
 		double gain;