| Message ID | 20211110195901.85597-3-jeanmichel.hautbois@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Jean-Michel Hautbois (2021-11-10 19:58:49) > From: Han-Lin Chen <hanlinchen@chromium.org> > > The Intel close sourced IPA requires the effective controls applied to > the sensor when the statistics are generated. Report effective sensor controls > with the statistics to IPA. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > [Jean-Michel: Reword s/stastistics/statistics and move reset after IPA > start] > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> > --- > src/libcamera/pipeline/ipu3/frames.h | 4 ++++ > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h > index 3ef7e445..614701e2 100644 > --- a/src/libcamera/pipeline/ipu3/frames.h > +++ b/src/libcamera/pipeline/ipu3/frames.h > @@ -14,6 +14,8 @@ > > #include <libcamera/base/signal.h> > > +#include <libcamera/controls.h> > + > namespace libcamera { > > class FrameBuffer; > @@ -34,6 +36,8 @@ public: > FrameBuffer *paramBuffer; > FrameBuffer *statBuffer; > > + ControlList effectiveSensorControls; > + > bool paramDequeued; > bool metadataProcessed; > }; > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp > index 8816efc5..3fcfa777 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -767,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis > if (ret) > goto error; > > + /* > + * Reset the delayed controls with the gain and exposure values set by > + * the IPA. This seems to imply that resetting it is setting some values. Is that accurate? Looking at DelayedControls::reset() - it reads the controls from the device. Has the IPA already 'set' some controls before this point (not through the DelayedControls...) Other than that, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > + */ > + data->delayedCtrls_->reset(); > + > /* > * Start the ImgU video devices, buffers will be queued to the > * ImgU output and viewfinder when requests will be queued. > @@ -1363,6 +1369,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) > request->metadata().set(controls::SensorTimestamp, > buffer->metadata().timestamp); > > + info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); > + > if (request->findBuffer(&rawStream_)) > pipe()->completeBuffer(request, buffer); > > @@ -1419,6 +1427,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer) > ev.frame = info->id; > ev.bufferId = info->statBuffer->cookie(); > ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp); > + ev.sensorControls = info->effectiveSensorControls; > ipa_->processEvent(ev); > } > > -- > 2.32.0 >
On 10/11/2021 22:53, Kieran Bingham wrote: > Quoting Jean-Michel Hautbois (2021-11-10 19:58:49) >> From: Han-Lin Chen <hanlinchen@chromium.org> >> >> The Intel close sourced IPA requires the effective controls applied to >> the sensor when the statistics are generated. Report effective sensor controls >> with the statistics to IPA. >> >> Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> >> [Jean-Michel: Reword s/stastistics/statistics and move reset after IPA >> start] >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> >> --- >> src/libcamera/pipeline/ipu3/frames.h | 4 ++++ >> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++ >> 2 files changed, 13 insertions(+) >> >> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h >> index 3ef7e445..614701e2 100644 >> --- a/src/libcamera/pipeline/ipu3/frames.h >> +++ b/src/libcamera/pipeline/ipu3/frames.h >> @@ -14,6 +14,8 @@ >> >> #include <libcamera/base/signal.h> >> >> +#include <libcamera/controls.h> >> + >> namespace libcamera { >> >> class FrameBuffer; >> @@ -34,6 +36,8 @@ public: >> FrameBuffer *paramBuffer; >> FrameBuffer *statBuffer; >> >> + ControlList effectiveSensorControls; >> + >> bool paramDequeued; >> bool metadataProcessed; >> }; >> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp >> index 8816efc5..3fcfa777 100644 >> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp >> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp >> @@ -767,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis >> if (ret) >> goto error; >> >> + /* >> + * Reset the delayed controls with the gain and exposure values set by >> + * the IPA. > > This seems to imply that resetting it is setting some values. Is that > accurate? > > Looking at DelayedControls::reset() - it reads the controls from the > device. Has the IPA already 'set' some controls before this point (not > through the DelayedControls...) > Not yet, except HFLIP and VFLIP controls, which are not useful for us in this ? Maybe some later ? I must say this is what RPi is doing too :-). > Other than that, > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> + */ >> + data->delayedCtrls_->reset(); >> + >> /* >> * Start the ImgU video devices, buffers will be queued to the >> * ImgU output and viewfinder when requests will be queued. >> @@ -1363,6 +1369,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) >> request->metadata().set(controls::SensorTimestamp, >> buffer->metadata().timestamp); >> >> + info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); >> + >> if (request->findBuffer(&rawStream_)) >> pipe()->completeBuffer(request, buffer); >> >> @@ -1419,6 +1427,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer) >> ev.frame = info->id; >> ev.bufferId = info->statBuffer->cookie(); >> ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp); >> + ev.sensorControls = info->effectiveSensorControls; >> ipa_->processEvent(ev); >> } >> >> -- >> 2.32.0 >>
diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h index 3ef7e445..614701e2 100644 --- a/src/libcamera/pipeline/ipu3/frames.h +++ b/src/libcamera/pipeline/ipu3/frames.h @@ -14,6 +14,8 @@ #include <libcamera/base/signal.h> +#include <libcamera/controls.h> + namespace libcamera { class FrameBuffer; @@ -34,6 +36,8 @@ public: FrameBuffer *paramBuffer; FrameBuffer *statBuffer; + ControlList effectiveSensorControls; + bool paramDequeued; bool metadataProcessed; }; diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp index 8816efc5..3fcfa777 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -767,6 +767,12 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis if (ret) goto error; + /* + * Reset the delayed controls with the gain and exposure values set by + * the IPA. + */ + data->delayedCtrls_->reset(); + /* * Start the ImgU video devices, buffers will be queued to the * ImgU output and viewfinder when requests will be queued. @@ -1363,6 +1369,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer) request->metadata().set(controls::SensorTimestamp, buffer->metadata().timestamp); + info->effectiveSensorControls = delayedCtrls_->get(buffer->metadata().sequence); + if (request->findBuffer(&rawStream_)) pipe()->completeBuffer(request, buffer); @@ -1419,6 +1427,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer) ev.frame = info->id; ev.bufferId = info->statBuffer->cookie(); ev.frameTimestamp = request->metadata().get(controls::SensorTimestamp); + ev.sensorControls = info->effectiveSensorControls; ipa_->processEvent(ev); }