| Message ID | 20241013175719.187166-1-robert.mader@collabora.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Robert Subject can be : "libcamera: software_isp: Reset IPA context" On 13/10/24 11:27 pm, Robert Mader wrote: > Like the hardware pipelines do. Not clearing frameContexts otherwise can s/Like the hardware pipelines do/Similar to what hardware pipelines does, clear IPA context in configure() and stop()/ > trigger asserts like "Frame context for ... has been overwritten by ..." > when switching between cameras using the swISP, e.g. on phones. > > Clearing the configuration and active state will become more important > with upcoming changes such as getting the black level from the camera > helper. > > Fixes: 04d171e6 ("libcamera: software_isp: Call Algorithm::queueRequest") > Signed-off-by: Robert Mader <robert.mader@collabora.com> Minors commit message nits, can be applied during merge it self. Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > --- > > Changes in version 2: > - Also clear frameContexts as well as other context data on configure > - Change commit title and message accordingly, previous title: > "libcamera: software_isp: Clear frameContexts on Stop()" > --- > src/ipa/simple/soft_simple.cpp | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index b28c7039..3b25d7d0 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -184,6 +184,11 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) > const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second; > const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second; > > + /* Clear the IPA context before the streaming session. */ > + context_.configuration = {}; > + context_.activeState = {}; > + context_.frameContexts.clear(); > + > context_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>(); > context_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>(); > if (!context_.configuration.agc.exposureMin) { > @@ -245,6 +250,7 @@ int IPASoftSimple::start() > > void IPASoftSimple::stop() > { > + context_.frameContexts.clear(); > } > > void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
Hi Robert, thank you for the fix. Robert Mader <robert.mader@collabora.com> writes: > Like the hardware pipelines do. Not clearing frameContexts otherwise can > trigger asserts like "Frame context for ... has been overwritten by ..." > when switching between cameras using the swISP, e.g. on phones. > > Clearing the configuration and active state will become more important > with upcoming changes such as getting the black level from the camera > helper. > > Fixes: 04d171e6 ("libcamera: software_isp: Call Algorithm::queueRequest") > Signed-off-by: Robert Mader <robert.mader@collabora.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > > Changes in version 2: > - Also clear frameContexts as well as other context data on configure > - Change commit title and message accordingly, previous title: > "libcamera: software_isp: Clear frameContexts on Stop()" > --- > src/ipa/simple/soft_simple.cpp | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index b28c7039..3b25d7d0 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -184,6 +184,11 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) > const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second; > const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second; > > + /* Clear the IPA context before the streaming session. */ > + context_.configuration = {}; > + context_.activeState = {}; > + context_.frameContexts.clear(); > + > context_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>(); > context_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>(); > if (!context_.configuration.agc.exposureMin) { > @@ -245,6 +250,7 @@ int IPASoftSimple::start() > > void IPASoftSimple::stop() > { > + context_.frameContexts.clear(); > } > > void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index b28c7039..3b25d7d0 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -184,6 +184,11 @@ int IPASoftSimple::configure(const IPAConfigInfo &configInfo) const ControlInfo &exposureInfo = sensorInfoMap_.find(V4L2_CID_EXPOSURE)->second; const ControlInfo &gainInfo = sensorInfoMap_.find(V4L2_CID_ANALOGUE_GAIN)->second; + /* Clear the IPA context before the streaming session. */ + context_.configuration = {}; + context_.activeState = {}; + context_.frameContexts.clear(); + context_.configuration.agc.exposureMin = exposureInfo.min().get<int32_t>(); context_.configuration.agc.exposureMax = exposureInfo.max().get<int32_t>(); if (!context_.configuration.agc.exposureMin) { @@ -245,6 +250,7 @@ int IPASoftSimple::start() void IPASoftSimple::stop() { + context_.frameContexts.clear(); } void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls)
Like the hardware pipelines do. Not clearing frameContexts otherwise can trigger asserts like "Frame context for ... has been overwritten by ..." when switching between cameras using the swISP, e.g. on phones. Clearing the configuration and active state will become more important with upcoming changes such as getting the black level from the camera helper. Fixes: 04d171e6 ("libcamera: software_isp: Call Algorithm::queueRequest") Signed-off-by: Robert Mader <robert.mader@collabora.com> --- Changes in version 2: - Also clear frameContexts as well as other context data on configure - Change commit title and message accordingly, previous title: "libcamera: software_isp: Clear frameContexts on Stop()" --- src/ipa/simple/soft_simple.cpp | 6 ++++++ 1 file changed, 6 insertions(+)