libcamera: pipelines: Align bookkeeping in queueRequest()

Expecting pipeline handler implementations of queueRequest() to call
the base class queueRequest() at the correct point have led to different
behaviors between the pipelines.

Fix this by splitting queueRequest() into a base class implementation
which handles the bookkeeping and a new queueRequestDevice() that is
to be implemented by pipeline handlers and only deals with committing the
request to the device.

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

src/libcamera/include/pipeline_handler.h

@@ -77,7 +77,7 @@ public:
 	virtual int start(Camera *camera) = 0;
 	virtual void stop(Camera *camera) = 0;
 
-	virtual int queueRequest(Camera *camera, Request *request);
+	int queueRequest(Camera *camera, Request *request);
 
 	bool completeBuffer(Camera *camera, Request *request, Buffer *buffer);
 	void completeRequest(Camera *camera, Request *request);
@@ -89,6 +89,8 @@ protected:
 			    std::unique_ptr<CameraData> data);
 	void hotplugMediaDevice(MediaDevice *media);
 
+	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
+
 	CameraData *cameraData(const Camera *camera);
 
 	CameraManager *manager_;

src/libcamera/pipeline/ipu3/ipu3.cpp

@@ -220,7 +220,7 @@ public:
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
-	int queueRequest(Camera *camera, Request *request) override;
+	int queueRequestDevice(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator) override;
 
@@ -756,7 +756,7 @@ void PipelineHandlerIPU3::stop(Camera *camera)
 	data->rawBuffers_.clear();
 }
 
-int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
+int PipelineHandlerIPU3::queueRequestDevice(Camera *camera, Request *request)
 {
 	int error = 0;
 
@@ -769,8 +769,6 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
 			error = ret;
 	}
 
-	PipelineHandler::queueRequest(camera, request);
-
 	return error;
 }
 

src/libcamera/pipeline/rkisp1/rkisp1.cpp

@@ -184,7 +184,7 @@ public:
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
-	int queueRequest(Camera *camera, Request *request) override;
+	int queueRequestDevice(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator) override;
 
@@ -810,13 +810,12 @@ void PipelineHandlerRkISP1::stop(Camera *camera)
 	activeCamera_ = nullptr;
 }
 
-int PipelineHandlerRkISP1::queueRequest(Camera *camera, Request *request)
+int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera,
+					      Request *request)
 {
 	RkISP1CameraData *data = cameraData(camera);
 	Stream *stream = &data->stream_;
 
-	PipelineHandler::queueRequest(camera, request);
-
 	RkISP1FrameInfo *info = data->frameInfo_.create(data->frame_, request,
 							stream);
 	if (!info)

src/libcamera/pipeline/uvcvideo.cpp

@@ -72,7 +72,7 @@ public:
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
-	int queueRequest(Camera *camera, Request *request) override;
+	int queueRequestDevice(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator) override;
 
@@ -262,7 +262,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 	return ret;
 }
 
-int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
+int PipelineHandlerUVC::queueRequestDevice(Camera *camera, Request *request)
 {
 	UVCCameraData *data = cameraData(camera);
 	Buffer *buffer = request->findBuffer(&data->stream_);
@@ -281,8 +281,6 @@ int PipelineHandlerUVC::queueRequest(Camera *camera, Request *request)
 	if (ret < 0)
 		return ret;
 
-	PipelineHandler::queueRequest(camera, request);
-
 	return 0;
 }
 

src/libcamera/pipeline/vimc.cpp

@@ -90,7 +90,7 @@ public:
 	int start(Camera *camera) override;
 	void stop(Camera *camera) override;
 
-	int queueRequest(Camera *camera, Request *request) override;
+	int queueRequestDevice(Camera *camera, Request *request) override;
 
 	bool match(DeviceEnumerator *enumerator) override;
 
@@ -323,7 +323,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 	return ret;
 }
 
-int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
+int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)
 {
 	VimcCameraData *data = cameraData(camera);
 	Buffer *buffer = request->findBuffer(&data->stream_);
@@ -342,8 +342,6 @@ int PipelineHandlerVimc::queueRequest(Camera *camera, Request *request)
 	if (ret < 0)
 		return ret;
 
-	PipelineHandler::queueRequest(camera, request);
-
 	return 0;
 }
 

src/libcamera/pipeline_handler.cpp

@@ -345,16 +345,12 @@ const ControlInfoMap &PipelineHandler::controls(Camera *camera)
  * \param[in] request The request to queue
  *
  * This method queues a capture request to the pipeline handler for processing.
- * The request contains a set of buffers associated with streams and a set of
+ * The request is first added to the internal list of queued requests, and
- * parameters. The pipeline handler shall program the device to ensure that the
+ * then passed to the pipeline handler with a call to queueRequestDevice().
- * parameters will be applied to the frames captured in the buffers provided in
- * the request.
  *
- * Pipeline handlers shall override this method. The base implementation in the
+ * Keeping track of queued requests ensures automatic completion of all requests
- * PipelineHandler class keeps track of queued requests in order to ensure
+ * when the pipeline handler is stopped with stop(). Request completion shall be
- * completion of all requests when the pipeline handler is stopped with stop().
+ * signalled by the pipeline handler using the completeRequest() method.
- * Requests completion shall be signalled by the pipeline handler using the
- * completeRequest() method.
  *
  * \return 0 on success or a negative error code otherwise
  */
@@ -363,9 +359,28 @@ int PipelineHandler::queueRequest(Camera *camera, Request *request)
 	CameraData *data = cameraData(camera);
 	data->queuedRequests_.push_back(request);
 
-	return 0;
+	int ret = queueRequestDevice(camera, request);
+	if (ret)
+		data->queuedRequests_.remove(request);
+
+	return ret;
 }
 
+/**
+ * \fn PipelineHandler::queueRequestDevice()
+ * \brief Queue a request to the device
+ * \param[in] camera The camera to queue the request to
+ * \param[in] request The request to queue
+ *
+ * This method queues a capture request to the device for processing. The
+ * request contains a set of buffers associated with streams and a set of
+ * parameters. The pipeline handler shall program the device to ensure that the
+ * parameters will be applied to the frames captured in the buffers provided in
+ * the request.
+ *
+ * \return 0 on success or a negative error code otherwise
+ */
+
 /**
  * \brief Complete a buffer for a request
  * \param[in] camera The camera the request belongs to