[libcamera-devel,v3,5/9] ipu3: Update IPAIPU3Interface::fillParamsBuffer with captureBufferId
diff mbox series

Message ID 20220629103018.4025635-6-chenghaoyang@google.com
State Superseded
Delegated to: Umang Jain
Headers show
Series
  • Use two imgus in ipu3 pipeline handler
Related show

Commit Message

Harvey Yang June 29, 2022, 10:30 a.m. UTC 
From: Harvey Yang <chenghaoyang@chromium.org>

This patch updates the ipa interface |IPAIPU3Interface::fillParamsBuffer|
with additional |captureBufferId| to fill the param buffer for the
StillCapture stream as well.

Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
---
 include/libcamera/ipa/ipu3.mojom     |  2 +-
 src/ipa/ipu3/ipu3.cpp                | 21 ++++++++++++++++++---
 src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
 3 files changed, 21 insertions(+), 5 deletions(-)

Comments

Umang Jain July 27, 2022, 8:54 a.m. UTC | #1 
Hi Harvey,

On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> From: Harvey Yang <chenghaoyang@chromium.org>
>
> This patch updates the ipa interface |IPAIPU3Interface::fillParamsBuffer|
> with additional |captureBufferId| to fill the param buffer for the
> StillCapture stream as well.


Whenever the IPAIPU3 Interface is updated,

     https://git.libcamera.org/libcamera/ipu3-ipa.git/tree/ (closed 
source IPA IPU3 wrapper)

will need corresponding updates as well (Just mentioning for the records)

>
> Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> ---
>   include/libcamera/ipa/ipu3.mojom     |  2 +-
>   src/ipa/ipu3/ipu3.cpp                | 21 ++++++++++++++++++---
>   src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
>   3 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
> index d1b1c6b8..d94c344e 100644
> --- a/include/libcamera/ipa/ipu3.mojom
> +++ b/include/libcamera/ipa/ipu3.mojom
> @@ -31,7 +31,7 @@ interface IPAIPU3Interface {
>   	unmapBuffers(array<uint32> ids);
>   
>   	[async] queueRequest(uint32 frame, libcamera.ControlList controls);
> -	[async] fillParamsBuffer(uint32 frame, uint32 bufferId);
> +	[async] fillParamsBuffer(uint32 frame, uint32 bufferId, uint32 captureBufferId);
>   	[async] processStatsBuffer(uint32 frame, int64 frameTimestamp,
>   				   uint32 bufferId, libcamera.ControlList sensorControls);
>   };
> diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> index dd6cfd79..149a3958 100644
> --- a/src/ipa/ipu3/ipu3.cpp
> +++ b/src/ipa/ipu3/ipu3.cpp
> @@ -146,7 +146,8 @@ public:
>   	void unmapBuffers(const std::vector<unsigned int> &ids) override;
>   
>   	void queueRequest(const uint32_t frame, const ControlList &controls) override;
> -	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;
> +	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,
> +			      const uint32_t captureBufferId) override;
>   	void processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
>   				const uint32_t bufferId,
>   				const ControlList &sensorControls) override;
> @@ -508,12 +509,14 @@ void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
>   /**
>    * \brief Fill and return a buffer with ISP processing parameters for a frame
>    * \param[in] frame The frame number
> - * \param[in] bufferId ID of the parameter buffer to fill
> + * \param[in] bufferId ID of the video parameter buffer to fill
> + * \param[in] captureBufferId ID of the capture parameter buffer to fill
>    *
>    * Algorithms are expected to fill the IPU3 parameter buffer for the next
>    * frame given their most recent processing of the ImgU statistics.
>    */
> -void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
> +void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,


Since now there are couple of parameter buffers to fill, this can be 
renamed to depict plural buffers:

     IPAIPU3::fillParamsBuffers(const uint32_t frame, const uint32_t 
bufferId,

> +			       const uint32_t captureBufferId)
>   {
>   	auto it = buffers_.find(bufferId);
>   	if (it == buffers_.end()) {
> @@ -536,6 +539,18 @@ void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
>   	 */
>   	params->use = {};
>   
> +	for (auto const &algo : algorithms_)
> +		algo->prepare(context_, params);
> +
> +	// TODO: use different algorithms to set StillCapture params.


Can you provide a brief summary here, specific to algorithms targetting 
StillCapture use-case? How would they differ (and I wonder if they 
work/need statistics from Imgu1).

> +	it = buffers_.find(captureBufferId);
> +	if (it == buffers_.end()) {
> +		LOG(IPAIPU3, Error) << "Could not find capture param buffer!";
> +		return;
> +	}
> +	mem = it->second.planes()[0];
> +	params = reinterpret_cast<ipu3_uapi_params *>(mem.data());
> +	params->use = {};
>   	for (auto const &algo : algorithms_)
>   		algo->prepare(context_, params);
>   
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index a201c5ca..a13fb881 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -1424,7 +1424,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
>   	if (request->findBuffer(&rawStream_))
>   		pipe()->completeBuffer(request, buffer);
>   
> -	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie());
> +	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie(),
> +			       info->captureParamBuffer->cookie());
>   }
>   
>   void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
Harvey Yang Aug. 2, 2022, 10:31 a.m. UTC | #2 
Hi Umang,

