[libcamera-devel,v3,05/13] libcamera: request: Add IPAMetaData

Message ID 20190927024417.725906-6-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: ipa: Add basic IPA support
Related show

Commit Message

Niklas Söderlund Sept. 27, 2019, 2:44 a.m. UTC
Add a new structure to hold meta data coming out of the IPA. The
structure will grow over time but for now only add information about the
auto exposure state as it can be directly used by the rkisp1 IPA, which
is capable of controlling exposure.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/request.h | 12 +++++++++++
 src/libcamera/request.cpp   | 42 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

Comments

Jacopo Mondi Sept. 28, 2019, 10:55 a.m. UTC | #1
Hi Niklas,

On Fri, Sep 27, 2019 at 04:44:09AM +0200, Niklas Söderlund wrote:
> Add a new structure to hold meta data coming out of the IPA. The
> structure will grow over time but for now only add information about the
> auto exposure state as it can be directly used by the rkisp1 IPA, which
> is capable of controlling exposure.
>

As commented on the IPA interface, why a C struct ? won't we have the
same issue we would have for Controls? Do you think this could be
replaced by a ControlList ?

Thanks
  j

> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/request.h | 12 +++++++++++
>  src/libcamera/request.cpp   | 42 ++++++++++++++++++++++++++++++++++++-
>  2 files changed, 53 insertions(+), 1 deletion(-)
>
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 5eb012e41b4a377b..38a6f008d53dc53a 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -21,6 +21,16 @@ class Buffer;
>  class Camera;
>  class Stream;
>
> +enum AeState {
> +	Inactive,
> +	Searching,
> +	Converged,
> +};
> +
> +struct IPAMetaData {
> +	AeState aeState;
> +	bool ready;
> +};
>
>  class Request
>  {
> @@ -41,6 +51,7 @@ public:
>  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
>  	int addBuffer(std::unique_ptr<Buffer> buffer);
>  	Buffer *findBuffer(Stream *stream) const;
> +	const IPAMetaData &metaData() const { return metaData_; };
>
>  	uint64_t cookie() const { return cookie_; }
>  	Status status() const { return status_; }
> @@ -60,6 +71,7 @@ private:
>  	ControlList controls_;
>  	std::map<Stream *, Buffer *> bufferMap_;
>  	std::unordered_set<Buffer *> pending_;
> +	IPAMetaData metaData_;
>
>  	const uint64_t cookie_;
>  	Status status_;
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index ebae99b07c696512..226a10b6a3c048c5 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -24,6 +24,40 @@ namespace libcamera {
>
>  LOG_DEFINE_CATEGORY(Request)
>
> +/**
> + * \enum AeState
> + * State of Auto Exposure algorithm
> + * \var AeState::Inactive
> + * AE not running
> + * \var AeState::Searching
> + * AE is not converged to a good value and is adjusting exposure parameters.
> + * \var AeState::Converged
> + * AE has found good exposure values for the current scene.
> + */
> +
> +/**
> + * \struct IPAMetaData
> + * \brief Meta data describing the state of the IPA
> + *
> + * Container for IPA meta data. The intended creator of this object is an IPA
> + * and the intended consumer is applications. Applications access the object
> + * thru the Request object that corresponds to the specific capture event
> + * that generated the meta data.
> + */
> +
> +/**
> + * \var IPAMetaData::aeState
> + * \brief Holds the state of the Auto Exposure algorithm
> + */
> +
> +/**
> + * \var IPAMetaData::ready
> + * \brief Flag to indicate the pipeline have validated the meta data
> + *
> + * The meta data should not be returned to the application by the specific
> + * pipeline handler implementation before this flag is set to true.
> + */
> +
>  /**
>   * \enum Request::Status
>   * Request completion status
> @@ -55,7 +89,7 @@ LOG_DEFINE_CATEGORY(Request)
>   *
>   */
>  Request::Request(Camera *camera, uint64_t cookie)
> -	: camera_(camera), controls_(camera), cookie_(cookie),
> +	: camera_(camera), controls_(camera), metaData_({}), cookie_(cookie),
>  	  status_(RequestPending), cancelled_(false)
>  {
>  }
> @@ -157,6 +191,12 @@ Buffer *Request::findBuffer(Stream *stream) const
>  	return it->second;
>  }
>
> +/**
> + * \fn Request::metaData()
> + * \brief Retrieve the request's meta data
> + * \return The meta data associated with the request
> + */
> +
>  /**
>   * \fn Request::cookie()
>   * \brief Retrieve the cookie set when the request was created
> --
> 2.23.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Sept. 28, 2019, 7:21 p.m. UTC | #2
Hello,

On Sat, Sep 28, 2019 at 12:55:33PM +0200, Jacopo Mondi wrote:
> On Fri, Sep 27, 2019 at 04:44:09AM +0200, Niklas Söderlund wrote:
> > Add a new structure to hold meta data coming out of the IPA. The
> > structure will grow over time but for now only add information about the
> > auto exposure state as it can be directly used by the rkisp1 IPA, which
> > is capable of controlling exposure.
> 
> As commented on the IPA interface, why a C struct ? won't we have the
> same issue we would have for Controls? Do you think this could be
> replaced by a ControlList ?

That's what I was going to mention too, it should be a ControlList.

> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  include/libcamera/request.h | 12 +++++++++++
> >  src/libcamera/request.cpp   | 42 ++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 53 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> > index 5eb012e41b4a377b..38a6f008d53dc53a 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -21,6 +21,16 @@ class Buffer;
> >  class Camera;
> >  class Stream;
> >
> > +enum AeState {
> > +	Inactive,
> > +	Searching,
> > +	Converged,
> > +};
> > +
> > +struct IPAMetaData {
> > +	AeState aeState;
> > +	bool ready;
> > +};
> >
> >  class Request
> >  {
> > @@ -41,6 +51,7 @@ public:
> >  	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
> >  	int addBuffer(std::unique_ptr<Buffer> buffer);
> >  	Buffer *findBuffer(Stream *stream) const;
> > +	const IPAMetaData &metaData() const { return metaData_; };
> >
> >  	uint64_t cookie() const { return cookie_; }
> >  	Status status() const { return status_; }
> > @@ -60,6 +71,7 @@ private:
> >  	ControlList controls_;
> >  	std::map<Stream *, Buffer *> bufferMap_;
> >  	std::unordered_set<Buffer *> pending_;
> > +	IPAMetaData metaData_;
> >
> >  	const uint64_t cookie_;
> >  	Status status_;
> > diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> > index ebae99b07c696512..226a10b6a3c048c5 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -24,6 +24,40 @@ namespace libcamera {
> >
> >  LOG_DEFINE_CATEGORY(Request)
> >
> > +/**
> > + * \enum AeState
> > + * State of Auto Exposure algorithm
> > + * \var AeState::Inactive
> > + * AE not running
> > + * \var AeState::Searching
> > + * AE is not converged to a good value and is adjusting exposure parameters.
> > + * \var AeState::Converged
> > + * AE has found good exposure values for the current scene.
> > + */
> > +
> > +/**
> > + * \struct IPAMetaData
> > + * \brief Meta data describing the state of the IPA
> > + *
> > + * Container for IPA meta data. The intended creator of this object is an IPA
> > + * and the intended consumer is applications. Applications access the object
> > + * thru the Request object that corresponds to the specific capture event
> > + * that generated the meta data.
> > + */
> > +
> > +/**
> > + * \var IPAMetaData::aeState
> > + * \brief Holds the state of the Auto Exposure algorithm
> > + */
> > +
> > +/**
> > + * \var IPAMetaData::ready
> > + * \brief Flag to indicate the pipeline have validated the meta data
> > + *
> > + * The meta data should not be returned to the application by the specific
> > + * pipeline handler implementation before this flag is set to true.
> > + */
> > +
> >  /**
> >   * \enum Request::Status
> >   * Request completion status
> > @@ -55,7 +89,7 @@ LOG_DEFINE_CATEGORY(Request)
> >   *
> >   */
> >  Request::Request(Camera *camera, uint64_t cookie)
> > -	: camera_(camera), controls_(camera), cookie_(cookie),
> > +	: camera_(camera), controls_(camera), metaData_({}), cookie_(cookie),
> >  	  status_(RequestPending), cancelled_(false)
> >  {
> >  }
> > @@ -157,6 +191,12 @@ Buffer *Request::findBuffer(Stream *stream) const
> >  	return it->second;
> >  }
> >
> > +/**
> > + * \fn Request::metaData()
> > + * \brief Retrieve the request's meta data
> > + * \return The meta data associated with the request
> > + */
> > +
> >  /**
> >   * \fn Request::cookie()
> >   * \brief Retrieve the cookie set when the request was created

Patch

diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 5eb012e41b4a377b..38a6f008d53dc53a 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -21,6 +21,16 @@  class Buffer;
 class Camera;
 class Stream;
 
+enum AeState {
+	Inactive,
+	Searching,
+	Converged,
+};
+
+struct IPAMetaData {
+	AeState aeState;
+	bool ready;
+};
 
 class Request
 {
@@ -41,6 +51,7 @@  public:
 	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
 	int addBuffer(std::unique_ptr<Buffer> buffer);
 	Buffer *findBuffer(Stream *stream) const;
+	const IPAMetaData &metaData() const { return metaData_; };
 
 	uint64_t cookie() const { return cookie_; }
 	Status status() const { return status_; }
@@ -60,6 +71,7 @@  private:
 	ControlList controls_;
 	std::map<Stream *, Buffer *> bufferMap_;
 	std::unordered_set<Buffer *> pending_;
+	IPAMetaData metaData_;
 
 	const uint64_t cookie_;
 	Status status_;
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index ebae99b07c696512..226a10b6a3c048c5 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -24,6 +24,40 @@  namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Request)
 
+/**
+ * \enum AeState
+ * State of Auto Exposure algorithm
+ * \var AeState::Inactive
+ * AE not running
+ * \var AeState::Searching
+ * AE is not converged to a good value and is adjusting exposure parameters.
+ * \var AeState::Converged
+ * AE has found good exposure values for the current scene.
+ */
+
+/**
+ * \struct IPAMetaData
+ * \brief Meta data describing the state of the IPA
+ *
+ * Container for IPA meta data. The intended creator of this object is an IPA
+ * and the intended consumer is applications. Applications access the object
+ * thru the Request object that corresponds to the specific capture event
+ * that generated the meta data.
+ */
+
+/**
+ * \var IPAMetaData::aeState
+ * \brief Holds the state of the Auto Exposure algorithm
+ */
+
+/**
+ * \var IPAMetaData::ready
+ * \brief Flag to indicate the pipeline have validated the meta data
+ *
+ * The meta data should not be returned to the application by the specific
+ * pipeline handler implementation before this flag is set to true.
+ */
+
 /**
  * \enum Request::Status
  * Request completion status
@@ -55,7 +89,7 @@  LOG_DEFINE_CATEGORY(Request)
  *
  */
 Request::Request(Camera *camera, uint64_t cookie)
-	: camera_(camera), controls_(camera), cookie_(cookie),
+	: camera_(camera), controls_(camera), metaData_({}), cookie_(cookie),
 	  status_(RequestPending), cancelled_(false)
 {
 }
@@ -157,6 +191,12 @@  Buffer *Request::findBuffer(Stream *stream) const
 	return it->second;
 }
 
+/**
+ * \fn Request::metaData()
+ * \brief Retrieve the request's meta data
+ * \return The meta data associated with the request
+ */
+
 /**
  * \fn Request::cookie()
  * \brief Retrieve the cookie set when the request was created