[libcamera-devel,v4,13/14] ipa: ipu3: Move ExposureTime to IPA
diff mbox series

Message ID 20211111140928.136111-14-jeanmichel.hautbois@ideasonboard.com
State Accepted
Headers show
Series
  • IPA: IPU3: Introduce per-frame controls
Related show

Commit Message

Jean-Michel Hautbois Nov. 11, 2021, 2:09 p.m. UTC 
Now that we have the exposure time calculated, pass it to the
controls::ExposureTime and don't use the pipeline handler for it
anymore.

Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/ipa/ipu3/ipu3.cpp                |  2 ++
 src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +---------------
 2 files changed, 3 insertions(+), 15 deletions(-)

Comments

Umang Jain Nov. 12, 2021, 12:43 p.m. UTC | #1 
Hi JM

Thank you for the patch,

On 11/11/21 7:39 PM, Jean-Michel Hautbois wrote:
> Now that we have the exposure time calculated, pass it to the
> controls::ExposureTime and don't use the pipeline handler for it
> anymore.
>
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

> ---
>   src/ipa/ipu3/ipu3.cpp                |  2 ++
>   src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +---------------
>   2 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index f1597f78..dc6f2ced 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -630,6 +630,8 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>   
>   	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>   
> +	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
> +
>   	/*
>   	 * \todo The Metadata provides a path to getting extended data
>   	 * out to the application. Further data such as a simplifed Histogram
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index c984ed12..319bb1a3 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -47,7 +47,7 @@ class IPU3CameraData : public Camera::Private
>   {
>   public:
>   	IPU3CameraData(PipelineHandler *pipe)
> -		: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)
> +		: Camera::Private(pipe), supportsFlips_(false)
>   	{
>   	}
>   
> @@ -67,7 +67,6 @@ public:
>   	Stream vfStream_;
>   	Stream rawStream_;
>   
> -	uint32_t exposureTime_;
>   	Rectangle cropRegion_;
>   	bool supportsFlips_;
>   	Transform rotationTransform_;
> @@ -1045,17 +1044,6 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)
>   
>   	controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
>   
> -	/*
> -	 * \todo Report the actual exposure time, use the default for the
> -	 * moment.
> -	 */
> -	const auto exposureInfo = data->ipaControls_.find(&controls::ExposureTime);
> -	if (exposureInfo == data->ipaControls_.end()) {
> -		LOG(IPU3, Error) << "Exposure control not initialized by the IPA";
> -		return -EINVAL;
> -	}
> -	data->exposureTime_ = exposureInfo->second.def().get<int32_t>();
> -
>   	/* Add the IPA registered controls to list of camera controls. */
>   	for (const auto &ipaControl : data->ipaControls_)
>   		controls[ipaControl.first] = ipaControl.second;
> @@ -1317,8 +1305,6 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>   	pipe()->completeBuffer(request, buffer);
>   
>   	request->metadata().set(controls::draft::PipelineDepth, 3);
> -	/* \todo Move the ExposureTime control to the IPA. */
> -	request->metadata().set(controls::ExposureTime, exposureTime_);
>   	/* \todo Actually apply the scaler crop region to the ImgU. */
>   	if (request->controls().contains(controls::ScalerCrop))
>   		cropRegion_ = request->controls().get(controls::ScalerCrop);
Paul Elder Nov. 12, 2021, 11:24 p.m. UTC | #2 
Hi Jean-Michel,

On Thu, Nov 11, 2021 at 03:09:27PM +0100, Jean-Michel Hautbois wrote:
> Now that we have the exposure time calculated, pass it to the

s/pass it to the/report it in/

> controls::ExposureTime and don't use the pipeline handler for it
> anymore.

yaaaaassss

