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

[Laurent Pinchart]

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 at 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 at 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

-- 
Regards,

Laurent Pinchart