| Message ID | 20220908014200.28728-12-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Laurent Pinchart via libcamera-devel (2022-09-08 02:41:39) > The the Algorithm::queueRequest() function of all algorithms when a Call the ? > request is queue, to pass the request controls to the algorithms. We can /queue/queued/ > now drop the copy of the control list stored in IPAFrameContext as it > isn't used anymore. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/ipu3/ipa_context.cpp | 3 --- > src/ipa/ipu3/ipa_context.h | 3 --- > src/ipa/ipu3/ipu3.cpp | 4 ++-- > 3 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index 6904ccbbdf8b..bd71b615365d 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -168,9 +168,6 @@ namespace libcamera::ipa::ipu3 { > * \struct IPAFrameContext > * \brief IPU3-specific FrameContext > * > - * \var IPAFrameContext::frameControls > - * \brief Controls sent in by the application while queuing the request > - * > * \var IPAFrameContext::sensor > * \brief Effective sensor values that were applied for the frame > * > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index bfc0196e098a..36099353e9f2 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -12,7 +12,6 @@ > > #include <libcamera/base/utils.h> > > -#include <libcamera/controls.h> > #include <libcamera/geometry.h> > > #include <libipa/fc_queue.h> > @@ -78,8 +77,6 @@ struct IPAFrameContext : public FrameContext { > uint32_t exposure; > double gain; > } sensor; > - > - ControlList frameControls; > }; > > struct IPAContext { > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 8158ca0883e8..844ab6de03c7 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -666,8 +666,8 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) > /* \todo Start processing for 'frame' based on 'controls'. */ I think we can drop this comment here too. It's now a todo specific to each algorithm. With that, Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > IPAFrameContext &frameContext = context_.frameContexts.init(frame); > > - /* \todo Implement queueRequest to each algorithm. */ > - frameContext.frameControls = controls; > + for (auto const &algo : algorithms()) > + algo->queueRequest(context_, frame, frameContext, controls); > } > > /** > -- > Regards, > > Laurent Pinchart >
Hi Laurent, On Thu, Sep 08, 2022 at 04:41:39AM +0300, Laurent Pinchart via libcamera-devel wrote: > The the Algorithm::queueRequest() function of all algorithms when a > request is queue, to pass the request controls to the algorithms. We can > now drop the copy of the control list stored in IPAFrameContext as it > isn't used anymore. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/ipa/ipu3/ipa_context.cpp | 3 --- > src/ipa/ipu3/ipa_context.h | 3 --- > src/ipa/ipu3/ipu3.cpp | 4 ++-- > 3 files changed, 2 insertions(+), 8 deletions(-) > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > index 6904ccbbdf8b..bd71b615365d 100644 > --- a/src/ipa/ipu3/ipa_context.cpp > +++ b/src/ipa/ipu3/ipa_context.cpp > @@ -168,9 +168,6 @@ namespace libcamera::ipa::ipu3 { > * \struct IPAFrameContext > * \brief IPU3-specific FrameContext > * > - * \var IPAFrameContext::frameControls > - * \brief Controls sent in by the application while queuing the request > - * > * \var IPAFrameContext::sensor > * \brief Effective sensor values that were applied for the frame > * > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > index bfc0196e098a..36099353e9f2 100644 > --- a/src/ipa/ipu3/ipa_context.h > +++ b/src/ipa/ipu3/ipa_context.h > @@ -12,7 +12,6 @@ > > #include <libcamera/base/utils.h> > > -#include <libcamera/controls.h> > #include <libcamera/geometry.h> > > #include <libipa/fc_queue.h> > @@ -78,8 +77,6 @@ struct IPAFrameContext : public FrameContext { > uint32_t exposure; > double gain; > } sensor; > - > - ControlList frameControls; Maybe I've missed a point, if the FrameContext does not even contain the controls associated with a Request, as they're passed to queueRequest() and used there, what is its purpose? Seems like a really stupid question, as our overall design is centered around th FrameContext... > }; > > struct IPAContext { > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > index 8158ca0883e8..844ab6de03c7 100644 > --- a/src/ipa/ipu3/ipu3.cpp > +++ b/src/ipa/ipu3/ipu3.cpp > @@ -666,8 +666,8 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) > /* \todo Start processing for 'frame' based on 'controls'. */ > IPAFrameContext &frameContext = context_.frameContexts.init(frame); > > - /* \todo Implement queueRequest to each algorithm. */ > - frameContext.frameControls = controls; > + for (auto const &algo : algorithms()) > + algo->queueRequest(context_, frame, frameContext, controls); > } > > /** > -- > Regards, > > Laurent Pinchart >
Self-reply to a dumb question On Wed, Sep 21, 2022 at 08:19:48PM +0200, Jacopo Mondi wrote: > Hi Laurent, > > On Thu, Sep 08, 2022 at 04:41:39AM +0300, Laurent Pinchart via libcamera-devel wrote: > > The the Algorithm::queueRequest() function of all algorithms when a > > request is queue, to pass the request controls to the algorithms. We can > > now drop the copy of the control list stored in IPAFrameContext as it > > isn't used anymore. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > src/ipa/ipu3/ipa_context.cpp | 3 --- > > src/ipa/ipu3/ipa_context.h | 3 --- > > src/ipa/ipu3/ipu3.cpp | 4 ++-- > > 3 files changed, 2 insertions(+), 8 deletions(-) > > > > diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp > > index 6904ccbbdf8b..bd71b615365d 100644 > > --- a/src/ipa/ipu3/ipa_context.cpp > > +++ b/src/ipa/ipu3/ipa_context.cpp > > @@ -168,9 +168,6 @@ namespace libcamera::ipa::ipu3 { > > * \struct IPAFrameContext > > * \brief IPU3-specific FrameContext > > * > > - * \var IPAFrameContext::frameControls > > - * \brief Controls sent in by the application while queuing the request > > - * > > * \var IPAFrameContext::sensor > > * \brief Effective sensor values that were applied for the frame > > * > > diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h > > index bfc0196e098a..36099353e9f2 100644 > > --- a/src/ipa/ipu3/ipa_context.h > > +++ b/src/ipa/ipu3/ipa_context.h > > @@ -12,7 +12,6 @@ > > > > #include <libcamera/base/utils.h> > > > > -#include <libcamera/controls.h> > > #include <libcamera/geometry.h> > > > > #include <libipa/fc_queue.h> > > @@ -78,8 +77,6 @@ struct IPAFrameContext : public FrameContext { > > uint32_t exposure; > > double gain; > > } sensor; > > - > > - ControlList frameControls; > > Maybe I've missed a point, if the FrameContext does not even contain > the controls associated with a Request, as they're passed to queueRequest() > and used there, what is its purpose? Seems like a really stupid question, as > our overall design is centered around th FrameContext... > .. the algorithm-computed per-frame results Sorry for the noise > > }; > > > > struct IPAContext { > > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp > > index 8158ca0883e8..844ab6de03c7 100644 > > --- a/src/ipa/ipu3/ipu3.cpp > > +++ b/src/ipa/ipu3/ipu3.cpp > > @@ -666,8 +666,8 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) > > /* \todo Start processing for 'frame' based on 'controls'. */ > > IPAFrameContext &frameContext = context_.frameContexts.init(frame); > > > > - /* \todo Implement queueRequest to each algorithm. */ > > - frameContext.frameControls = controls; > > + for (auto const &algo : algorithms()) > > + algo->queueRequest(context_, frame, frameContext, controls); > > } > > > > /** > > -- > > Regards, > > > > Laurent Pinchart > >
diff --git a/src/ipa/ipu3/ipa_context.cpp b/src/ipa/ipu3/ipa_context.cpp index 6904ccbbdf8b..bd71b615365d 100644 --- a/src/ipa/ipu3/ipa_context.cpp +++ b/src/ipa/ipu3/ipa_context.cpp @@ -168,9 +168,6 @@ namespace libcamera::ipa::ipu3 { * \struct IPAFrameContext * \brief IPU3-specific FrameContext * - * \var IPAFrameContext::frameControls - * \brief Controls sent in by the application while queuing the request - * * \var IPAFrameContext::sensor * \brief Effective sensor values that were applied for the frame * diff --git a/src/ipa/ipu3/ipa_context.h b/src/ipa/ipu3/ipa_context.h index bfc0196e098a..36099353e9f2 100644 --- a/src/ipa/ipu3/ipa_context.h +++ b/src/ipa/ipu3/ipa_context.h @@ -12,7 +12,6 @@ #include <libcamera/base/utils.h> -#include <libcamera/controls.h> #include <libcamera/geometry.h> #include <libipa/fc_queue.h> @@ -78,8 +77,6 @@ struct IPAFrameContext : public FrameContext { uint32_t exposure; double gain; } sensor; - - ControlList frameControls; }; struct IPAContext { diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp index 8158ca0883e8..844ab6de03c7 100644 --- a/src/ipa/ipu3/ipu3.cpp +++ b/src/ipa/ipu3/ipu3.cpp @@ -666,8 +666,8 @@ void IPAIPU3::queueRequest(const uint32_t frame, const ControlList &controls) /* \todo Start processing for 'frame' based on 'controls'. */ IPAFrameContext &frameContext = context_.frameContexts.init(frame); - /* \todo Implement queueRequest to each algorithm. */ - frameContext.frameControls = controls; + for (auto const &algo : algorithms()) + algo->queueRequest(context_, frame, frameContext, controls); } /**
The the Algorithm::queueRequest() function of all algorithms when a request is queue, to pass the request controls to the algorithms. We can now drop the copy of the control list stored in IPAFrameContext as it isn't used anymore. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/ipa/ipu3/ipa_context.cpp | 3 --- src/ipa/ipu3/ipa_context.h | 3 --- src/ipa/ipu3/ipu3.cpp | 4 ++-- 3 files changed, 2 insertions(+), 8 deletions(-)