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>

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 */

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_;
 };

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);
 

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;
 

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";

src/libcamera/framebuffer_allocator.cpp

@@ -53,29 +53,6 @@ LOG_DEFINE_CATEGORY(Allocator)
  * are provided externally applications shall not use this class.
  */
 
-/**
- * \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;
 }
 
 /**

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;

src/v4l2/v4l2_camera.cpp

@@ -40,7 +40,7 @@ int V4L2Camera::open()
 		return -EINVAL;
 	}
 
-	bufferAllocator_ = FrameBufferAllocator::create(camera_);
+	bufferAllocator_ = new FrameBufferAllocator(camera_);
 
 	return 0;
 }

test/camera/capture.cpp

@@ -63,7 +63,7 @@ protected:
 			return TestFail;
 		}
 
-		allocator_ = FrameBufferAllocator::create(camera_);
+		allocator_ = new FrameBufferAllocator(camera_);
 
 		return TestPass;
 	}

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;