[libcamera-devel,v2,24/25] libcamera: pipeline: Remove explicit buffer handling
diff mbox series

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

Commit Message

Niklas Söderlund Dec. 30, 2019, 12:05 p.m. UTC 
With the FrameBuffer interface in place there is no need for the Camera
to call into the specific pipelines allocation and freeing of buffers as
it no longer needs to be synchronized with buffer allocation by the
application.

Remove the function prototypes in the pipeline handler base class and
fold the functionality in the pipelines start() and stop() functions
where needed. A follow up patch will remove the now no-op
Camera::allocateBuffers() and Camera::freeBuffers().

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/camera.cpp                 |  8 +------
 src/libcamera/include/pipeline_handler.h |  5 ----
 src/libcamera/pipeline/ipu3/ipu3.cpp     | 24 ++++++++++++--------
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 24 ++++++++++++--------
 src/libcamera/pipeline/uvcvideo.cpp      | 17 --------------
 src/libcamera/pipeline/vimc.cpp          | 17 --------------
 src/libcamera/pipeline_handler.cpp       | 29 ------------------------
 7 files changed, 30 insertions(+), 94 deletions(-)

Patch
diff mbox series 

diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 5ea663ef85720f2e..dba537ef0da61b44 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -722,12 +722,6 @@  int Camera::allocateBuffers()
 		return -EINVAL;
 	}
 
-	int ret = pipe_->allocateBuffers(this, activeStreams_);
-	if (ret) {
-		LOG(Camera, Error) << "Failed to allocate buffers";
-		return ret;
-	}
-
 	state_ = CameraPrepared;
 
 	return 0;
@@ -748,7 +742,7 @@  int Camera::freeBuffers()
 
 	state_ = CameraConfigured;
 
-	return pipe_->freeBuffers(this, activeStreams_);
+	return 0;
 }
 
 /**
diff --git a/src/libcamera/include/pipeline_handler.h b/src/libcamera/include/pipeline_handler.h
index 20b866c4a511387a..6cee976a6ce1939d 100644
--- a/src/libcamera/include/pipeline_handler.h
+++ b/src/libcamera/include/pipeline_handler.h
@@ -76,11 +76,6 @@  public:
 				       unsigned int count) = 0;
 	virtual void freeFrameBuffers(Camera *camera, Stream *stream) = 0;
 
-	virtual int allocateBuffers(Camera *camera,
-				    const std::set<Stream *> &streams) = 0;
-	virtual int freeBuffers(Camera *camera,
-				const std::set<Stream *> &streams) = 0;
-
 	virtual int start(Camera *camera) = 0;
 	virtual void stop(Camera *camera) = 0;
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index ea02a4e5f7f04718..4f837ebc68730883 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -213,11 +213,6 @@  public:
 			       unsigned int count) override;
 	void freeFrameBuffers(Camera *camera, Stream *stream) override;
 
-	int allocateBuffers(Camera *camera,
-			    const std::set<Stream *> &streams) override;
-	int freeBuffers(Camera *camera,
-			const std::set<Stream *> &streams) override;
-
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
@@ -234,6 +229,9 @@  private:
 
 	int registerCameras();
 
+	int allocateBuffers(Camera *camera);
+	int freeBuffers(Camera *camera);
+
 	ImgUDevice imgu0_;
 	ImgUDevice imgu1_;
 	MediaDevice *cio2MediaDev_;
@@ -654,8 +652,7 @@  void PipelineHandlerIPU3::freeFrameBuffers(Camera *camera, Stream *stream)
  * In order to be able to start the 'viewfinder' and 'stat' nodes, we need
  * memory to be reserved.
  */
