[libcamera-devel,v3,1/8] rkisp1: Add camera lens to PH and expose it to the IPA
diff mbox series

Message ID 20230119084112.20564-2-dse@thaumatec.com
State Superseded
Headers show
Series
  • ipa: rkisp1: Add autofocus algorithm
Related show

Commit Message

Daniel Semkowicz Jan. 19, 2023, 8:41 a.m. UTC 
Check in pipeline handler if camera lens exists, add expose its controls
to the IPA.

Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>
---
 include/libcamera/ipa/rkisp1.mojom       | 1 +
 src/ipa/rkisp1/rkisp1.cpp                | 5 +++++
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++++
 3 files changed, 11 insertions(+)

Comments

Kieran Bingham Feb. 4, 2023, 10:44 p.m. UTC | #1 
Quoting Daniel Semkowicz via libcamera-devel (2023-01-19 08:41:05)
> Check in pipeline handler if camera lens exists, add expose its controls
> to the IPA.
> 

Looks ok to me.


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>
> ---
>  include/libcamera/ipa/rkisp1.mojom       | 1 +
>  src/ipa/rkisp1/rkisp1.cpp                | 5 +++++
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> index 1009e970..bf6e9141 100644
> --- a/include/libcamera/ipa/rkisp1.mojom
> +++ b/include/libcamera/ipa/rkisp1.mojom
> @@ -11,6 +11,7 @@ import "include/libcamera/ipa/core.mojom";
>  struct IPAConfigInfo {
>         libcamera.IPACameraSensorInfo sensorInfo;
>         libcamera.ControlInfoMap sensorControls;
> +       libcamera.ControlInfoMap lensControls;
>  };
>  
>  interface IPARkISP1Interface {
> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> index 6544c925..9e861fc0 100644
> --- a/src/ipa/rkisp1/rkisp1.cpp
> +++ b/src/ipa/rkisp1/rkisp1.cpp
> @@ -7,6 +7,7 @@
>  
>  #include <algorithm>
>  #include <math.h>
> +#include <optional>
>  #include <queue>
>  #include <stdint.h>
>  #include <string.h>
> @@ -80,6 +81,7 @@ private:
>         std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
>  
>         ControlInfoMap sensorControls_;
> +       std::optional<ControlInfoMap> lensControls_;
>  
>         /* revision-specific data */
>         rkisp1_cif_isp_version hwRevision_;
> @@ -215,6 +217,9 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
>  {
>         sensorControls_ = ipaConfig.sensorControls;
>  
> +       if (!ipaConfig.lensControls.empty())
> +               lensControls_ = ipaConfig.lensControls;
> +
>         const auto itExp = sensorControls_.find(V4L2_CID_EXPOSURE);
>         int32_t minExposure = itExp->second.min().get<int32_t>();
>         int32_t maxExposure = itExp->second.max().get<int32_t>();
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 5bdb4d25..0559d261 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -31,6 +31,7 @@
>  #include <libcamera/ipa/rkisp1_ipa_proxy.h>
>  
>  #include "libcamera/internal/camera.h"
> +#include "libcamera/internal/camera_lens.h"
>  #include "libcamera/internal/camera_sensor.h"
>  #include "libcamera/internal/delayed_controls.h"
>  #include "libcamera/internal/device_enumerator.h"
> @@ -807,6 +808,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
>  
>         ipaConfig.sensorControls = data->sensor_->controls();
>  
> +       CameraLens *lens = data->sensor_->focusLens();
> +       if (lens)
> +               ipaConfig.lensControls = lens->controls();
> +
>         ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);
>         if (ret) {
>                 LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
> -- 
> 2.39.0
>
Jacopo Mondi Feb. 6, 2023, 9:13 a.m. UTC | #2 
Hi Daniel

On Sat, Feb 04, 2023 at 10:44:19PM +0000, Kieran Bingham via libcamera-devel wrote:
> Quoting Daniel Semkowicz via libcamera-devel (2023-01-19 08:41:05)
> > Check in pipeline handler if camera lens exists, add expose its controls
> > to the IPA.
> >
>
> Looks ok to me.
>
>
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

Looks good to me as well
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>
> > Signed-off-by: Daniel Semkowicz <dse@thaumatec.com>
> > ---
> >  include/libcamera/ipa/rkisp1.mojom       | 1 +
> >  src/ipa/rkisp1/rkisp1.cpp                | 5 +++++
> >  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 5 +++++
> >  3 files changed, 11 insertions(+)
> >
> > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
> > index 1009e970..bf6e9141 100644
> > --- a/include/libcamera/ipa/rkisp1.mojom
> > +++ b/include/libcamera/ipa/rkisp1.mojom
> > @@ -11,6 +11,7 @@ import "include/libcamera/ipa/core.mojom";
> >  struct IPAConfigInfo {
> >         libcamera.IPACameraSensorInfo sensorInfo;
> >         libcamera.ControlInfoMap sensorControls;
> > +       libcamera.ControlInfoMap lensControls;
> >  };
> >
> >  interface IPARkISP1Interface {
> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
> > index 6544c925..9e861fc0 100644
> > --- a/src/ipa/rkisp1/rkisp1.cpp
> > +++ b/src/ipa/rkisp1/rkisp1.cpp
> > @@ -7,6 +7,7 @@
> >
> >  #include <algorithm>
> >  #include <math.h>
> > +#include <optional>
> >  #include <queue>
> >  #include <stdint.h>
> >  #include <string.h>
> > @@ -80,6 +81,7 @@ private:
> >         std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
> >
> >         ControlInfoMap sensorControls_;
> > +       std::optional<ControlInfoMap> lensControls_;
> >
> >         /* revision-specific data */
> >         rkisp1_cif_isp_version hwRevision_;
> > @@ -215,6 +217,9 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
> >  {
> >         sensorControls_ = ipaConfig.sensorControls;
> >
> > +       if (!ipaConfig.lensControls.empty())
> > +               lensControls_ = ipaConfig.lensControls;
> > +
> >         const auto itExp = sensorControls_.find(V4L2_CID_EXPOSURE);
> >         int32_t minExposure = itExp->second.min().get<int32_t>();
> >         int32_t maxExposure = itExp->second.max().get<int32_t>();
> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > index 5bdb4d25..0559d261 100644
> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> > @@ -31,6 +31,7 @@
> >  #include <libcamera/ipa/rkisp1_ipa_proxy.h>
> >
> >  #include "libcamera/internal/camera.h"
> > +#include "libcamera/internal/camera_lens.h"
> >  #include "libcamera/internal/camera_sensor.h"
> >  #include "libcamera/internal/delayed_controls.h"
> >  #include "libcamera/internal/device_enumerator.h"
> > @@ -807,6 +808,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
> >
> >         ipaConfig.sensorControls = data->sensor_->controls();
> >
> > +       CameraLens *lens = data->sensor_->focusLens();
> > +       if (lens)
> > +               ipaConfig.lensControls = lens->controls();
> > +
> >         ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);
> >         if (ret) {
> >                 LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";
> > --
> > 2.39.0
> >

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom
index 1009e970..bf6e9141 100644
--- a/include/libcamera/ipa/rkisp1.mojom
+++ b/include/libcamera/ipa/rkisp1.mojom
@@ -11,6 +11,7 @@  import "include/libcamera/ipa/core.mojom";
 struct IPAConfigInfo {
 	libcamera.IPACameraSensorInfo sensorInfo;
 	libcamera.ControlInfoMap sensorControls;
+	libcamera.ControlInfoMap lensControls;
 };
 
 interface IPARkISP1Interface {
diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp
index 6544c925..9e861fc0 100644
--- a/src/ipa/rkisp1/rkisp1.cpp
+++ b/src/ipa/rkisp1/rkisp1.cpp
@@ -7,6 +7,7 @@ 
 
 #include <algorithm>
 #include <math.h>
+#include <optional>
 #include <queue>
 #include <stdint.h>
 #include <string.h>
@@ -80,6 +81,7 @@  private:
 	std::map<unsigned int, MappedFrameBuffer> mappedBuffers_;
 
 	ControlInfoMap sensorControls_;
+	std::optional<ControlInfoMap> lensControls_;
 
 	/* revision-specific data */
 	rkisp1_cif_isp_version hwRevision_;
@@ -215,6 +217,9 @@  int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,
 {
 	sensorControls_ = ipaConfig.sensorControls;
 
+	if (!ipaConfig.lensControls.empty())
+		lensControls_ = ipaConfig.lensControls;
+
 	const auto itExp = sensorControls_.find(V4L2_CID_EXPOSURE);
 	int32_t minExposure = itExp->second.min().get<int32_t>();
 	int32_t maxExposure = itExp->second.max().get<int32_t>();
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 5bdb4d25..0559d261 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -31,6 +31,7 @@ 
 #include <libcamera/ipa/rkisp1_ipa_proxy.h>
 
 #include "libcamera/internal/camera.h"
+#include "libcamera/internal/camera_lens.h"
 #include "libcamera/internal/camera_sensor.h"
 #include "libcamera/internal/delayed_controls.h"
 #include "libcamera/internal/device_enumerator.h"
@@ -807,6 +808,10 @@  int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)
 
 	ipaConfig.sensorControls = data->sensor_->controls();
 
+	CameraLens *lens = data->sensor_->focusLens();
+	if (lens)
+		ipaConfig.lensControls = lens->controls();
+
 	ret = data->ipa_->configure(ipaConfig, streamConfig, &data->controlInfo_);
 	if (ret) {
 		LOG(RkISP1, Error) << "failed configuring IPA (" << ret << ")";