[libcamera-devel] [PATCH v3] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
Niklas Söderlund
niklas.soderlund at ragnatech.se
Wed Sep 30 12:39:11 CEST 2020
Hi Paul,
Thanks for your work.
On 2020-09-30 19:17:17 +0900, Paul Elder wrote:
> Allow reuse of the Request object by implementing reset(),
> clearBuffers(), and reAddBuffers(). This means that the applications
> now have the responsibility of freeing the Request objects, so make all
> libcamera users (cam, qcam, v4l2-compat, gstreamer, android) do so.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v3:
> - reset() is called in Camera::queueRequest()
> - apps that use transient request (android, gstreamer) delete the
> request at the end of the completion handler
> - apps that want to reuse the buffers (cam) use reAddBuffers()
> - apps that need to change the buffers (qcam, v4l2) use clearBuffers()
> - update the documentation accordingly
>
> Changes in v2:
> - clear controls_ and metadata_ and validator_ in Request::reset()
> - use unique_ptr on application side, prior to queueRequest, and use
> regular pointer for completion handler
> - make qcam's reuse request nicer
> - update Camera::queueRequest() and Camera::createRequest() documentation
> - add documentation for Request::reset()
> - make v4l2-compat reuse request
> - make gstreamer and android use the new createRequest API, though they
> do not actually reuse the requests
> ---
> include/libcamera/camera.h | 2 +-
> include/libcamera/request.h | 4 ++
> src/android/camera_device.cpp | 8 +++-
> src/cam/capture.cpp | 29 ++++----------
> src/cam/capture.h | 3 ++
> src/gstreamer/gstlibcamerasrc.cpp | 6 ++-
> src/libcamera/camera.cpp | 19 ++++-----
> src/libcamera/request.cpp | 64 +++++++++++++++++++++++++++++++
> src/qcam/main_window.cpp | 48 ++++++++++++++---------
> src/qcam/main_window.h | 26 ++++---------
> src/v4l2/v4l2_camera.cpp | 38 ++++++++++++------
> src/v4l2/v4l2_camera.h | 4 +-
> test/camera/buffer_import.cpp | 15 ++++----
> test/camera/capture.cpp | 15 ++++----
> test/camera/statemachine.cpp | 9 ++---
> 15 files changed, 186 insertions(+), 104 deletions(-)
>
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 272c12c3..e4036b63 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -93,7 +93,7 @@ public:
> std::unique_ptr<CameraConfiguration> generateConfiguration(const StreamRoles &roles = {});
> int configure(CameraConfiguration *config);
>
> - Request *createRequest(uint64_t cookie = 0);
> + std::unique_ptr<Request> createRequest(uint64_t cookie = 0);
> int queueRequest(Request *request);
>
> int start();
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 5976ac50..6a651f36 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -38,6 +38,10 @@ public:
> Request &operator=(const Request &) = delete;
> ~Request();
>
> + void reset();
> + void clearBuffers();
> + void reAddBuffers();
> +
> ControlList &controls() { return *controls_; }
> ControlList &metadata() { return *metadata_; }
> const BufferMap &buffers() const { return bufferMap_; }
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 70d77a17..7f288617 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1395,8 +1395,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> new Camera3RequestDescriptor(camera3Request->frame_number,
> camera3Request->num_output_buffers);
>
> - Request *request =
> + std::unique_ptr<Request> r =
> camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> + Request *request = r.release();
> + if (!request) {
> + LOG(HAL, Error) << "Failed to create request";
> + return -ENOMEM;
> + }
>
> for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> CameraStream *cameraStream =
> @@ -1593,6 +1598,7 @@ void CameraDevice::requestComplete(Request *request)
> callbacks_->process_capture_result(callbacks_, &captureResult);
>
> delete descriptor;
> + delete request;
> }
>
> std::string CameraDevice::logPrefix() const
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index 5510c009..5b223365 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -65,6 +65,8 @@ int Capture::run(const OptionsParser::Options &options)
> writer_ = nullptr;
> }
>
> + requests_.clear();
> +
> delete allocator;
>
> return ret;
> @@ -92,9 +94,8 @@ int Capture::capture(FrameBufferAllocator *allocator)
> * example pushing a button. For now run all streams all the time.
> */
>
> - std::vector<Request *> requests;
> for (unsigned int i = 0; i < nbuffers; i++) {
> - Request *request = camera_->createRequest();
> + std::unique_ptr<Request> request = camera_->createRequest();
> if (!request) {
> std::cerr << "Can't create request" << std::endl;
> return -ENOMEM;
> @@ -117,7 +118,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
> writer_->mapBuffer(buffer.get());
> }
>
> - requests.push_back(request);
> + requests_.push_back(std::move(request));
> }
>
> ret = camera_->start();
> @@ -126,8 +127,8 @@ int Capture::capture(FrameBufferAllocator *allocator)
> return ret;
> }
>
> - for (Request *request : requests) {
> - ret = camera_->queueRequest(request);
> + for (std::unique_ptr<Request> &request : requests_) {
> + ret = camera_->queueRequest(request.get());
> if (ret < 0) {
> std::cerr << "Can't queue request" << std::endl;
> camera_->stop();
> @@ -202,22 +203,6 @@ void Capture::requestComplete(Request *request)
> return;
> }
>
> - /*
> - * Create a new request and populate it with one buffer for each
> - * stream.
> - */
> - request = camera_->createRequest();
> - if (!request) {
> - std::cerr << "Can't create request" << std::endl;
> - return;
> - }
> -
> - for (auto it = buffers.begin(); it != buffers.end(); ++it) {
> - const Stream *stream = it->first;
> - FrameBuffer *buffer = it->second;
> -
> - request->addBuffer(stream, buffer);
> - }
> -
> + request->reAddBuffers();
> camera_->queueRequest(request);
> }
> diff --git a/src/cam/capture.h b/src/cam/capture.h
> index 0aebdac9..45e5e8a9 100644
> --- a/src/cam/capture.h
> +++ b/src/cam/capture.h
> @@ -9,6 +9,7 @@
>
> #include <memory>
> #include <stdint.h>
> +#include <vector>
>
> #include <libcamera/buffer.h>
> #include <libcamera/camera.h>
> @@ -43,6 +44,8 @@ private:
> EventLoop *loop_;
> unsigned int captureCount_;
> unsigned int captureLimit_;
> +
> + std::vector<std::unique_ptr<libcamera::Request>> requests_;
> };
>
> #endif /* __CAM_CAPTURE_H__ */
> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
> index 1bfc2e2f..ca223df6 100644
> --- a/src/gstreamer/gstlibcamerasrc.cpp
> +++ b/src/gstreamer/gstlibcamerasrc.cpp
> @@ -74,6 +74,8 @@ RequestWrap::~RequestWrap()
> if (item.second)
> gst_buffer_unref(item.second);
> }
> +
> + delete request_;
> }
>
> void RequestWrap::attachBuffer(GstBuffer *buffer)
> @@ -266,7 +268,8 @@ gst_libcamera_src_task_run(gpointer user_data)
> GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
> GstLibcameraSrcState *state = self->state;
>
> - Request *request = state->cam_->createRequest();
> + std::unique_ptr<Request> request_unique = state->cam_->createRequest();
> + Request *request = request_unique.release();
> auto wrap = std::make_unique<RequestWrap>(request);
> for (GstPad *srcpad : state->srcpads_) {
> GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);
> @@ -281,7 +284,6 @@ gst_libcamera_src_task_run(gpointer user_data)
> * queueing this one due to lack of buffers.
> */
> delete request;
> - request = nullptr;
> break;
> }
>
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index ae16a64a..d4c18c72 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -833,21 +833,25 @@ int Camera::configure(CameraConfiguration *config)
> * handler, and is completely opaque to libcamera.
> *
> * The ownership of the returned request is passed to the caller, which is
> - * responsible for either queueing the request or deleting it.
> + * responsible for either queueing the request or deleting it. The caller
> + * continues to own the request after queueing it, so the application is
> + * responsible for deleting the request after the request completes.
> + * Alternatively, the request may be reused after clearing its buffers via
> + * Request::clearBuffers().
> *
> * \context This function is \threadsafe. It may only be called when the camera
> * is in the Configured or Running state as defined in \ref camera_operation.
> *
> * \return A pointer to the newly created request, or nullptr on error
> */
> -Request *Camera::createRequest(uint64_t cookie)
> +std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
> {
> int ret = p_->isAccessAllowed(Private::CameraConfigured,
> Private::CameraRunning);
> if (ret < 0)
> return nullptr;
>
> - return new Request(this, cookie);
> + return std::make_unique<Request>(this, cookie);
> }
>
> /**
> @@ -863,9 +867,6 @@ Request *Camera::createRequest(uint64_t cookie)
> * Once the request has been queued, the camera will notify its completion
> * through the \ref requestCompleted signal.
> *
> - * Ownership of the request is transferred to the camera. It will be deleted
> - * automatically after it completes.
> - *
> * \context This function is \threadsafe. It may only be called when the camera
> * is in the Running state as defined in \ref camera_operation.
> *
> @@ -877,6 +878,8 @@ Request *Camera::createRequest(uint64_t cookie)
> */
> int Camera::queueRequest(Request *request)
> {
> + request->reset();
> +
> int ret = p_->isAccessAllowed(Private::CameraRunning);
> if (ret < 0)
> return ret;
> @@ -974,13 +977,11 @@ int Camera::stop()
> * \param[in] request The request that has completed
> *
> * This function is called by the pipeline handler to notify the camera that
> - * the request has completed. It emits the requestCompleted signal and deletes
> - * the request.
> + * the request has completed. It emits the requestCompleted signal.
> */
> void Camera::requestComplete(Request *request)
> {
> requestCompleted.emit(request);
> - delete request;
> }
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 60b30692..7d51e504 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -85,6 +85,70 @@ Request::~Request()
> delete validator_;
> }
>
> +/*
> + * \brief Reset the request
> + *
> + * Reset the status and controls associated with the request, to allow it to
> + * be reused and requeued without destruction. This function is meant to be
> + * called by Camera::queueRequest().
> + *
> + * \todo Make this private and make Camera a friend class?
> + */
> +void Request::reset()
> +{
> + status_ = RequestPending;
> + cancelled_ = false;
> +
> + delete metadata_;
> + delete controls_;
> + delete validator_;
> +
> + validator_ = new CameraControlValidator(camera_);
> + controls_ = new ControlList(controls::controls, validator_);
> + metadata_ = new ControlList(controls::controls);
> +}
> +
> +/*
> + * \brief Clear buffers associated with the request
> + *
> + * Delete the pointers to the FrameBuffers that were added to this request via
> + * addBuffers().
> + *
> + * This function should be called in (or after) the request completion handler
> + * in applications prior to modifying the buffers and requeueing the request to
> + * the camera. If the buffers do not need to be modified, and are to be reused,
> + * use reAddBuffers(). If the request is meant to be transient, then it
> + * should be deleted at the end of the completion handler.
> + */
> +void Request::clearBuffers()
> +{
> + pending_.clear();
> +
> + bufferMap_.clear();
> +}
> +
> +/*
> + * \brief Re-add the buffers that were added to the request
> + *
> + * Re-add the FrameBuffers that were added to this request via addBuffers().
> + *
> + * This function should be called in (or after) the request completion handler
> + * in applications that wish to reuse the buffers that were added to the request
> + * when it was queued to the camera. If the buffers need to be modified, they
> + * should be cleared with clearBuffers() first instead. If the request is
> + * meant to be transient, then it should be deleted at the end of the
> + * completion handler.
> + */
> +void Request::reAddBuffers()
> +{
> + pending_.clear();
> +
> + for (auto pair : bufferMap_) {
> + pair.second->request_ = this;
> + pending_.insert(pair.second);
> + }
> +}
I wonder would it not be neat to call this unconditionally in
PipelineHandler::completeRequest()?
An application wishing to reuse a request can then simply just requeue
it without touching it. While an application wishing to modify the
request before reusing it will be responsible for removeAllBuffers() or
removeBuffer(Stream *) the Request to fit it's new usage.
> +
> /**
> * \fn Request::controls()
> * \brief Retrieve the request's ControlList
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 985743f3..466f3053 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -367,7 +367,6 @@ void MainWindow::toggleCapture(bool start)
> int MainWindow::startCapture()
> {
> StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
> - std::vector<Request *> requests;
> int ret;
>
> /* Verify roles are supported. */
> @@ -486,7 +485,7 @@ int MainWindow::startCapture()
> while (!freeBuffers_[vfStream_].isEmpty()) {
> FrameBuffer *buffer = freeBuffers_[vfStream_].dequeue();
>
> - Request *request = camera_->createRequest();
> + std::unique_ptr<Request> request = camera_->createRequest();
> if (!request) {
> qWarning() << "Can't create request";
> ret = -ENOMEM;
> @@ -499,7 +498,7 @@ int MainWindow::startCapture()
> goto error;
> }
>
> - requests.push_back(request);
> + requests_.push_back(std::move(request));
> }
>
> /* Start the title timer and the camera. */
> @@ -518,8 +517,8 @@ int MainWindow::startCapture()
> camera_->requestCompleted.connect(this, &MainWindow::requestComplete);
>
> /* Queue all requests. */
> - for (Request *request : requests) {
> - ret = camera_->queueRequest(request);
> + for (std::unique_ptr<Request> &request : requests_) {
> + ret = camera_->queueRequest(request.get());
> if (ret < 0) {
> qWarning() << "Can't queue request";
> goto error_disconnect;
> @@ -535,8 +534,7 @@ error_disconnect:
> camera_->stop();
>
> error:
> - for (Request *request : requests)
> - delete request;
> + requests_.clear();
>
> for (auto &iter : mappedBuffers_) {
> const MappedBuffer &buffer = iter.second;
> @@ -580,6 +578,8 @@ void MainWindow::stopCapture()
> }
> mappedBuffers_.clear();
>
> + requests_.clear();
> +
> delete allocator_;
>
> isCapturing_ = false;
> @@ -701,7 +701,7 @@ void MainWindow::requestComplete(Request *request)
> */
> {
> QMutexLocker locker(&mutex_);
> - doneQueue_.enqueue({ request->buffers(), request->metadata() });
> + doneQueue_.enqueue(request);
> }
>
> QCoreApplication::postEvent(this, new CaptureEvent);
> @@ -714,8 +714,7 @@ void MainWindow::processCapture()
> * if stopCapture() has been called while a CaptureEvent was posted but
> * not processed yet. Return immediately in that case.
> */
> - CaptureRequest request;
> -
> + Request *request;
> {
> QMutexLocker locker(&mutex_);
> if (doneQueue_.isEmpty())
> @@ -725,11 +724,19 @@ void MainWindow::processCapture()
> }
>
> /* Process buffers. */
> - if (request.buffers_.count(vfStream_))
> - processViewfinder(request.buffers_[vfStream_]);
> + if (request->buffers().count(vfStream_)) {
> + {
> + QMutexLocker locker(&mutex_);
> + freeQueue_.enqueue(request);
> + }
> + Request::BufferMap buffers = request->buffers();
> + processViewfinder(buffers[vfStream_]);
> + }
>
> - if (request.buffers_.count(rawStream_))
> - processRaw(request.buffers_[rawStream_], request.metadata_);
> + if (request->buffers().count(rawStream_)) {
> + Request::BufferMap buffers = request->buffers();
> + processRaw(buffers[rawStream_], request->metadata());
> + }
> }
>
> void MainWindow::processViewfinder(FrameBuffer *buffer)
> @@ -754,12 +761,17 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>
> void MainWindow::queueRequest(FrameBuffer *buffer)
> {
> - Request *request = camera_->createRequest();
> - if (!request) {
> - qWarning() << "Can't create request";
> - return;
> + Request *request;
> + {
> + QMutexLocker locker(&mutex_);
> + if (freeQueue_.isEmpty())
> + return;
> +
> + request = freeQueue_.dequeue();
> }
>
> + request->clearBuffers();
> +
> request->addBuffer(vfStream_, buffer);
>
> if (captureRaw_) {
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 5c61a4df..64bcfebc 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -8,6 +8,7 @@
> #define __QCAM_MAIN_WINDOW_H__
>
> #include <memory>
> +#include <vector>
>
> #include <QElapsedTimer>
> #include <QIcon>
> @@ -22,6 +23,7 @@
> #include <libcamera/camera_manager.h>
> #include <libcamera/controls.h>
> #include <libcamera/framebuffer_allocator.h>
> +#include <libcamera/request.h>
> #include <libcamera/stream.h>
>
> #include "../cam/stream_options.h"
> @@ -41,23 +43,6 @@ enum {
> OptStream = 's',
> };
>
> -class CaptureRequest
> -{
> -public:
> - CaptureRequest()
> - {
> - }
> -
> - CaptureRequest(const Request::BufferMap &buffers,
> - const ControlList &metadata)
> - : buffers_(buffers), metadata_(metadata)
> - {
> - }
> -
> - Request::BufferMap buffers_;
> - ControlList metadata_;
> -};
> -
> class MainWindow : public QMainWindow
> {
> Q_OBJECT
> @@ -128,13 +113,16 @@ private:
> Stream *vfStream_;
> Stream *rawStream_;
> std::map<const Stream *, QQueue<FrameBuffer *>> freeBuffers_;
> - QQueue<CaptureRequest> doneQueue_;
> - QMutex mutex_; /* Protects freeBuffers_ and doneQueue_ */
> + QQueue<Request *> doneQueue_;
> + QQueue<Request *> freeQueue_;
> + QMutex mutex_; /* Protects freeBuffers_, doneQueue_, and freeQueue_ */
>
> uint64_t lastBufferTime_;
> QElapsedTimer frameRateInterval_;
> uint32_t previousFrames_;
> uint32_t framesCaptured_;
> +
> + std::vector<std::unique_ptr<Request>> requests_;
> };
>
> #endif /* __QCAM_MAIN_WINDOW__ */
> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
> index 3565f369..5a9701ff 100644
> --- a/src/v4l2/v4l2_camera.cpp
> +++ b/src/v4l2/v4l2_camera.cpp
> @@ -49,6 +49,8 @@ int V4L2Camera::open(StreamConfiguration *streamConfig)
>
> void V4L2Camera::close()
> {
> + requestPool_.clear();
> +
> delete bufferAllocator_;
> bufferAllocator_ = nullptr;
>
> @@ -154,16 +156,30 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
> return 0;
> }
>
> -int V4L2Camera::allocBuffers([[maybe_unused]] unsigned int count)
> +int V4L2Camera::allocBuffers(unsigned int count)
> {
> Stream *stream = config_->at(0).stream();
>
> - return bufferAllocator_->allocate(stream);
> + int ret = bufferAllocator_->allocate(stream);
> + if (ret < 0)
> + return ret;
> +
> + for (unsigned int i = 0; i < count; i++) {
> + std::unique_ptr<Request> request = camera_->createRequest(i);
> + if (!request) {
> + requestPool_.clear();
> + return -ENOMEM;
> + }
> + requestPool_.push_back(std::move(request));
> + }
> +
> + return ret;
> }
>
> void V4L2Camera::freeBuffers()
> {
> pendingRequests_.clear();
> + requestPool_.clear();
>
> Stream *stream = config_->at(0).stream();
> bufferAllocator_->free(stream);
> @@ -192,9 +208,9 @@ int V4L2Camera::streamOn()
>
> isRunning_ = true;
>
> - for (std::unique_ptr<Request> &req : pendingRequests_) {
> + for (Request *req : pendingRequests_) {
> /* \todo What should we do if this returns -EINVAL? */
> - ret = camera_->queueRequest(req.release());
> + ret = camera_->queueRequest(req);
> if (ret < 0)
> return ret == -EACCES ? -EBUSY : ret;
> }
> @@ -226,12 +242,12 @@ int V4L2Camera::streamOff()
>
> int V4L2Camera::qbuf(unsigned int index)
> {
> - std::unique_ptr<Request> request =
> - std::unique_ptr<Request>(camera_->createRequest(index));
> - if (!request) {
> - LOG(V4L2Compat, Error) << "Can't create request";
> - return -ENOMEM;
> + if (index >= requestPool_.size()) {
> + LOG(V4L2Compat, Error) << "Invalid index";
> + return -EINVAL;
> }
> + Request *request = requestPool_[index].get();
> + request->clearBuffers();
>
> Stream *stream = config_->at(0).stream();
> FrameBuffer *buffer = bufferAllocator_->buffers(stream)[index].get();
> @@ -242,11 +258,11 @@ int V4L2Camera::qbuf(unsigned int index)
> }
>
> if (!isRunning_) {
> - pendingRequests_.push_back(std::move(request));
> + pendingRequests_.push_back(request);
> return 0;
> }
>
> - ret = camera_->queueRequest(request.release());
> + ret = camera_->queueRequest(request);
> if (ret < 0) {
> LOG(V4L2Compat, Error) << "Can't queue request";
> return ret == -EACCES ? -EBUSY : ret;
> diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
> index 1fc5ebef..a6c35a2e 100644
> --- a/src/v4l2/v4l2_camera.h
> +++ b/src/v4l2/v4l2_camera.h
> @@ -76,7 +76,9 @@ private:
> std::mutex bufferLock_;
> FrameBufferAllocator *bufferAllocator_;
>
> - std::deque<std::unique_ptr<Request>> pendingRequests_;
> + std::vector<std::unique_ptr<Request>> requestPool_;
> +
> + std::deque<Request *> pendingRequests_;
> std::deque<std::unique_ptr<Buffer>> completedBuffers_;
>
> int efd_;
> diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp
> index 64e96264..b4e8a6d8 100644
> --- a/test/camera/buffer_import.cpp
> +++ b/test/camera/buffer_import.cpp
> @@ -50,7 +50,7 @@ protected:
> if (request->status() != Request::RequestComplete)
> return;
>
> - const Request::BufferMap &buffers = request->buffers();
> + const Request::BufferMap buffers = request->buffers();
>
> completeRequestsCount_++;
>
> @@ -58,7 +58,7 @@ protected:
> const Stream *stream = buffers.begin()->first;
> FrameBuffer *buffer = buffers.begin()->second;
>
> - request = camera_->createRequest();
> + request->clearBuffers();
> request->addBuffer(stream, buffer);
> camera_->queueRequest(request);
> }
> @@ -98,9 +98,8 @@ protected:
> if (ret != TestPass)
> return ret;
>
> - std::vector<Request *> requests;
> for (const std::unique_ptr<FrameBuffer> &buffer : source.buffers()) {
> - Request *request = camera_->createRequest();
> + std::unique_ptr<Request> request = camera_->createRequest();
> if (!request) {
> std::cout << "Failed to create request" << std::endl;
> return TestFail;
> @@ -111,7 +110,7 @@ protected:
> return TestFail;
> }
>
> - requests.push_back(request);
> + requests_.push_back(std::move(request));
> }
>
> completeRequestsCount_ = 0;
> @@ -125,8 +124,8 @@ protected:
> return TestFail;
> }
>
> - for (Request *request : requests) {
> - if (camera_->queueRequest(request)) {
> + for (std::unique_ptr<Request> &request : requests_) {
> + if (camera_->queueRequest(request.get())) {
> std::cout << "Failed to queue request" << std::endl;
> return TestFail;
> }
> @@ -160,6 +159,8 @@ protected:
> }
>
> private:
> + std::vector<std::unique_ptr<Request>> requests_;
> +
> unsigned int completeBuffersCount_;
> unsigned int completeRequestsCount_;
> std::unique_ptr<CameraConfiguration> config_;
> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp
> index 51bbd258..0116481b 100644
> --- a/test/camera/capture.cpp
> +++ b/test/camera/capture.cpp
> @@ -44,7 +44,7 @@ protected:
> if (request->status() != Request::RequestComplete)
> return;
>
> - const Request::BufferMap &buffers = request->buffers();
> + const Request::BufferMap buffers = request->buffers();
>
> completeRequestsCount_++;
>
> @@ -52,7 +52,7 @@ protected:
> const Stream *stream = buffers.begin()->first;
> FrameBuffer *buffer = buffers.begin()->second;
>
> - request = camera_->createRequest();
> + request->clearBuffers();
> request->addBuffer(stream, buffer);
> camera_->queueRequest(request);
> }
> @@ -98,9 +98,8 @@ protected:
> if (ret < 0)
> return TestFail;
>
> - std::vector<Request *> requests;
> for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
> - Request *request = camera_->createRequest();
> + std::unique_ptr<Request> request = camera_->createRequest();
> if (!request) {
> cout << "Failed to create request" << endl;
> return TestFail;
> @@ -111,7 +110,7 @@ protected:
> return TestFail;
> }
>
> - requests.push_back(request);
> + requests_.push_back(std::move(request));
> }
>
> completeRequestsCount_ = 0;
> @@ -125,8 +124,8 @@ protected:
> return TestFail;
> }
>
> - for (Request *request : requests) {
> - if (camera_->queueRequest(request)) {
> + for (std::unique_ptr<Request> &request : requests_) {
> + if (camera_->queueRequest(request.get())) {
> cout << "Failed to queue request" << endl;
> return TestFail;
> }
> @@ -161,6 +160,8 @@ protected:
> return TestPass;
> }
>
> + std::vector<std::unique_ptr<Request>> requests_;
> +
> std::unique_ptr<CameraConfiguration> config_;
> FrameBufferAllocator *allocator_;
> };
> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
> index 28faeb91..e63ab298 100644
> --- a/test/camera/statemachine.cpp
> +++ b/test/camera/statemachine.cpp
> @@ -101,13 +101,10 @@ protected:
> return TestFail;
>
> /* Test operations which should pass. */
> - Request *request2 = camera_->createRequest();
> + std::unique_ptr<Request> request2 = camera_->createRequest();
> if (!request2)
> return TestFail;
>
> - /* Never handed to hardware so need to manually delete it. */
> - delete request2;
> -
> /* Test valid state transitions, end in Running state. */
> if (camera_->release())
> return TestFail;
> @@ -146,7 +143,7 @@ protected:
> return TestFail;
>
> /* Test operations which should pass. */
> - Request *request = camera_->createRequest();
> + std::unique_ptr<Request> request = camera_->createRequest();
> if (!request)
> return TestFail;
>
> @@ -154,7 +151,7 @@ protected:
> if (request->addBuffer(stream, allocator_->buffers(stream)[0].get()))
> return TestFail;
>
> - if (camera_->queueRequest(request))
> + if (camera_->queueRequest(request.get()))
> return TestFail;
>
> /* Test valid state transitions, end in Available state. */
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
--
Regards,
Niklas Söderlund
More information about the libcamera-devel
mailing list