-int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
-					 const std::set<Stream *> &streams)
+int PipelineHandlerIPU3::allocateBuffers(Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
 	IPU3Stream *outStream = &data->outStream_;
@@ -718,13 +715,12 @@  int PipelineHandlerIPU3::allocateBuffers(Camera *camera,
 	return 0;
 
 error:
-	freeBuffers(camera, streams);
+	freeBuffers(camera);
 
 	return ret;
 }
 
-int PipelineHandlerIPU3::freeBuffers(Camera *camera,
-				     const std::set<Stream *> &streams)
+int PipelineHandlerIPU3::freeBuffers(Camera *camera)
 {
 	IPU3CameraData *data = cameraData(camera);
 
@@ -741,6 +737,11 @@  int PipelineHandlerIPU3::start(Camera *camera)
 	ImgUDevice *imgu = data->imgu_;
 	int ret;
 
+	/* Allocate buffers for internal pipeline usage. */
+	ret = allocateBuffers(camera);
+	if (ret)
+		return ret;
+
 	/*
 	 * Start the ImgU video devices, buffers will be queued to the
 	 * ImgU output and viewfinder when requests will be queued.
@@ -759,6 +760,7 @@  int PipelineHandlerIPU3::start(Camera *camera)
 	return 0;
 
 error:
+	freeBuffers(camera);
 	LOG(IPU3, Error) << "Failed to start camera " << camera->name();
 
 	return ret;
@@ -774,6 +776,8 @@  void PipelineHandlerIPU3::stop(Camera *camera)
 	if (ret)
 		LOG(IPU3, Warning) << "Failed to stop camera "
 				   << camera->name();
+
+	freeBuffers(camera);
 }
 
 int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 7ed729fb7974d6ea..20d549f6dba649a5 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -181,11 +181,6 @@  public:
 			       unsigned int count) override;
 	void freeFrameBuffers(Camera *camera, Stream *stream) override;
 
-	int allocateBuffers(Camera *camera,
-		const std::set<Stream *> &streams) override;
-	int freeBuffers(Camera *camera,
-		const std::set<Stream *> &streams) override;
-
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
@@ -211,6 +206,9 @@  private:
 	void paramReady(FrameBuffer *buffer);
 	void statReady(FrameBuffer *buffer);
 
+	int allocateBuffers(Camera *camera);
+	int freeBuffers(Camera *camera);
+
 	MediaDevice *media_;
 	V4L2Subdevice *dphy_;
 	V4L2Subdevice *isp_;
@@ -678,8 +676,7 @@  void PipelineHandlerRkISP1::freeFrameBuffers(Camera *camera, Stream *stream)
 	video_->releaseBuffers();
 }
 
-int PipelineHandlerRkISP1::allocateBuffers(Camera *camera,
-					   const std::set<Stream *> &streams)
+int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)
 {
 	RkISP1CameraData *data = cameraData(camera);
 	unsigned int count = 1;
@@ -723,8 +720,7 @@  err:
 	return ret;
 }
 
-int PipelineHandlerRkISP1::freeBuffers(Camera *camera,
-				       const std::set<Stream *> &streams)
+int PipelineHandlerRkISP1::freeBuffers(Camera *camera)
 {
 	RkISP1CameraData *data = cameraData(camera);
 
@@ -758,10 +754,16 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 	RkISP1CameraData *data = cameraData(camera);
 	int ret;
 
+	/* Allocate buffers for internal pipeline usage. */
+	ret = allocateBuffers(camera);
+	if (ret)
+		return ret;
+
 	data->frame_ = 0;
 
 	ret = param_->streamOn();
 	if (ret) {
+		freeBuffers(camera);
 		LOG(RkISP1, Error)
 			<< "Failed to start parameters " << camera->name();
 		return ret;
@@ -770,6 +772,7 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 	ret = stat_->streamOn();
 	if (ret) {
 		param_->streamOff();
+		freeBuffers(camera);
 		LOG(RkISP1, Error)
 			<< "Failed to start statistics " << camera->name();
 		return ret;
@@ -779,6 +782,7 @@  int PipelineHandlerRkISP1::start(Camera *camera)
 	if (ret) {
 		param_->streamOff();
 		stat_->streamOff();
+		freeBuffers(camera);
 
 		LOG(RkISP1, Error)
 			<< "Failed to start camera " << camera->name();
@@ -823,6 +827,8 @@  void PipelineHandlerRkISP1::stop(Camera *camera)
 
 	data->timeline_.reset();
 
+	freeBuffers(camera);
+
 	activeCamera_ = nullptr;
 }
 
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 1eb809914f9aa0dc..d972d1bbdb113b95 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -71,11 +71,6 @@  public:
 			       unsigned int count) override;
 	void freeFrameBuffers(Camera *camera, Stream *stream) override;
 
-	int allocateBuffers(Camera *camera,
-			    const std::set<Stream *> &streams) override;
-	int freeBuffers(Camera *camera,
-			const std::set<Stream *> &streams) override;
-
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
@@ -223,18 +218,6 @@  void PipelineHandlerUVC::freeFrameBuffers(Camera *camera, Stream *stream)
 	data->video_->releaseBuffers();
 }
 
-int PipelineHandlerUVC::allocateBuffers(Camera *camera,
-					const std::set<Stream *> &streams)
-{
-	return 0;
-}
-
-int PipelineHandlerUVC::freeBuffers(Camera *camera,
-				    const std::set<Stream *> &streams)
-{
-	return 0;
-}
-
 int PipelineHandlerUVC::start(Camera *camera)
 {
 	UVCCameraData *data = cameraData(camera);
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index 13a7fc6cb5294eec..215d50a70256773e 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -89,11 +89,6 @@  public:
 			       unsigned int count) override;
 	void freeFrameBuffers(Camera *camera, Stream *stream) override;
 
-	int allocateBuffers(Camera *camera,
-			    const std::set<Stream *> &streams) override;
-	int freeBuffers(Camera *camera,
-			const std::set<Stream *> &streams) override;
-
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
@@ -290,18 +285,6 @@  void PipelineHandlerVimc::freeFrameBuffers(Camera *camera, Stream *stream)
 	data->video_->releaseBuffers();
 }
 
-int PipelineHandlerVimc::allocateBuffers(Camera *camera,
-					 const std::set<Stream *> &streams)
-{
-	return 0;
-}
-
-int PipelineHandlerVimc::freeBuffers(Camera *camera,
-				     const std::set<Stream *> &streams)
-{
-	return 0;
-}
-
 int PipelineHandlerVimc::start(Camera *camera)
 {
 	VimcCameraData *data = cameraData(camera);
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 11286d31d4e54213..19c9b6fe6e7fb03e 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -349,35 +349,6 @@  const ControlInfoMap &PipelineHandler::controls(Camera *camera)
  * The only intended caller is the FrameBufferAllocator helper.
  */
 
-/**
- * \fn PipelineHandler::allocateBuffers()
- * \brief Allocate buffers for a stream
- * \param[in] camera The camera the \a stream belongs to
- * \param[in] streams The set of streams to allocate buffers for
- *
- * This method allocates buffers internally in the pipeline handler for each
- * stream in the \a streams buffer set, and associates them with the stream's
- * buffer pool.
- *
- * The intended caller of this method is the Camera class.
- *
- * \return 0 on success or a negative error code otherwise
- */
-
-/**
- * \fn PipelineHandler::freeBuffers()
- * \brief Free all buffers associated with a stream
- * \param[in] camera The camera the \a stream belongs to
- * \param[in] streams The set of streams to free buffers from
- *
- * After a capture session has been stopped all buffers associated with each
- * stream shall be freed.
- *
- * The intended caller of this method is the Camera class.
- *
- * \return 0 on success or a negative error code otherwise
- */
-
 /**
  * \fn PipelineHandler::start()
  * \brief Start capturing from a group of streams