[libcamera-devel] [PATCH v2 13/16] libcamera: buffer: Add an accessor to the BufferMemory

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sun Jul 14 09:40:40 CEST 2019


Hi Jacopo,

On Sun, Jul 14, 2019 at 09:27:44AM +0200, Jacopo Mondi wrote:
> On Sat, Jul 13, 2019 at 08:23:48PM +0300, Laurent Pinchart wrote:
> > Buffer instances reference memory, which is modelled internally by a
> > BufferMemory instance. Store a pointer to the BufferMemory in the Buffer
> > class, and populate it when the buffer is queued to the camera through a
> > request. This is useful for applications to access the buffer memory in
> > the buffer or request completion handler.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > ---
> >  include/libcamera/buffer.h |  3 +++
> >  src/cam/buffer_writer.cpp  |  4 ++--
> >  src/cam/buffer_writer.h    |  3 +--
> >  src/cam/capture.cpp        |  3 +--
> >  src/libcamera/buffer.cpp   | 11 +++++++++++
> >  src/libcamera/camera.cpp   |  4 ++++
> >  src/qcam/main_window.cpp   |  7 +++----
> >  src/qcam/main_window.h     |  2 +-
> >  8 files changed, 26 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h
> > index f8569a6b67f3..fc5c7d4c41b6 100644
> > --- a/include/libcamera/buffer.h
> > +++ b/include/libcamera/buffer.h
> > @@ -77,6 +77,7 @@ public:
> >
> >  	unsigned int index() const { return index_; }
> >  	const std::array<int, 3> &dmabufs() const { return dmabuf_; }
> > +	BufferMemory *mem() { return mem_; }
> 
> Should this return a const * or is the buffer available in write mode
> too ?

The BufferMemory is used to retrieve a plane, which is then used to
access the plane's CPU-mapped memory, and that can't operate on a const
pointer.