On Wed, Jul 27, 2022 at 4:55 PM Umang Jain <umang.jain@ideasonboard.com>
wrote:

> Hi Harvey,
>
> On 6/29/22 16:00, Harvey Yang via libcamera-devel wrote:
> > From: Harvey Yang <chenghaoyang@chromium.org>
> >
> > This patch updates the ipa interface |IPAIPU3Interface::fillParamsBuffer|
> > with additional |captureBufferId| to fill the param buffer for the
> > StillCapture stream as well.
>
>
> Whenever the IPAIPU3 Interface is updated,
>
>      https://git.libcamera.org/libcamera/ipu3-ipa.git/tree/ (closed
> source IPA IPU3 wrapper)
>
> will need corresponding updates as well (Just mentioning for the records)
>
>
Will try later.


> >
> > Signed-off-by: Harvey Yang <chenghaoyang@chromium.org>
> > ---
> >   include/libcamera/ipa/ipu3.mojom     |  2 +-
> >   src/ipa/ipu3/ipu3.cpp                | 21 ++++++++++++++++++---
> >   src/libcamera/pipeline/ipu3/ipu3.cpp |  3 ++-
> >   3 files changed, 21 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/libcamera/ipa/ipu3.mojom
> b/include/libcamera/ipa/ipu3.mojom
> > index d1b1c6b8..d94c344e 100644
> > --- a/include/libcamera/ipa/ipu3.mojom
> > +++ b/include/libcamera/ipa/ipu3.mojom
> > @@ -31,7 +31,7 @@ interface IPAIPU3Interface {
> >       unmapBuffers(array<uint32> ids);
> >
> >       [async] queueRequest(uint32 frame, libcamera.ControlList controls);
> > -     [async] fillParamsBuffer(uint32 frame, uint32 bufferId);
> > +     [async] fillParamsBuffer(uint32 frame, uint32 bufferId, uint32
> captureBufferId);
> >       [async] processStatsBuffer(uint32 frame, int64 frameTimestamp,
> >                                  uint32 bufferId, libcamera.ControlList
> sensorControls);
> >   };
> > diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
> > index dd6cfd79..149a3958 100644
> > --- a/src/ipa/ipu3/ipu3.cpp
> > +++ b/src/ipa/ipu3/ipu3.cpp
> > @@ -146,7 +146,8 @@ public:
> >       void unmapBuffers(const std::vector<unsigned int> &ids) override;
> >
> >       void queueRequest(const uint32_t frame, const ControlList
> &controls) override;
> > -     void fillParamsBuffer(const uint32_t frame, const uint32_t
> bufferId) override;
> > +     void fillParamsBuffer(const uint32_t frame, const uint32_t
> bufferId,
> > +                           const uint32_t captureBufferId) override;
> >       void processStatsBuffer(const uint32_t frame, const int64_t
> frameTimestamp,
> >                               const uint32_t bufferId,
> >                               const ControlList &sensorControls)
> override;
> > @@ -508,12 +509,14 @@ void IPAIPU3::unmapBuffers(const
> std::vector<unsigned int> &ids)
> >   /**
> >    * \brief Fill and return a buffer with ISP processing parameters for
> a frame
> >    * \param[in] frame The frame number
> > - * \param[in] bufferId ID of the parameter buffer to fill
> > + * \param[in] bufferId ID of the video parameter buffer to fill
> > + * \param[in] captureBufferId ID of the capture parameter buffer to fill
> >    *
> >    * Algorithms are expected to fill the IPU3 parameter buffer for the
> next
> >    * frame given their most recent processing of the ImgU statistics.
> >    */
> > -void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t
> bufferId)
> > +void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t
> bufferId,
>
>
> Since now there are couple of parameter buffers to fill, this can be
> renamed to depict plural buffers:
>
>      IPAIPU3::fillParamsBuffers(const uint32_t frame, const uint32_t
> bufferId,
>
>
Right, thanks!


> > +                            const uint32_t captureBufferId)
> >   {
> >       auto it = buffers_.find(bufferId);
> >       if (it == buffers_.end()) {
> > @@ -536,6 +539,18 @@ void IPAIPU3::fillParamsBuffer(const uint32_t
> frame, const uint32_t bufferId)
> >        */
> >       params->use = {};
> >
> > +     for (auto const &algo : algorithms_)
> > +             algo->prepare(context_, params);
> > +
> > +     // TODO: use different algorithms to set StillCapture params.
>
>
> Can you provide a brief summary here, specific to algorithms targetting
> StillCapture use-case? How would they differ (and I wonder if they
> work/need statistics from Imgu1).
>
>
Hi Han-lin,
Can you help clarify this? I'm also not sure how the algorithms should be
different for YUV/StillCapture use cases, with the same statistics from the
YUV.
Thanks!


> > +     it = buffers_.find(captureBufferId);
> > +     if (it == buffers_.end()) {
> > +             LOG(IPAIPU3, Error) << "Could not find capture param
> buffer!";
> > +             return;
> > +     }
> > +     mem = it->second.planes()[0];
> > +     params = reinterpret_cast<ipu3_uapi_params *>(mem.data());
> > +     params->use = {};
> >       for (auto const &algo : algorithms_)
> >               algo->prepare(context_, params);
> >
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp
> b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index a201c5ca..a13fb881 100644
> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > @@ -1424,7 +1424,8 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer
> *buffer)
> >       if (request->findBuffer(&rawStream_))
> >               pipe()->completeBuffer(request, buffer);
> >
> > -     ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie());
> > +     ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie(),
> > +                            info->captureParamBuffer->cookie());
> >   }
> >
> >   void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)
>

