| Message ID | 20240703175119.1872585-12-mzamazal@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Milan, On 03/07/24 11:21 pm, Milan Zamazal wrote: > This patch adds Algorithm::queueRequest call for the defined algorithms. > This is preparation only since there are currently no Algorithm based > algorithms defined. I think the patch targets adding queueRequest() call to soft ISP and softIPA and Algorithms here. So this has to be reworded to reflect that. > Signed-off-by: Milan Zamazal <mzamazal@redhat.com> > --- > include/libcamera/internal/software_isp/software_isp.h | 1 + > include/libcamera/ipa/soft.mojom | 1 + > src/ipa/simple/soft_simple.cpp | 9 +++++++++ > src/libcamera/pipeline/simple/simple.cpp | 2 ++ > src/libcamera/software_isp/software_isp.cpp | 10 ++++++++++ > 5 files changed, 23 insertions(+) > > diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h > index ff26b1d4..1ba9eb62 100644 > --- a/include/libcamera/internal/software_isp/software_isp.h > +++ b/include/libcamera/internal/software_isp/software_isp.h > @@ -72,6 +72,7 @@ public: > int start(); > void stop(); > > + void queueRequest(const uint32_t frame, const ControlList &controls); > int queueBuffers(uint32_t frame, FrameBuffer *input, > const std::map<unsigned int, FrameBuffer *> &outputs); > > diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom > index 5e124016..426fee18 100644 > --- a/include/libcamera/ipa/soft.mojom > +++ b/include/libcamera/ipa/soft.mojom > @@ -23,6 +23,7 @@ interface IPASoftInterface { > configure(IPAConfigInfo configInfo) > => (int32 ret); > > + queueRequest(uint32 frame, libcamera.ControlList sensorControls); Usually this is [async] , but I suppose that's not the case with Soft ISP here? AS this is only queuing later on to Algorithms, I guess it's fine for now. Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > [async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls); > }; > > diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp > index 49fff312..bd0dea5f 100644 > --- a/src/ipa/simple/soft_simple.cpp > +++ b/src/ipa/simple/soft_simple.cpp > @@ -77,6 +77,7 @@ public: > int start() override; > void stop() override; > > + void queueRequest(const uint32_t frame, const ControlList &controls) override; > void processStats(const uint32_t frame, const uint32_t bufferId, > const ControlList &sensorControls) override; > > @@ -269,6 +270,14 @@ void IPASoftSimple::stop() > { > } > > +void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls) > +{ > + IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); > + > + for (auto const &algo : algorithms()) > + algo->queueRequest(context_, frame, frameContext, controls); > +} > + > void IPASoftSimple::processStats( > [[maybe_unused]] const uint32_t frame, > [[maybe_unused]] const uint32_t bufferId, > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index e0c9fe5c..6fb80209 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -1430,6 +1430,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) > if (data->useConversion_) > data->conversionQueue_.push(std::move(buffers)); > > + data->swIsp_->queueRequest(request->sequence(), request->controls()); > + > return 0; > } > > diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp > index 2bbb86da..4e6b3a2e 100644 > --- a/src/libcamera/software_isp/software_isp.cpp > +++ b/src/libcamera/software_isp/software_isp.cpp > @@ -279,6 +279,16 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count, > return count; > } > > +/** > + * \brief Queue a request and process the control list from the application > + * \param[in] frame The number of the frame which will be processed next > + * \param[in] controls The controls for the \a frame > + */ > +void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls) > +{ > + ipa_->queueRequest(frame, controls); > +} > + > /** > * \brief Queue buffers to Software ISP > * \param[in] frame The frame number
Hi Umang, thank you for review. Umang Jain <umang.jain@ideasonboard.com> writes: > Hi Milan, > > > On 03/07/24 11:21 pm, Milan Zamazal wrote: >> This patch adds Algorithm::queueRequest call for the defined algorithms. >> This is preparation only since there are currently no Algorithm based >> algorithms defined. > > I think the patch targets adding queueRequest() call to soft ISP and softIPA and Algorithms here. > > So this has to be reworded to reflect that. OK, will do. >> Signed-off-by: Milan Zamazal <mzamazal@redhat.com> >> --- >> include/libcamera/internal/software_isp/software_isp.h | 1 + >> include/libcamera/ipa/soft.mojom | 1 + >> src/ipa/simple/soft_simple.cpp | 9 +++++++++ >> src/libcamera/pipeline/simple/simple.cpp | 2 ++ >> src/libcamera/software_isp/software_isp.cpp | 10 ++++++++++ >> 5 files changed, 23 insertions(+) >> >> diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h >> index ff26b1d4..1ba9eb62 100644 >> --- a/include/libcamera/internal/software_isp/software_isp.h >> +++ b/include/libcamera/internal/software_isp/software_isp.h >> @@ -72,6 +72,7 @@ public: >> int start(); >> void stop(); >> + void queueRequest(const uint32_t frame, const ControlList &controls); >> int queueBuffers(uint32_t frame, FrameBuffer *input, >> const std::map<unsigned int, FrameBuffer *> &outputs); >> diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom >> index 5e124016..426fee18 100644 >> --- a/include/libcamera/ipa/soft.mojom >> +++ b/include/libcamera/ipa/soft.mojom >> @@ -23,6 +23,7 @@ interface IPASoftInterface { >> configure(IPAConfigInfo configInfo) >> => (int32 ret); >> + queueRequest(uint32 frame, libcamera.ControlList sensorControls); > > Usually this is [async] , but I suppose that's not the case with Soft ISP here? I'm not sure. I don't know what's the reason to make it async as it looks like the algorithms just look at control values and assign their counterparts; maybe because it is called at a sensitive moment? I'll change it to async to be safe and consistent. > AS this is only queuing later on to Algorithms, I guess it's fine for now. > > Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> >> [async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls); >> }; >> diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp >> index 49fff312..bd0dea5f 100644 >> --- a/src/ipa/simple/soft_simple.cpp >> +++ b/src/ipa/simple/soft_simple.cpp >> @@ -77,6 +77,7 @@ public: >> int start() override; >> void stop() override; >> + void queueRequest(const uint32_t frame, const ControlList &controls) override; >> void processStats(const uint32_t frame, const uint32_t bufferId, >> const ControlList &sensorControls) override; >> @@ -269,6 +270,14 @@ void IPASoftSimple::stop() >> { >> } >> +void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls) >> +{ >> + IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); >> + >> + for (auto const &algo : algorithms()) >> + algo->queueRequest(context_, frame, frameContext, controls); >> +} >> + >> void IPASoftSimple::processStats( >> [[maybe_unused]] const uint32_t frame, >> [[maybe_unused]] const uint32_t bufferId, >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index e0c9fe5c..6fb80209 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -1430,6 +1430,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) >> if (data->useConversion_) >> data->conversionQueue_.push(std::move(buffers)); >> + data->swIsp_->queueRequest(request->sequence(), request->controls()); >> + >> return 0; >> } >> diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp >> index 2bbb86da..4e6b3a2e 100644 >> --- a/src/libcamera/software_isp/software_isp.cpp >> +++ b/src/libcamera/software_isp/software_isp.cpp >> @@ -279,6 +279,16 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count, >> return count; >> } >> +/** >> + * \brief Queue a request and process the control list from the application >> + * \param[in] frame The number of the frame which will be processed next >> + * \param[in] controls The controls for the \a frame >> + */ >> +void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls) >> +{ >> + ipa_->queueRequest(frame, controls); >> +} >> + >> /** >> * \brief Queue buffers to Software ISP >> * \param[in] frame The frame number
diff --git a/include/libcamera/internal/software_isp/software_isp.h b/include/libcamera/internal/software_isp/software_isp.h index ff26b1d4..1ba9eb62 100644 --- a/include/libcamera/internal/software_isp/software_isp.h +++ b/include/libcamera/internal/software_isp/software_isp.h @@ -72,6 +72,7 @@ public: int start(); void stop(); + void queueRequest(const uint32_t frame, const ControlList &controls); int queueBuffers(uint32_t frame, FrameBuffer *input, const std::map<unsigned int, FrameBuffer *> &outputs); diff --git a/include/libcamera/ipa/soft.mojom b/include/libcamera/ipa/soft.mojom index 5e124016..426fee18 100644 --- a/include/libcamera/ipa/soft.mojom +++ b/include/libcamera/ipa/soft.mojom @@ -23,6 +23,7 @@ interface IPASoftInterface { configure(IPAConfigInfo configInfo) => (int32 ret); + queueRequest(uint32 frame, libcamera.ControlList sensorControls); [async] processStats(uint32 frame, uint32 bufferId, libcamera.ControlList sensorControls); }; diff --git a/src/ipa/simple/soft_simple.cpp b/src/ipa/simple/soft_simple.cpp index 49fff312..bd0dea5f 100644 --- a/src/ipa/simple/soft_simple.cpp +++ b/src/ipa/simple/soft_simple.cpp @@ -77,6 +77,7 @@ public: int start() override; void stop() override; + void queueRequest(const uint32_t frame, const ControlList &controls) override; void processStats(const uint32_t frame, const uint32_t bufferId, const ControlList &sensorControls) override; @@ -269,6 +270,14 @@ void IPASoftSimple::stop() { } +void IPASoftSimple::queueRequest(const uint32_t frame, const ControlList &controls) +{ + IPAFrameContext &frameContext = context_.frameContexts.alloc(frame); + + for (auto const &algo : algorithms()) + algo->queueRequest(context_, frame, frameContext, controls); +} + void IPASoftSimple::processStats( [[maybe_unused]] const uint32_t frame, [[maybe_unused]] const uint32_t bufferId, diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index e0c9fe5c..6fb80209 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -1430,6 +1430,8 @@ int SimplePipelineHandler::queueRequestDevice(Camera *camera, Request *request) if (data->useConversion_) data->conversionQueue_.push(std::move(buffers)); + data->swIsp_->queueRequest(request->sequence(), request->controls()); + return 0; } diff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp index 2bbb86da..4e6b3a2e 100644 --- a/src/libcamera/software_isp/software_isp.cpp +++ b/src/libcamera/software_isp/software_isp.cpp @@ -279,6 +279,16 @@ int SoftwareIsp::exportBuffers(unsigned int output, unsigned int count, return count; } +/** + * \brief Queue a request and process the control list from the application + * \param[in] frame The number of the frame which will be processed next + * \param[in] controls The controls for the \a frame + */ +void SoftwareIsp::queueRequest(const uint32_t frame, const ControlList &controls) +{ + ipa_->queueRequest(frame, controls); +} + /** * \brief Queue buffers to Software ISP * \param[in] frame The frame number
This patch adds Algorithm::queueRequest call for the defined algorithms. This is preparation only since there are currently no Algorithm based algorithms defined. Signed-off-by: Milan Zamazal <mzamazal@redhat.com> --- include/libcamera/internal/software_isp/software_isp.h | 1 + include/libcamera/ipa/soft.mojom | 1 + src/ipa/simple/soft_simple.cpp | 9 +++++++++ src/libcamera/pipeline/simple/simple.cpp | 2 ++ src/libcamera/software_isp/software_isp.cpp | 10 ++++++++++ 5 files changed, 23 insertions(+)