| Message ID | 20220316093951.33779-1-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-16 09:39:50) > The lens focus is controled by a VCM, which is linked to the sensor > using the ancillary links. > Pass the control to the config info structure and make it possible to > update by the IPA. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > include/libcamera/ipa/raspberrypi.mojom | 1 + > .../pipeline/raspberrypi/raspberrypi.cpp | 15 +++++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom > index acd3cafe..0c3922b0 100644 > --- a/include/libcamera/ipa/raspberrypi.mojom > +++ b/include/libcamera/ipa/raspberrypi.mojom > @@ -125,4 +125,5 @@ interface IPARPiEventInterface { > embeddedComplete(uint32 bufferId); > setIspControls(libcamera.ControlList controls); > setDelayedControls(libcamera.ControlList controls); > + setLensControls(libcamera.ControlList controls); > }; > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index c2230199..67a98daa 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -33,6 +33,7 @@ > > #include "libcamera/internal/bayer_format.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" > @@ -202,6 +203,7 @@ public: > void setIspControls(const ControlList &controls); > void setDelayedControls(const ControlList &controls); > void setSensorControls(ControlList &controls); > + void setLensControls(const ControlList &controls); > > /* bufferComplete signal handlers. */ > void unicamBufferDequeue(FrameBuffer *buffer); > @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig) > ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete); > ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls); > ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls); > + ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls); > > /* > * The configuration (tuning file) is made from the sensor name unless > @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) > entityControls.emplace(0, sensor_->controls()); > entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > + CameraLens *lens = sensor_->focusLens(); > + if (lens) > + entityControls.emplace(2, lens->controls()); What's 2 here, I suspect it's maybe something related to context that isn't shown in the diff - but it's not clear here. > + > /* Always send the user transform to the IPA. */ > ipaConfig.transform = static_cast<unsigned int>(config->transform); > > @@ -1735,6 +1742,14 @@ void RPiCameraData::setDelayedControls(const ControlList &controls) > handleState(); > } > > +void RPiCameraData::setLensControls(const ControlList &controls) > +{ > + ControlList ctrls = controls; > + > + sensor_->focusLens()->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>()); Doesn't this need to be guarded by an if (lens)? > + handleState(); > +} > + > void RPiCameraData::setSensorControls(ControlList &controls) > { > /* > -- > 2.32.0 >
On 16/03/2022 11:11, Kieran Bingham wrote: > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-16 09:39:50) >> The lens focus is controled by a VCM, which is linked to the sensor >> using the ancillary links. >> Pass the control to the config info structure and make it possible to >> update by the IPA. >> >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> include/libcamera/ipa/raspberrypi.mojom | 1 + >> .../pipeline/raspberrypi/raspberrypi.cpp | 15 +++++++++++++++ >> 2 files changed, 16 insertions(+) >> >> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom >> index acd3cafe..0c3922b0 100644 >> --- a/include/libcamera/ipa/raspberrypi.mojom >> +++ b/include/libcamera/ipa/raspberrypi.mojom >> @@ -125,4 +125,5 @@ interface IPARPiEventInterface { >> embeddedComplete(uint32 bufferId); >> setIspControls(libcamera.ControlList controls); >> setDelayedControls(libcamera.ControlList controls); >> + setLensControls(libcamera.ControlList controls); >> }; >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> index c2230199..67a98daa 100644 >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp >> @@ -33,6 +33,7 @@ >> >> #include "libcamera/internal/bayer_format.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" >> @@ -202,6 +203,7 @@ public: >> void setIspControls(const ControlList &controls); >> void setDelayedControls(const ControlList &controls); >> void setSensorControls(ControlList &controls); >> + void setLensControls(const ControlList &controls); >> >> /* bufferComplete signal handlers. */ >> void unicamBufferDequeue(FrameBuffer *buffer); >> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig) >> ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete); >> ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls); >> ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls); >> + ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls); >> >> /* >> * The configuration (tuning file) is made from the sensor name unless >> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) >> entityControls.emplace(0, sensor_->controls()); >> entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); >> >> + CameraLens *lens = sensor_->focusLens(); >> + if (lens) >> + entityControls.emplace(2, lens->controls()); > > What's 2 here, I suspect it's maybe something related to context that > isn't shown in the diff - but it's not clear here. It is after 1 :-) ? More seriously, the first controls are emplaced into entityControls at 0 and 1. So, we add another one. > > >> + >> /* Always send the user transform to the IPA. */ >> ipaConfig.transform = static_cast<unsigned int>(config->transform); >> >> @@ -1735,6 +1742,14 @@ void RPiCameraData::setDelayedControls(const ControlList &controls) >> handleState(); >> } >> >> +void RPiCameraData::setLensControls(const ControlList &controls) >> +{ >> + ControlList ctrls = controls; >> + >> + sensor_->focusLens()->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>()); > > Doesn't this need to be guarded by an if (lens)? Mmmh, probably yes, will do the change, thanks ! > >> + handleState(); >> +} >> + >> void RPiCameraData::setSensorControls(ControlList &controls) >> { >> /* >> -- >> 2.32.0 >>
Hi Jean-Michel, Thank you for your patch. On Wed, 16 Mar 2022 at 09:39, Jean-Michel Hautbois via libcamera-devel < libcamera-devel@lists.libcamera.org> wrote: > The lens focus is controled by a VCM, which is linked to the sensor > using the ancillary links. > Pass the control to the config info structure and make it possible to > update by the IPA. > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > include/libcamera/ipa/raspberrypi.mojom | 1 + > .../pipeline/raspberrypi/raspberrypi.cpp | 15 +++++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/include/libcamera/ipa/raspberrypi.mojom > b/include/libcamera/ipa/raspberrypi.mojom > index acd3cafe..0c3922b0 100644 > --- a/include/libcamera/ipa/raspberrypi.mojom > +++ b/include/libcamera/ipa/raspberrypi.mojom > @@ -125,4 +125,5 @@ interface IPARPiEventInterface { > embeddedComplete(uint32 bufferId); > setIspControls(libcamera.ControlList controls); > setDelayedControls(libcamera.ControlList controls); > + setLensControls(libcamera.ControlList controls); > }; > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index c2230199..67a98daa 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -33,6 +33,7 @@ > > #include "libcamera/internal/bayer_format.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" > @@ -202,6 +203,7 @@ public: > void setIspControls(const ControlList &controls); > void setDelayedControls(const ControlList &controls); > void setSensorControls(ControlList &controls); > + void setLensControls(const ControlList &controls); > > /* bufferComplete signal handlers. */ > void unicamBufferDequeue(FrameBuffer *buffer); > @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig > *sensorConfig) > ipa_->embeddedComplete.connect(this, > &RPiCameraData::embeddedComplete); > ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls); > ipa_->setDelayedControls.connect(this, > &RPiCameraData::setDelayedControls); > + ipa_->setLensControls.connect(this, > &RPiCameraData::setLensControls); > > /* > * The configuration (tuning file) is made from the sensor name > unless > @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const > CameraConfiguration *config) > entityControls.emplace(0, sensor_->controls()); > entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); > > + CameraLens *lens = sensor_->focusLens(); > + if (lens) > + entityControls.emplace(2, lens->controls()); > + > /* Always send the user transform to the IPA. */ > ipaConfig.transform = static_cast<unsigned int>(config->transform); > > @@ -1735,6 +1742,14 @@ void RPiCameraData::setDelayedControls(const > ControlList &controls) > handleState(); > } > > +void RPiCameraData::setLensControls(const ControlList &controls) > +{ > + ControlList ctrls = controls; > + > + > sensor_->focusLens()->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>()); > + handleState(); > +} > Do you need an if (lens_) test before the setFocusPosition()? The call to handleState() is not necessary here, I would remove it. Regards, Naush > + > void RPiCameraData::setSensorControls(ControlList &controls) > { > /* > -- > 2.32.0 > >
diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index acd3cafe..0c3922b0 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -125,4 +125,5 @@ interface IPARPiEventInterface { embeddedComplete(uint32 bufferId); setIspControls(libcamera.ControlList controls); setDelayedControls(libcamera.ControlList controls); + setLensControls(libcamera.ControlList controls); }; diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index c2230199..67a98daa 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -33,6 +33,7 @@ #include "libcamera/internal/bayer_format.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" @@ -202,6 +203,7 @@ public: void setIspControls(const ControlList &controls); void setDelayedControls(const ControlList &controls); void setSensorControls(ControlList &controls); + void setLensControls(const ControlList &controls); /* bufferComplete signal handlers. */ void unicamBufferDequeue(FrameBuffer *buffer); @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig) ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete); ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls); ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls); + ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls); /* * The configuration (tuning file) is made from the sensor name unless @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config) entityControls.emplace(0, sensor_->controls()); entityControls.emplace(1, isp_[Isp::Input].dev()->controls()); + CameraLens *lens = sensor_->focusLens(); + if (lens) + entityControls.emplace(2, lens->controls()); + /* Always send the user transform to the IPA. */ ipaConfig.transform = static_cast<unsigned int>(config->transform); @@ -1735,6 +1742,14 @@ void RPiCameraData::setDelayedControls(const ControlList &controls) handleState(); } +void RPiCameraData::setLensControls(const ControlList &controls) +{ + ControlList ctrls = controls; + + sensor_->focusLens()->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>()); + handleState(); +} + void RPiCameraData::setSensorControls(ControlList &controls) { /*
The lens focus is controled by a VCM, which is linked to the sensor using the ancillary links. Pass the control to the config info structure and make it possible to update by the IPA. Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> --- include/libcamera/ipa/raspberrypi.mojom | 1 + .../pipeline/raspberrypi/raspberrypi.cpp | 15 +++++++++++++++ 2 files changed, 16 insertions(+)