[libcamera-devel,16/30] libcamera: buffer: Buffer remove metadata information

Message ID 20191126233620.1695316-17-niklas.soderlund@ragnatech.se
State Superseded
Headers show
Series
  • libcamera: Rework buffer API
Related show

Commit Message

Niklas Söderlund Nov. 26, 2019, 11:36 p.m. UTC
All metadata information is now stored in the associated BufferInfo
object, remove all metadata information from the Buffer object.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/buffer.h | 14 ----------
 src/libcamera/buffer.cpp   | 53 +-------------------------------------
 2 files changed, 1 insertion(+), 66 deletions(-)

Comments

Jacopo Mondi Dec. 2, 2019, 10:07 a.m. UTC | #1
Hi Niklas,

On Wed, Nov 27, 2019 at 12:36:06AM +0100, Niklas Söderlund wrote:
> All metadata information is now stored in the associated BufferInfo
> object, remove all metadata information from the Buffer object.

s/object/class

Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

Thanks
   j

>
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  include/libcamera/buffer.h | 14 ----------
>  src/libcamera/buffer.cpp   | 53 +-------------------------------------
>  2 files changed, 1 insertion(+), 66 deletions(-)
>
> diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> index 2e5376fb8b53a4c5..acc876eec7d93873 100644
> --- a/include/libcamera/buffer.h
> +++ b/include/libcamera/buffer.h
> @@ -105,12 +105,6 @@ private:
>  class Buffer final
>  {
>  public:
> -	enum Status {
> -		BufferSuccess,
> -		BufferError,
> -		BufferCancelled,
> -	};
> -
>  	Buffer(unsigned int index = -1, const Buffer *metadata = nullptr);
>  	Buffer(const Buffer &) = delete;
>  	Buffer &operator=(const Buffer &) = delete;
> @@ -119,12 +113,8 @@ public:
>  	const std::array<int, 3> &dmabufs() const { return dmabuf_; }
>  	BufferMemory *mem() { return mem_; }
>
> -	unsigned int bytesused() const { return bytesused_; }
> -	uint64_t timestamp() const { return timestamp_; }
> -	unsigned int sequence() const { return sequence_; }
>  	const BufferInfo &info() const { return info_; };
>
> -	Status status() const { return status_; }
>  	Request *request() const { return request_; }
>  	Stream *stream() const { return stream_; }
>
> @@ -140,12 +130,8 @@ private:
>  	std::array<int, 3> dmabuf_;
>  	BufferMemory *mem_;
>
> -	unsigned int bytesused_;
> -	uint64_t timestamp_;
> -	unsigned int sequence_;
>  	BufferInfo info_;
>
> -	Status status_;
>  	Request *request_;
>  	Stream *stream_;
>  };
> diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644
> --- a/src/libcamera/buffer.cpp
> +++ b/src/libcamera/buffer.cpp
> @@ -349,20 +349,6 @@ void BufferPool::destroyBuffers()
>   * deleted automatically after the request complete handler returns.
>   */
>
> -/**
> - * \enum Buffer::Status
> - * Buffer completion status
> - * \var Buffer::BufferSuccess
> - * The buffer has completed with success and contains valid data. All its other
> - * metadata (such as bytesused(), timestamp() or sequence() number) are valid.
> - * \var Buffer::BufferError
> - * The buffer has completed with an error and doesn't contain valid data. Its
> - * other metadata are valid.
> - * \var Buffer::BufferCancelled
> - * The buffer has been cancelled due to capture stop. Its other metadata are
> - * invalid and shall not be used.
> - */
> -
>  /**
>   * \brief Construct a buffer not associated with any stream
>   *
> @@ -371,8 +357,7 @@ void BufferPool::destroyBuffers()
>   * for a stream with Stream::createBuffer().
>   */
>  Buffer::Buffer(unsigned int index, const Buffer *metadata)
> -	: index_(index), dmabuf_({ -1, -1, -1 }),
> -	  status_(Buffer::BufferSuccess), request_(nullptr),
> +	: index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr),
>  	  stream_(nullptr)
>  {
>  	unsigned int sequence;
> @@ -420,32 +405,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)
>   * \return The BufferMemory this buffer is associated with
>   */
>
> -/**
> - * \fn Buffer::bytesused()
> - * \brief Retrieve the number of bytes occupied by the data in the buffer
> - * \return Number of bytes occupied in the buffer
> - */
> -
> -/**
> - * \fn Buffer::timestamp()
> - * \brief Retrieve the time when the buffer was processed
> - *
> - * The timestamp is expressed as a number of nanoseconds since the epoch.
> - *
> - * \return Timestamp when the buffer was processed
> - */
> -
> -/**
> - * \fn Buffer::sequence()
> - * \brief Retrieve the buffer sequence number
> - *
> - * The sequence number is a monotonically increasing number assigned to the
> - * buffer processed by the stream. Gaps in the sequence numbers indicate
> - * dropped frames.
> - *
> - * \return Sequence number of the buffer
> - */
> -
>  /**
>   * \fn Buffer::info()
>   * \brief Retrieve the buffer metadata information
> @@ -456,16 +415,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)
>   * \return Metadata of the buffer
>   */
>
> -/**
> - * \fn Buffer::status()
> - * \brief Retrieve the buffer status
> - *
> - * The buffer status reports whether the buffer has completed successfully
> - * (BufferSuccess) or if an error occurred (BufferError).
> - *
> - * \return The buffer status
> - */
> -
>  /**
>   * \fn Buffer::request()
>   * \brief Retrieve the request this buffer belongs to
> --
> 2.24.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart Dec. 9, 2019, 9:22 p.m. UTC | #2
Hi Niklas,

Thank you for the patch.

On Mon, Dec 02, 2019 at 11:07:36AM +0100, Jacopo Mondi wrote:
> On Wed, Nov 27, 2019 at 12:36:06AM +0100, Niklas Söderlund wrote:
> > All metadata information is now stored in the associated BufferInfo
> > object, remove all metadata information from the Buffer object.
> 
> s/object/class
> 
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

And I would squash this with 15/30. Reviewing 15/30 left me wondering if
there could be usages of the old fields that were missed, if you remove
them in the same patch review will be easier.

> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  include/libcamera/buffer.h | 14 ----------
> >  src/libcamera/buffer.cpp   | 53 +-------------------------------------
> >  2 files changed, 1 insertion(+), 66 deletions(-)
> >
> > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> > index 2e5376fb8b53a4c5..acc876eec7d93873 100644
> > --- a/include/libcamera/buffer.h
> > +++ b/include/libcamera/buffer.h
> > @@ -105,12 +105,6 @@ private:
> >  class Buffer final
> >  {
> >  public:
> > -	enum Status {
> > -		BufferSuccess,
> > -		BufferError,
> > -		BufferCancelled,
> > -	};
> > -
> >  	Buffer(unsigned int index = -1, const Buffer *metadata = nullptr);
> >  	Buffer(const Buffer &) = delete;
> >  	Buffer &operator=(const Buffer &) = delete;
> > @@ -119,12 +113,8 @@ public:
> >  	const std::array<int, 3> &dmabufs() const { return dmabuf_; }
> >  	BufferMemory *mem() { return mem_; }
> >
> > -	unsigned int bytesused() const { return bytesused_; }
> > -	uint64_t timestamp() const { return timestamp_; }
> > -	unsigned int sequence() const { return sequence_; }
> >  	const BufferInfo &info() const { return info_; };
> >
> > -	Status status() const { return status_; }
> >  	Request *request() const { return request_; }
> >  	Stream *stream() const { return stream_; }
> >
> > @@ -140,12 +130,8 @@ private:
> >  	std::array<int, 3> dmabuf_;
> >  	BufferMemory *mem_;
> >
> > -	unsigned int bytesused_;
> > -	uint64_t timestamp_;
> > -	unsigned int sequence_;
> >  	BufferInfo info_;
> >
> > -	Status status_;
> >  	Request *request_;
> >  	Stream *stream_;
> >  };
> > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> > index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644
> > --- a/src/libcamera/buffer.cpp
> > +++ b/src/libcamera/buffer.cpp
> > @@ -349,20 +349,6 @@ void BufferPool::destroyBuffers()
> >   * deleted automatically after the request complete handler returns.
> >   */
> >
> > -/**
> > - * \enum Buffer::Status
> > - * Buffer completion status
> > - * \var Buffer::BufferSuccess
> > - * The buffer has completed with success and contains valid data. All its other
> > - * metadata (such as bytesused(), timestamp() or sequence() number) are valid.
> > - * \var Buffer::BufferError
> > - * The buffer has completed with an error and doesn't contain valid data. Its
> > - * other metadata are valid.
> > - * \var Buffer::BufferCancelled
> > - * The buffer has been cancelled due to capture stop. Its other metadata are
> > - * invalid and shall not be used.
> > - */
> > -
> >  /**
> >   * \brief Construct a buffer not associated with any stream
> >   *
> > @@ -371,8 +357,7 @@ void BufferPool::destroyBuffers()
> >   * for a stream with Stream::createBuffer().
> >   */
> >  Buffer::Buffer(unsigned int index, const Buffer *metadata)
> > -	: index_(index), dmabuf_({ -1, -1, -1 }),
> > -	  status_(Buffer::BufferSuccess), request_(nullptr),
> > +	: index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr),
> >  	  stream_(nullptr)
> >  {
> >  	unsigned int sequence;
> > @@ -420,32 +405,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)
> >   * \return The BufferMemory this buffer is associated with
> >   */
> >
> > -/**
> > - * \fn Buffer::bytesused()
> > - * \brief Retrieve the number of bytes occupied by the data in the buffer
> > - * \return Number of bytes occupied in the buffer
> > - */
> > -
> > -/**
> > - * \fn Buffer::timestamp()
> > - * \brief Retrieve the time when the buffer was processed
> > - *
> > - * The timestamp is expressed as a number of nanoseconds since the epoch.
> > - *
> > - * \return Timestamp when the buffer was processed
> > - */
> > -
> > -/**
> > - * \fn Buffer::sequence()
> > - * \brief Retrieve the buffer sequence number
> > - *
> > - * The sequence number is a monotonically increasing number assigned to the
> > - * buffer processed by the stream. Gaps in the sequence numbers indicate
> > - * dropped frames.
> > - *
> > - * \return Sequence number of the buffer
> > - */
> > -
> >  /**
> >   * \fn Buffer::info()
> >   * \brief Retrieve the buffer metadata information
> > @@ -456,16 +415,6 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)
> >   * \return Metadata of the buffer
> >   */
> >
> > -/**
> > - * \fn Buffer::status()
> > - * \brief Retrieve the buffer status
> > - *
> > - * The buffer status reports whether the buffer has completed successfully
> > - * (BufferSuccess) or if an error occurred (BufferError).
> > - *
> > - * \return The buffer status
> > - */
> > -
> >  /**
> >   * \fn Buffer::request()
> >   * \brief Retrieve the request this buffer belongs to

Patch

diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
index 2e5376fb8b53a4c5..acc876eec7d93873 100644
--- a/include/libcamera/buffer.h
+++ b/include/libcamera/buffer.h
@@ -105,12 +105,6 @@  private:
 class Buffer final
 {
 public:
-	enum Status {
-		BufferSuccess,
-		BufferError,
-		BufferCancelled,
-	};
-
 	Buffer(unsigned int index = -1, const Buffer *metadata = nullptr);
 	Buffer(const Buffer &) = delete;
 	Buffer &operator=(const Buffer &) = delete;
@@ -119,12 +113,8 @@  public:
 	const std::array<int, 3> &dmabufs() const { return dmabuf_; }
 	BufferMemory *mem() { return mem_; }
 
-	unsigned int bytesused() const { return bytesused_; }
-	uint64_t timestamp() const { return timestamp_; }
-	unsigned int sequence() const { return sequence_; }
 	const BufferInfo &info() const { return info_; };
 
-	Status status() const { return status_; }
 	Request *request() const { return request_; }
 	Stream *stream() const { return stream_; }
 
@@ -140,12 +130,8 @@  private:
 	std::array<int, 3> dmabuf_;
 	BufferMemory *mem_;
 
-	unsigned int bytesused_;
-	uint64_t timestamp_;
-	unsigned int sequence_;
 	BufferInfo info_;
 
-	Status status_;
 	Request *request_;
 	Stream *stream_;
 };
diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
index d5a4815a0bb8c528..ab28e0d76b8c40f7 100644
--- a/src/libcamera/buffer.cpp
+++ b/src/libcamera/buffer.cpp
@@ -349,20 +349,6 @@  void BufferPool::destroyBuffers()
  * deleted automatically after the request complete handler returns.
  */
 