Patch
diff mbox series 

diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom
index d1b1c6b8..d94c344e 100644
--- a/include/libcamera/ipa/ipu3.mojom
+++ b/include/libcamera/ipa/ipu3.mojom
@@ -31,7 +31,7 @@  interface IPAIPU3Interface {
 	unmapBuffers(array<uint32> ids);
 
 	[async] queueRequest(uint32 frame, libcamera.ControlList controls);
-	[async] fillParamsBuffer(uint32 frame, uint32 bufferId);
+	[async] fillParamsBuffer(uint32 frame, uint32 bufferId, uint32 captureBufferId);
 	[async] processStatsBuffer(uint32 frame, int64 frameTimestamp,
 				   uint32 bufferId, libcamera.ControlList sensorControls);
 };
diff --git a/src/ipa/ipu3/ipu3.cpp b/src/ipa/ipu3/ipu3.cpp
index dd6cfd79..149a3958 100644
--- a/src/ipa/ipu3/ipu3.cpp
+++ b/src/ipa/ipu3/ipu3.cpp
@@ -146,7 +146,8 @@  public:
 	void unmapBuffers(const std::vector<unsigned int> &ids) override;
 
 	void queueRequest(const uint32_t frame, const ControlList &controls) override;
-	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId) override;
+	void fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,
+			      const uint32_t captureBufferId) override;
 	void processStatsBuffer(const uint32_t frame, const int64_t frameTimestamp,
 				const uint32_t bufferId,
 				const ControlList &sensorControls) override;
@@ -508,12 +509,14 @@  void IPAIPU3::unmapBuffers(const std::vector<unsigned int> &ids)
 /**
  * \brief Fill and return a buffer with ISP processing parameters for a frame
  * \param[in] frame The frame number
- * \param[in] bufferId ID of the parameter buffer to fill
+ * \param[in] bufferId ID of the video parameter buffer to fill
+ * \param[in] captureBufferId ID of the capture parameter buffer to fill
  *
  * Algorithms are expected to fill the IPU3 parameter buffer for the next
  * frame given their most recent processing of the ImgU statistics.
  */
-void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
+void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId,
+			       const uint32_t captureBufferId)
 {
 	auto it = buffers_.find(bufferId);
 	if (it == buffers_.end()) {
@@ -536,6 +539,18 @@  void IPAIPU3::fillParamsBuffer(const uint32_t frame, const uint32_t bufferId)
 	 */
 	params->use = {};
 
+	for (auto const &algo : algorithms_)
+		algo->prepare(context_, params);
+
+	// TODO: use different algorithms to set StillCapture params.
+	it = buffers_.find(captureBufferId);
+	if (it == buffers_.end()) {
+		LOG(IPAIPU3, Error) << "Could not find capture param buffer!";
+		return;
+	}
+	mem = it->second.planes()[0];
+	params = reinterpret_cast<ipu3_uapi_params *>(mem.data());
+	params->use = {};
 	for (auto const &algo : algorithms_)
 		algo->prepare(context_, params);
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index a201c5ca..a13fb881 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -1424,7 +1424,8 @@  void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
 	if (request->findBuffer(&rawStream_))
 		pipe()->completeBuffer(request, buffer);
 
-	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie());
+	ipa_->fillParamsBuffer(info->id, info->paramBuffer->cookie(),
+			       info->captureParamBuffer->cookie());
 }
 
 void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)