> 
> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> ---
>  src/ipa/ipu3/ipu3.cpp                |  2 ++
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 16 +---------------
>  2 files changed, 3 insertions(+), 15 deletions(-)
> 
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index f1597f78..dc6f2ced 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -630,6 +630,8 @@ void IPAIPU3::parseStatistics(unsigned int frame,
>  
>  	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
>  
> +	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
> +
>  	/*
>  	 * \todo The Metadata provides a path to getting extended data
>  	 * out to the application. Further data such as a simplifed Histogram
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index c984ed12..319bb1a3 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -47,7 +47,7 @@ class IPU3CameraData : public Camera::Private
>  {
>  public:
>  	IPU3CameraData(PipelineHandler *pipe)
> -		: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)
> +		: Camera::Private(pipe), supportsFlips_(false)
>  	{
>  	}
>  
> @@ -67,7 +67,6 @@ public:
>  	Stream vfStream_;
>  	Stream rawStream_;
>  
> -	uint32_t exposureTime_;
>  	Rectangle cropRegion_;
>  	bool supportsFlips_;
>  	Transform rotationTransform_;
> @@ -1045,17 +1044,6 @@ int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)
>  
>  	controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
>  
> -	/*
> -	 * \todo Report the actual exposure time, use the default for the
> -	 * moment.
> -	 */
> -	const auto exposureInfo = data->ipaControls_.find(&controls::ExposureTime);
> -	if (exposureInfo == data->ipaControls_.end()) {
> -		LOG(IPU3, Error) << "Exposure control not initialized by the IPA";
> -		return -EINVAL;
> -	}
> -	data->exposureTime_ = exposureInfo->second.def().get<int32_t>();
> -
>  	/* Add the IPA registered controls to list of camera controls. */
>  	for (const auto &ipaControl : data->ipaControls_)
>  		controls[ipaControl.first] = ipaControl.second;
> @@ -1317,8 +1305,6 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
>  	pipe()->completeBuffer(request, buffer);
>  
>  	request->metadata().set(controls::draft::PipelineDepth, 3);
> -	/* \todo Move the ExposureTime control to the IPA. */
> -	request->metadata().set(controls::ExposureTime, exposureTime_);
>  	/* \todo Actually apply the scaler crop region to the ImgU. */
>  	if (request->controls().contains(controls::ScalerCrop))
>  		cropRegion_ = request->controls().get(controls::ScalerCrop);
> -- 
> 2.32.0
>

Patch
diff mbox series 

diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index f1597f78..dc6f2ced 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -630,6 +630,8 @@  void IPAIPU3::parseStatistics(unsigned int frame,
 
 	ctrls.set(controls::ColourTemperature, context_.frameContext.awb.temperatureK);
 
+	ctrls.set(controls::ExposureTime, context_.frameContext.sensor.exposure * lineDuration_.get<std::micro>());
+
 	/*
 	 * \todo The Metadata provides a path to getting extended data
 	 * out to the application. Further data such as a simplifed Histogram
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index c984ed12..319bb1a3 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -47,7 +47,7 @@  class IPU3CameraData : public Camera::Private
 {
 public:
 	IPU3CameraData(PipelineHandler *pipe)
-		: Camera::Private(pipe), exposureTime_(0), supportsFlips_(false)
+		: Camera::Private(pipe), supportsFlips_(false)
 	{
 	}
 
@@ -67,7 +67,6 @@  public:
 	Stream vfStream_;
 	Stream rawStream_;
 
-	uint32_t exposureTime_;
 	Rectangle cropRegion_;
 	bool supportsFlips_;
 	Transform rotationTransform_;
@@ -1045,17 +1044,6 @@  int PipelineHandlerIPU3::updateControls(IPU3CameraData *data)
 
 	controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);
 
-	/*
-	 * \todo Report the actual exposure time, use the default for the
-	 * moment.
-	 */
-	const auto exposureInfo = data->ipaControls_.find(&controls::ExposureTime);
-	if (exposureInfo == data->ipaControls_.end()) {
-		LOG(IPU3, Error) << "Exposure control not initialized by the IPA";
-		return -EINVAL;
-	}
-	data->exposureTime_ = exposureInfo->second.def().get<int32_t>();
-
 	/* Add the IPA registered controls to list of camera controls. */
 	for (const auto &ipaControl : data->ipaControls_)
 		controls[ipaControl.first] = ipaControl.second;
@@ -1317,8 +1305,6 @@  void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
 	pipe()->completeBuffer(request, buffer);
 
 	request->metadata().set(controls::draft::PipelineDepth, 3);
-	/* \todo Move the ExposureTime control to the IPA. */
-	request->metadata().set(controls::ExposureTime, exposureTime_);
 	/* \todo Actually apply the scaler crop region to the ImgU. */
 	if (request->controls().contains(controls::ScalerCrop))
 		cropRegion_ = request->controls().get(controls::ScalerCrop);