-/**
- * \enum Buffer::Status
- * Buffer completion status
- * \var Buffer::BufferSuccess
- * The buffer has completed with success and contains valid data. All its other
- * metadata (such as bytesused(), timestamp() or sequence() number) are valid.
- * \var Buffer::BufferError
- * The buffer has completed with an error and doesn't contain valid data. Its
- * other metadata are valid.
- * \var Buffer::BufferCancelled
- * The buffer has been cancelled due to capture stop. Its other metadata are
- * invalid and shall not be used.
- */
-
 /**
  * \brief Construct a buffer not associated with any stream
  *
@@ -371,8 +357,7 @@  void BufferPool::destroyBuffers()
  * for a stream with Stream::createBuffer().
  */
 Buffer::Buffer(unsigned int index, const Buffer *metadata)
-	: index_(index), dmabuf_({ -1, -1, -1 }),
-	  status_(Buffer::BufferSuccess), request_(nullptr),
+	: index_(index), dmabuf_({ -1, -1, -1 }), request_(nullptr),
 	  stream_(nullptr)
 {
 	unsigned int sequence;
@@ -420,32 +405,6 @@  Buffer::Buffer(unsigned int index, const Buffer *metadata)
  * \return The BufferMemory this buffer is associated with
  */
 
-/**
- * \fn Buffer::bytesused()
- * \brief Retrieve the number of bytes occupied by the data in the buffer
- * \return Number of bytes occupied in the buffer
- */
-
-/**
- * \fn Buffer::timestamp()
- * \brief Retrieve the time when the buffer was processed
- *
- * The timestamp is expressed as a number of nanoseconds since the epoch.
- *
- * \return Timestamp when the buffer was processed
- */
-
-/**
- * \fn Buffer::sequence()
- * \brief Retrieve the buffer sequence number
- *
- * The sequence number is a monotonically increasing number assigned to the
- * buffer processed by the stream. Gaps in the sequence numbers indicate
- * dropped frames.
- *
- * \return Sequence number of the buffer
- */
-
 /**
  * \fn Buffer::info()
  * \brief Retrieve the buffer metadata information
@@ -456,16 +415,6 @@  Buffer::Buffer(unsigned int index, const Buffer *metadata)
  * \return Metadata of the buffer
  */
 
-/**
- * \fn Buffer::status()
- * \brief Retrieve the buffer status
- *
- * The buffer status reports whether the buffer has completed successfully
- * (BufferSuccess) or if an error occurred (BufferError).
- *
- * \return The buffer status
- */
-
 /**
  * \fn Buffer::request()
  * \brief Retrieve the request this buffer belongs to