> >
> >  	unsigned int bytesused() const { return bytesused_; }
> >  	uint64_t timestamp() const { return timestamp_; }
> > @@ -87,6 +88,7 @@ public:
> >  	Stream *stream() const { return stream_; }
> >
> >  private:
> > +	friend class Camera;
> >  	friend class PipelineHandler;
> >  	friend class Request;
> >  	friend class Stream;
> > @@ -98,6 +100,7 @@ private:
> >
> >  	unsigned int index_;
> >  	std::array<int, 3> dmabuf_;
> > +	BufferMemory *mem_;
> >
> >  	unsigned int bytesused_;
> >  	uint64_t timestamp_;
> > diff --git a/src/cam/buffer_writer.cpp b/src/cam/buffer_writer.cpp
> > index b7f2ed4f2d2d..1c044b063467 100644
> > --- a/src/cam/buffer_writer.cpp
> > +++ b/src/cam/buffer_writer.cpp
> > @@ -19,8 +19,7 @@ BufferWriter::BufferWriter(const std::string &pattern)
> >  {
> >  }
> >
> > -int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,
> > -			const std::string &streamName)
> > +int BufferWriter::write(libcamera::Buffer *buffer, const std::string &streamName)
> >  {
> >  	std::string filename;
> >  	size_t pos;
> > @@ -41,6 +40,7 @@ int BufferWriter::write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,
> >  	if (fd == -1)
> >  		return -errno;
> >
> > +	libcamera::BufferMemory *mem = buffer->mem();
> >  	for (libcamera::Plane &plane : mem->planes()) {
> >  		void *data = plane.mem();
> >  		unsigned int length = plane.length();
> > diff --git a/src/cam/buffer_writer.h b/src/cam/buffer_writer.h
> > index 9bea205fac1d..7bf785d1e832 100644
> > --- a/src/cam/buffer_writer.h
> > +++ b/src/cam/buffer_writer.h
> > @@ -16,8 +16,7 @@ class BufferWriter
> >  public:
> >  	BufferWriter(const std::string &pattern = "frame-#.bin");
> >
> > -	int write(libcamera::Buffer *buffer, libcamera::BufferMemory *mem,
> > -		  const std::string &streamName);
> > +	int write(libcamera::Buffer *buffer, const std::string &streamName);
> >
> >  private:
> >  	std::string pattern_;
> > diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> > index 5ffa4ae291da..df9602de4ab8 100644
> > --- a/src/cam/capture.cpp
> > +++ b/src/cam/capture.cpp
> > @@ -154,7 +154,6 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer
> >  	for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> >  		Stream *stream = it->first;
> >  		Buffer *buffer = it->second;
> > -		BufferMemory *mem = &stream->buffers()[buffer->index()];
> >  		const std::string &name = streamName_[stream];
> >
> >  		info << " " << name
> > @@ -163,7 +162,7 @@ void Capture::requestComplete(Request *request, const std::map<Stream *, Buffer
> >  		     << " bytesused: " << buffer->bytesused();
> >
> >  		if (writer_)
> > -			writer_->write(buffer, mem, name);
> > +			writer_->write(buffer, name);
> >  	}
> >
> >  	std::cout << info.str() << std::endl;
> > diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp
> > index 99358633a088..fe29c2f68619 100644
> > --- a/src/libcamera/buffer.cpp
> > +++ b/src/libcamera/buffer.cpp
> > @@ -300,6 +300,17 @@ Buffer::Buffer(unsigned int index, const Buffer *metadata)
> >   * \return The dmabuf file descriptors
> >   */
> >
> > +/**
> > + * \fn Buffer::mem()
> > + * \brief Retrieve the BufferMemory this buffer is associated with
> > + *
> > + * The association between the buffer and a BufferMemory instance is valid from
> > + * the time the request containing this buffer is queued to a camera to the end
> > + * of that request's completion handler.
> > + *
> > + * \return The BufferMemory this buffer is associated with
> > + */
> > +
> >  /**
> >   * \fn Buffer::bytesused()
> >   * \brief Retrieve the number of bytes occupied by the data in the buffer
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index af69607b19e6..db15fd46cd51 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -811,10 +811,14 @@ int Camera::queueRequest(Request *request)
> >
> >  	for (auto const &it : request->buffers()) {
> >  		Stream *stream = it.first;
> > +		Buffer *buffer = it.second;
> > +
> >  		if (activeStreams_.find(stream) == activeStreams_.end()) {
> >  			LOG(Camera, Error) << "Invalid request";
> >  			return -EINVAL;
> >  		}
> > +
> > +		buffer->mem_ = &stream->buffers()[buffer->index_];
> >  	}
> >
> >  	int ret = request->prepare();
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 92f888323f0b..5c26ab8ebf70 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -221,7 +221,6 @@ void MainWindow::requestComplete(Request *request,
> >
> >  	framesCaptured_++;
> >
> > -	Stream *stream = buffers.begin()->first;
> >  	Buffer *buffer = buffers.begin()->second;
> >
> >  	double fps = buffer->timestamp() - lastBufferTime_;
> > @@ -235,8 +234,7 @@ void MainWindow::requestComplete(Request *request,
> >  		  << " fps: " << std::fixed << std::setprecision(2) << fps
> >  		  << std::endl;
> >
> > -	BufferMemory *mem = &stream->buffers()[buffer->index()];
> > -	display(buffer, mem);
> > +	display(buffer);
> >
> >  	request = camera_->createRequest();
> >  	if (!request) {
> > @@ -261,8 +259,9 @@ void MainWindow::requestComplete(Request *request,
> >  	camera_->queueRequest(request);
> >  }
> >
> > -int MainWindow::display(Buffer *buffer, BufferMemory *mem)
> > +int MainWindow::display(Buffer *buffer)
> >  {
> > +	BufferMemory *mem = buffer->mem();
> >  	if (mem->planes().size() != 1)
> >  		return -EINVAL;
> >
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index b4f6f747e16e..f58cb6a65b2b 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -48,7 +48,7 @@ private:
> >
> >  	void requestComplete(Request *request,
> >  			     const std::map<Stream *, Buffer *> &buffers);
> > -	int display(Buffer *buffer, BufferMemory *mem);
> > +	int display(Buffer *buffer);
> >
> >  	QString title_;
> >  	QTimer titleTimer_;

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list