libcamera: framebuffer_allocator: Lift camera restrictions on allocator

The Camera class currently requires the allocator to have no allocated
buffer before the camera is reconfigured, and the allocator to be
destroyed before the camera is released. There's no basis for these
restrictions anymore, remove them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

10 files changed, 8 insertions, 54 deletions

diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
index a5e2b49f..9c0e58f7 100644
--- a/include/libcamera/camera.h
+++ b/include/libcamera/camera.h
@@ -113,8 +113,6 @@ private:
 	friend class FrameBufferAllocator;
 	int exportFrameBuffers(Stream *stream,
 			       std::vector<std::unique_ptr<FrameBuffer>> *buffers);
-	/* \todo Remove allocator_ from the exposed API */
-	FrameBufferAllocator *allocator_;
 };
 
 } /* namespace libcamera */
diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
index 42812253..78f13539 100644
--- a/include/libcamera/framebuffer_allocator.h
+++ b/include/libcamera/framebuffer_allocator.h
@@ -20,8 +20,7 @@ class Stream;
 class FrameBufferAllocator
 {
 public:
-	static FrameBufferAllocator *create(std::shared_ptr<Camera> camera);
-
+	FrameBufferAllocator(std::shared_ptr<Camera> camera);
 	FrameBufferAllocator(const Camera &) = delete;
 	FrameBufferAllocator &operator=(const Camera &) = delete;
 
@@ -34,8 +33,6 @@ public:
 	const std::vector<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;
 
 private:
-	FrameBufferAllocator(std::shared_ptr<Camera> camera);
-
 	std::shared_ptr<Camera> camera_;
 	std::map<Stream *, std::vector<std::unique_ptr<FrameBuffer>>> buffers_;
 };
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
index 6cbdc98c..55fa2dab 100644
--- a/src/cam/capture.cpp
+++ b/src/cam/capture.cpp
@@ -52,7 +52,7 @@ int Capture::run(EventLoop *loop, const OptionsParser::Options &options)
 	}
 
 
-	FrameBufferAllocator *allocator = FrameBufferAllocator::create(camera_);
+	FrameBufferAllocator *allocator = new FrameBufferAllocator(camera_);
 
 	ret = capture(loop, allocator);
 
diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp
index d0b90eca..1d5959c0 100644
--- a/src/gstreamer/gstlibcameraallocator.cpp
+++ b/src/gstreamer/gstlibcameraallocator.cpp
@@ -188,7 +188,7 @@ gst_libcamera_allocator_new(std::shared_ptr<Camera> camera)
 	auto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,
 							  nullptr));
 
-	self->fb_allocator = FrameBufferAllocator::create(camera);
+	self->fb_allocator = new FrameBufferAllocator(camera);
 	for (Stream *stream : camera->streams()) {
 		gint ret;
 
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index 5593c1b3..8c3bb2c2 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -508,7 +508,7 @@ const std::string &Camera::name() const
 
 Camera::Camera(PipelineHandler *pipe, const std::string &name,
 	       const std::set<Stream *> &streams)
-	: p_(new Private(pipe, name, streams)), allocator_(nullptr)
+	: p_(new Private(pipe, name, streams))
 {
 }
 
@@ -620,16 +620,6 @@ int Camera::release()
 	if (ret < 0)
 		return ret == -EACCES ? -EBUSY : ret;
 
-	if (allocator_) {
-		/*
-		 * \todo Try to find a better API that would make this error
-		 * impossible.
-		 */
-		LOG(Camera, Error)
-			<< "Buffers must be freed before the camera can be reconfigured";
-		return -EBUSY;
-	}
-
 	p_->pipe_->unlock();
 
 	p_->setState(Private::CameraAvailable);
@@ -763,12 +753,6 @@ int Camera::configure(CameraConfiguration *config)
 	if (ret < 0)
 		return ret;
 
-	if (allocator_ && allocator_->allocated()) {
-		LOG(Camera, Error)
-			<< "Allocator must be deleted before camera can be reconfigured";
-		return -EBUSY;
-	}
-
 	if (config->validate() != CameraConfiguration::Valid) {
 		LOG(Camera, Error)
 			<< "Can't configure camera with invalid configuration";
diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
index 6f7a2e90..a37b564c 100644
--- a/src/libcamera/framebuffer_allocator.cpp
+++ b/src/libcamera/framebuffer_allocator.cpp
@@ -54,29 +54,6 @@ LOG_DEFINE_CATEGORY(Allocator)
  */
 
 /**
- * \brief Create a FrameBuffer allocator
- * \param[in] camera The camera the allocator serves
- *
- * A single allocator may be created for a Camera instance.
- *
- * The caller is responsible for deleting the allocator before the camera is
- * released.
- *
- * \return A pointer to the newly created allocator object or nullptr on error
- */
-FrameBufferAllocator *
-FrameBufferAllocator::create(std::shared_ptr<Camera> camera)
-{
-	if (camera->allocator_) {
-		LOG(Allocator, Error) << "Camera already has an allocator";
-		return nullptr;
-	}
-
-	camera->allocator_ = new FrameBufferAllocator(camera);
-	return camera->allocator_;
-}
-
-/**
  * \brief Construct a FrameBufferAllocator serving a camera
  * \param[in] camera The camera
  */
@@ -88,8 +65,6 @@ FrameBufferAllocator::FrameBufferAllocator(std::shared_ptr<Camera> camera)
 FrameBufferAllocator::~FrameBufferAllocator()
 {
 	buffers_.clear();
-
-	camera_->allocator_ = nullptr;
 }
 
 /**
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index ae1760df..47d37c3e 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -240,7 +240,7 @@ int MainWindow::startCapture()
 
 	adjustSize();
 
-	allocator_ = FrameBufferAllocator::create(camera_);
+	allocator_ = new FrameBufferAllocator(camera_);
 	ret = allocator_->allocate(stream);
 	if (ret < 0) {
 		std::cerr << "Failed to allocate capture buffers" << std::endl;
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index b1463559..ecbb70ac 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -40,7 +40,7 @@ int V4L2Camera::open()
 		return -EINVAL;
 	}
 
-	bufferAllocator_ = FrameBufferAllocator::create(camera_);
+	bufferAllocator_ = new FrameBufferAllocator(camera_);
 
 	return 0;
 }
diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
index b304d59c..f6b2f348 100644
--- a/test/camera/capture.cpp
+++ b/test/camera/capture.cpp
@@ -63,7 +63,7 @@ protected:
 			return TestFail;
 		}
 
-		allocator_ = FrameBufferAllocator::create(camera_);
+		allocator_ = new FrameBufferAllocator(camera_);
 
 		return TestPass;
 	}
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index 20541b3e..325b4674 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -117,7 +117,7 @@ protected:
 			return TestFail;
 
 		/* Use internally allocated buffers. */
-		allocator_ = FrameBufferAllocator::create(camera_);
+		allocator_ = new FrameBufferAllocator(camera_);
 		Stream *stream = *camera_->streams().begin();
 		if (allocator_->allocate(stream) < 0)
 			return TestFail;