[libcamera-devel,v2,3/4] ipa: rkisp1: Fail hard on empty CameraSensorInfo
diff mbox series

Message ID 20221123134346.129807-4-jacopo@jmondi.org
State Accepted
Commit 13d85e632a507d68d0c04f6329078a40b79c7afd
Headers show
Series
  • ipa: Validate controls in CameraSensor
Related show

Commit Message

Jacopo Mondi Nov. 23, 2022, 1:43 p.m. UTC 
The RkISP1 pipeline and IPA module allows for the CameraSensorInfo to be
empty, probably to accommodate some sensor used in a test platform that
does not provide the mandatory libcamera requirements.

As the \todo item in the IPA reports, there is a possibility that the
received CameraSensorInfo is empty and it should be checked before
accessing it, but currently such requirement is not enforced in the
code.

This allows to assume all the test platforms in use have now
successfully moved their sensor driver to comply with the minimum
requirements and provide a populated CameraSensorInfo to the IPA.

As the safety check is not enforced, and as we don't want to allow
faulty sensors to send empty CameraSensorInfo to the IPA, remove the
\todo item in the IPA and fail hard in the pipeline handler if the
sensor does not comply with libcamera requirements.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/ipa/rkisp1/rkisp1.cpp                | 6 ------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 ++---
 2 files changed, 2 insertions(+), 9 deletions(-)

Patch
diff mbox series 

diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 088d7c74d448..a9e144303072 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -199,12 +199,6 @@  void IPARkISP1::stop()
 	context_.frameContexts.clear();
 }
 
-/**
- * \todo The RkISP1 pipeline currently provides an empty IPACameraSensorInfo
- * if the connected sensor does not provide enough information to properly
- * assemble one. Make sure the reported sensor information are relevant
- * before accessing them.
- */
 int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
 			 [[maybe_unused]] const std::map<uint32_t, IPAStream> &streamConfig)
 {
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 4f0e1f8b1a35..e946ccc4c930 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -719,9 +719,8 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	ret = data->sensor_->sensorInfo(&ipaConfig.sensorInfo);
 	if (ret) {
-		/* \todo Turn this into a hard failure. */
-		LOG(RkISP1, Warning) << "Camera sensor information not available";
-		ipaConfig.sensorInfo = {};
+		LOG(RkISP1, Error) << "Camera sensor information not available";
+		return ret;
 	}
 
 	ipaConfig.sensorControls = data->sensor_->controls();