[libcamera-devel] [PATCH 3/3] ipa: raspberrypi: Update sensor's V4L2_CID_NOTIFY_GAINS control when present
David Plowman
david.plowman at raspberrypi.com
Thu Dec 30 12:21:52 CET 2021
Hi Laurent
Thanks for the comments.
On Wed, 29 Dec 2021 at 23:53, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi David,
>
> Thank you for the patch.
>
> On Thu, Dec 23, 2021 at 08:01:10AM +0000, David Plowman wrote:
> > If the sensor exposes the V4L2_CID_NOTIFY_GAINS controls, assume it
> > wants to be told the colour gains.
> >
> > We want these to be applied as soon as possible so we add a new
> > setSensorControls signal to the IPA which passes these back to the
> > pipeline handler without using the DelayedControls mechanism.
> >
> > Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> > ---
> > include/libcamera/ipa/raspberrypi.mojom | 1 +
> > src/ipa/raspberrypi/raspberrypi.cpp | 22 +++++++++++++++++++
> > .../pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++
> > 3 files changed, 33 insertions(+)
> >
> > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom
> > index acd3cafe..e7647724 100644
> > --- a/include/libcamera/ipa/raspberrypi.mojom
> > +++ b/include/libcamera/ipa/raspberrypi.mojom
> > @@ -123,6 +123,7 @@ interface IPARPiEventInterface {
> > statsMetadataComplete(uint32 bufferId, libcamera.ControlList controls);
> > runIsp(uint32 bufferId);
> > embeddedComplete(uint32 bufferId);
> > + setSensorControls(libcamera.ControlList controls);
> > setIspControls(libcamera.ControlList controls);
> > setDelayedControls(libcamera.ControlList controls);
> > };
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 0ed41385..8c20c066 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -1061,6 +1061,28 @@ void IPARPi::processStats(unsigned int bufferId)
> >
> > setDelayedControls.emit(ctrls);
> > }
> > +
> > + auto itCtrl = sensorCtrls_.find(V4L2_CID_NOTIFY_GAINS);
> > + if (itCtrl != sensorCtrls_.end()) {
> > + struct AwbStatus awbStatus;
> > + if (rpiMetadata_.Get("awb.status", awbStatus) == 0) {
> > + /*
> > + * This control is a linear gain value where the default is
> > + * defined to correspond to a gain of 1. The order of the gains
> > + * is always B, Gb, Gr, R.
> > + */
> > + int unity = itCtrl->second.def().get<int32_t>();
> > + ControlList ctrls(sensorCtrls_);
> > + int32_t gains[4] = { static_cast<int32_t>(awbStatus.gain_b * unity),
> > + unity, unity,
> > + static_cast<int32_t>(awbStatus.gain_r * unity) };
> > + ControlValue c(Span<const uint8_t>{ reinterpret_cast<uint8_t *>(gains),
> > + sizeof(gains) });
> > + ctrls.set(V4L2_CID_NOTIFY_GAINS, c);
> > +
> > + setSensorControls.emit(ctrls);
>
> Could we avoid adding another operation by setting V4L2_CID_NOTIFY_GAINS
> in the pipeline handler, when receiving the request metadata in
> RPiCameraData::statsMetadataComplete() ?
That's an interesting thought. Let me check with Naush once he's back,
but I guess we could "intercept" the colour gain values there and
update those controls in much the same way.
>
> > + }
> > + }
> > }
> >
> > void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index b5c687da..59d3bbd8 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -196,6 +196,7 @@ public:
> > void statsMetadataComplete(uint32_t bufferId, const ControlList &controls);
> > void runIsp(uint32_t bufferId);
> > void embeddedComplete(uint32_t bufferId);
> > + void setSensorControls(const ControlList &controls);
> > void setIspControls(const ControlList &controls);
> > void setDelayedControls(const ControlList &controls);
> > void setSensorControls(ControlList &controls);
> > @@ -1405,6 +1406,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)
> > ipa_->statsMetadataComplete.connect(this, &RPiCameraData::statsMetadataComplete);
> > ipa_->runIsp.connect(this, &RPiCameraData::runIsp);
> > ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);
> > + ipa_->setSensorControls.connect(this, &RPiCameraData::setSensorControls);
> > ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);
> > ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);
> >
> > @@ -1524,6 +1526,14 @@ void RPiCameraData::embeddedComplete(uint32_t bufferId)
> > handleState();
> > }
> >
> > +void RPiCameraData::setSensorControls(const ControlList &controls)
> > +{
> > + ControlList ctrls = controls;
> > +
> > + sensor_->setControls(&ctrls);
> > + handleState();
>
> Do we need to call handleState() here ?
Probably not! But maybe this disappears altogether...
Thanks!
David
>
> > +}
> > +
> > void RPiCameraData::setIspControls(const ControlList &controls)
> > {
> > ControlList ctrls = controls;
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list