| Message ID | 20211029115917.2467936-2-hanlinchen@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Han-Lin, On 29/10/2021 13:59, Han-Lin Chen wrote: > The Intel close sourced IPA requires the effective controls applied to > the sensor when the stastistics are generated. Report effective sensor controls > with the stastistics to IPA. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > --- > src/libcamera/pipeline/ipu3/frames.h | 3 +++ > src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++ > 2 files changed, 8 insertions(+) > > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h > index 3ef7e445..a897e307 100644 > --- a/src/libcamera/pipeline/ipu3/frames.h > +++ b/src/libcamera/pipeline/ipu3/frames.h > @@ -12,6 +12,7 @@ > #include <queue> > #include <vector> > > +#include <libcamera/controls.h> > #include <libcamera/base/signal.h> > > namespace libcamera { > @@ -34,6 +35,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..6a7f5b9a 100644 > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > return ret; > } > > + data->delayedCtrls_->reset(); > + Is it a separate bug maybe ? One patch for this single line ;-) ? > return updateControls(data); > } > > @@ -1363,6 +1365,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 +1423,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; It means the IPA needs to parse the controls in its "EventStatReady" event parsing. Fine for me, even if we expected in open IPA to use the "EventProcessControls" event for that, using a queue to store the controls per frame. > ipa_->processEvent(ev); > } > >
Hi Jean-Michel, On Thu, Nov 4, 2021 at 7:45 PM Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com> wrote: > > Hi Han-Lin, > > On 29/10/2021 13:59, Han-Lin Chen wrote: > > The Intel close sourced IPA requires the effective controls applied to > > the sensor when the stastistics are generated. Report effective sensor controls > > with the stastistics to IPA. > > > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> > > --- > > src/libcamera/pipeline/ipu3/frames.h | 3 +++ > > src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++ > > 2 files changed, 8 insertions(+) > > > > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h > > index 3ef7e445..a897e307 100644 > > --- a/src/libcamera/pipeline/ipu3/frames.h > > +++ b/src/libcamera/pipeline/ipu3/frames.h > > @@ -12,6 +12,7 @@ > > #include <queue> > > #include <vector> > > > > +#include <libcamera/controls.h> > > #include <libcamera/base/signal.h> > > > > namespace libcamera { > > @@ -34,6 +35,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..6a7f5b9a 100644 > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp > > @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) > > return ret; > > } > > > > + data->delayedCtrls_->reset(); > > + > > Is it a separate bug maybe ? One patch for this single line ;-) ? > > > return updateControls(data); > > } > > > > @@ -1363,6 +1365,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 +1423,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; > > It means the IPA needs to parse the controls in its "EventStatReady" > event parsing. Fine for me, even if we expected in open IPA to use the > "EventProcessControls" event for that, using a queue to store the > controls per frame. It seems that they have different meanings, but only differentiated by the context. 1. I suppose the controls in "EventProcessControls" are the Controls for per-frame control from application, for example, AF pre-catpure or manual NR controls? 2. On the other hand, the effective sensor controls for "EventStatReady" means the exact controls which are effective on the sensor when the raw frame, and hence the statistics, is generated. The series needs that because the close IPA is sensitive to it. Not sure if it's worth changing the interface to make it clearer. > > > ipa_->processEvent(ev); > > } > > > >
diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h index 3ef7e445..a897e307 100644 --- a/src/libcamera/pipeline/ipu3/frames.h +++ b/src/libcamera/pipeline/ipu3/frames.h @@ -12,6 +12,7 @@ #include <queue> #include <vector> +#include <libcamera/controls.h> #include <libcamera/base/signal.h> namespace libcamera { @@ -34,6 +35,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..6a7f5b9a 100644 --- a/src/libcamera/pipeline/ipu3/ipu3.cpp +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp @@ -667,6 +667,8 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c) return ret; } + data->delayedCtrls_->reset(); + return updateControls(data); } @@ -1363,6 +1365,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 +1423,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); }
The Intel close sourced IPA requires the effective controls applied to the sensor when the stastistics are generated. Report effective sensor controls with the stastistics to IPA. Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> --- src/libcamera/pipeline/ipu3/frames.h | 3 +++ src/libcamera/pipeline/ipu3/ipu3.cpp | 5 +++++ 2 files changed, 8 insertions(+)