[libcamera-devel] [PATCH v6] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Thu Oct 8 08:29:20 CEST 2020
- Previous message (by thread): [libcamera-devel] [PATCH v6] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
- Next message (by thread): [libcamera-devel] [PATCH v6] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
Hi Jacopo,
On Wed, Oct 07, 2020 at 11:43:31AM +0200, Jacopo Mondi wrote:
> Hi Paul,
>
> On Wed, Oct 07, 2020 at 04:34:18PM +0900, Paul Elder wrote:
> > Allow reuse of the Request object by implementing reuse(). This means
> > the applications now have the responsibility of freeing the Request
> > objects, so make all libcamera users (cam, qcam, v4l2-compat, gstreamer,
> > android) do so.
>
> Maybe not a question to ask at v6, I've gone through the previous
> iterations and I see the proposed API has been discussed already, so
> feel free to ignore, but: I'm not sure I like the flags passed to
> reuse() :)
>
> To be more precise, I don't see much much value in ReuseBuffers.
>
> How usual is it that the same [Stream|buffer] pair is repeated ? I
> understand we might want to capture from the same set of Streams, but
> application would more likely cycle buffers, don't they ?
It depends on the application. In cam we have:
- /*
- * 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->reuse(Request::ReuseBuffers);
Without ReuseBuffers, we'd have:
- /*
- * 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;
- }
-
+ request->reuse();
for (auto it = buffers.begin(); it != buffers.end(); ++it) {
const Stream *stream = it->first;
FrameBuffer *buffer = it->second;
request->addBuffer(stream, buffer);
}
Which still isn't that bad imo but somebody requested ReuseBuffers.
> I understand the ReuseBuffers flag might be used to model a
> 'repeating' request mechanism, but I don't think our API is currently
> ready to support such a feature. All our requests are one-shot, they
> get queued, notified when completed, and need to be re-queued. There's
> nothing like a mechanism that let's you create one request, queue it
> once, and have it running until you don't stop you (without the need
> for a requeue. For reference [1]). Implementing something like this
> will need some more thinking, in example how to make it possible for a
> repeating request to cycle on a set of buffers automatically.
>
> I don't think we should try to emulate a repeating mechanism by
> allowing application to re-use the same stream-buffer pair. It has a
> limited use (in my understanding) and will confuse application as they
> need anyway to re-queue it.
Ah, I see. I guess this is more of convenience for the specific use case
of cam, and whatever applications that will have a similar demand.
> Don't get me wrong, I think moving ownership of Request to application
> is correct and more clear, but I don't see much use for the flag [*]
I think the flag is for future-proofing... if we want to add any other
options to resetting the reuqest.
> I would prefer if we keep thinking of our request as one shot, so they
> could either be reset everytime, and application will have to
> addBuffers() again as they will anyway have to cycle buffers.
I think we still preserve that. The application can use the request as
one-shot if it wants to, or it can reset it everytime. We're just
providing an option to automatically re-addBuffers().
Paul
> [*] This makes me unconfortable for another reason. I understand we
> might want flags for, say, Controls handling and other possible tuning
> (have you though about other possible flags ?). I don't think the
> output buffer handling lives in the same category as low level control
> of the Request, it's a primary Request feature and should not be expressed
> with flags as in example "Use template XYZ for the Request's controls"
>
> [1] https://medium.com/androiddevelopers/understanding-android-camera-capture-sessions-and-requests-4e54d9150295
>
> >
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > ---
> > Changes in v6:
> > - fix doxygen
> > - rename enum ReuseBuffer to ReuseFlag, and change to flags
> > - move request->reuse() in v4l2-compat from qbuf time to completion
> > time
> > - streamOff when the stream is already off needs to also
> > request->reuse(), so do that too
> > - update documentation
> >
> > Changes in v5:
> > - rename reset() to reuse()
> > - make reuse()'s reuseBuffers parameter into enum instead of bool
> > - fix qcam qt assertion error on the first queueRequest, where
> > freeQueue_ is empty
> >
> > Changes in v4:
> > - no more implicit calls of anything that we added in this patch
> > - make reset() take a reuseBuffers boolean parameters
> > - use transient request - delete request
> > - reuse request, reset buffers - reset()
> > - reuse request, reuse buffesr - reset(true)
> > - update apps and tests and documentation accordingly
> >
> > 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 | 7 +++++
> > src/android/camera_device.cpp | 14 +++++++---
> > src/cam/capture.cpp | 29 +++++---------------
> > src/cam/capture.h | 3 +++
> > src/gstreamer/gstlibcamerasrc.cpp | 14 ++++++----
> > src/libcamera/camera.cpp | 14 ++++------
> > src/libcamera/request.cpp | 38 ++++++++++++++++++++++++++
> > src/qcam/main_window.cpp | 42 ++++++++++++++++-------------
> > src/qcam/main_window.h | 26 +++++-------------
> > src/v4l2/v4l2_camera.cpp | 44 ++++++++++++++++++++++---------
> > src/v4l2/v4l2_camera.h | 4 ++-
> > test/camera/buffer_import.cpp | 13 ++++-----
> > test/camera/capture.cpp | 13 ++++-----
> > test/camera/statemachine.cpp | 9 +++----
> > 15 files changed, 163 insertions(+), 109 deletions(-)
> >
> > diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> > index a2ee4e7e..79ff8d6b 100644
> > --- a/include/libcamera/camera.h
> > +++ b/include/libcamera/camera.h
> > @@ -96,7 +96,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..655b1324 100644
> > --- a/include/libcamera/request.h
> > +++ b/include/libcamera/request.h
> > @@ -31,6 +31,11 @@ public:
> > RequestCancelled,
> > };
> >
> > + enum ReuseFlag {
> > + Default = 0,
> > + ReuseBuffers = (1 << 0),
> > + };
> > +
> > using BufferMap = std::map<const Stream *, FrameBuffer *>;
> >
> > Request(Camera *camera, uint64_t cookie = 0);
> > @@ -38,6 +43,8 @@ public:
> > Request &operator=(const Request &) = delete;
> > ~Request();
> >
> > + void reuse(ReuseFlag flags = Default);
> > +
> > 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 751699cd..052c9292 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1395,8 +1395,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > new Camera3RequestDescriptor(camera3Request->frame_number,
> > camera3Request->num_output_buffers);
> >
> > - Request *request =
> > + std::unique_ptr<Request> request =
> > camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
> > + if (!request) {
> > + LOG(HAL, Error) << "Failed to create request";
> > + return -ENOMEM;
> > + }
> >
> > for (unsigned int i = 0; i < descriptor->numBuffers; ++i) {
> > CameraStream *cameraStream =
> > @@ -1422,7 +1426,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[i].buffer);
> > if (!buffer) {
> > LOG(HAL, Error) << "Failed to create buffer";
> > - delete request;
> > delete descriptor;
> > return -ENOMEM;
> > }
> > @@ -1434,14 +1437,16 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
> > request->addBuffer(stream, buffer);
> > }
> >
> > - int ret = camera_->queueRequest(request);
> > + int ret = camera_->queueRequest(request.get());
> > if (ret) {
> > LOG(HAL, Error) << "Failed to queue request";
> > - delete request;
> > delete descriptor;
> > return ret;
> > }
> >
> > + /* The request will be deleted in the completion handler. */
> > + request.release();
> > +
> > return 0;
> > }
> >
> > @@ -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..8c8faa4b 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->reuse(Request::ReuseBuffers);
> > 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..5001083a 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,8 +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();
> > - auto wrap = std::make_unique<RequestWrap>(request);
> > + std::unique_ptr<Request> request = state->cam_->createRequest();
> > + auto wrap = std::make_unique<RequestWrap>(request.get());
> > for (GstPad *srcpad : state->srcpads_) {
> > GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);
> > GstBuffer *buffer;
> > @@ -280,8 +282,7 @@ gst_libcamera_src_task_run(gpointer user_data)
> > * RequestWrap does not take ownership, and we won't be
> > * queueing this one due to lack of buffers.
> > */
> > - delete request;
> > - request = nullptr;
> > + request.reset();
> > break;
> > }
> >
> > @@ -291,8 +292,11 @@ gst_libcamera_src_task_run(gpointer user_data)
> > if (request) {
> > GLibLocker lock(GST_OBJECT(self));
> > GST_TRACE_OBJECT(self, "Requesting buffers");
> > - state->cam_->queueRequest(request);
> > + state->cam_->queueRequest(request.get());
> > state->requests_.push(std::move(wrap));
> > +
> > + /* The request will be deleted in the completion handler. */
> > + request.release();
> > }
> >
> > GstFlowReturn ret = GST_FLOW_OK;
> > diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> > index fb76077f..9590ab72 100644
> > --- a/src/libcamera/camera.cpp
> > +++ b/src/libcamera/camera.cpp
> > @@ -847,21 +847,22 @@ 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 deleting it. The request may be deleted in the completion
> > + * handler, or reused after resetting its state with Request::reuse().
> > *
> > * \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);
> > }
> >
> > /**
> > @@ -877,9 +878,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.
> > *
> > @@ -988,13 +986,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..ae8b1660 100644
> > --- a/src/libcamera/request.cpp
> > +++ b/src/libcamera/request.cpp
> > @@ -37,6 +37,15 @@ LOG_DEFINE_CATEGORY(Request)
> > * The request has been cancelled due to capture stop
> > */
> >
> > +/**
> > + * \enum Request::ReuseFlag
> > + * Flags to control the behavior of Request::reuse()
> > + * \var Request::Default
> > + * Don't reuse buffers
> > + * \var Request::ReuseBuffers
> > + * Reuse the buffers that were previously added by addBuffer()
> > + */
> > +
> > /**
> > * \typedef Request::BufferMap
> > * \brief A map of Stream to FrameBuffer pointers
> > @@ -85,6 +94,35 @@ Request::~Request()
> > delete validator_;
> > }
> >
> > +/**
> > + * \brief Reset the request for reuse
> > + * \param[in] flags Indicate whether or not to reuse the buffers
> > + *
> > + * Reset the status and controls associated with the request, to allow it to
> > + * be reused and requeued without destruction. This function shall be called
> > + * prior to queueing the request to the camera, in lieu of constructing a new
> > + * request. The application can reuse the buffers that were previously added
> > + * to the request via addBuffer() by setting \a flags to ReuseBuffers.
> > + */
> > +void Request::reuse(ReuseFlag flags)
> > +{
> > + pending_.clear();
> > + if (flags & ReuseBuffers) {
> > + for (auto pair : bufferMap_) {
> > + pair.second->request_ = this;
> > + pending_.insert(pair.second);
> > + }
> > + } else {
> > + bufferMap_.clear();
> > + }
> > +
> > + status_ = RequestPending;
> > + cancelled_ = false;
> > +
> > + controls_->clear();
> > + metadata_->clear();
> > +}
> > +
> > /**
> > * \fn Request::controls()
> > * \brief Retrieve the request's ControlList
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index ecb9dd66..0cbdab9a 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,15 @@ void MainWindow::processCapture()
> > }
> >
> > /* Process buffers. */
> > - if (request.buffers_.count(vfStream_))
> > - processViewfinder(request.buffers_[vfStream_]);
> > + if (request->buffers().count(vfStream_))
> > + processViewfinder(request->buffers().at(vfStream_));
> >
> > - if (request.buffers_.count(rawStream_))
> > - processRaw(request.buffers_[rawStream_], request.metadata_);
> > + if (request->buffers().count(rawStream_))
> > + processRaw(request->buffers().at(rawStream_), request->metadata());
> > +
> > + request->reuse();
> > + QMutexLocker locker(&mutex_);
> > + freeQueue_.enqueue(request);
> > }
> >
> > void MainWindow::processViewfinder(FrameBuffer *buffer)
> > @@ -754,10 +757,13 @@ 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->addBuffer(vfStream_, buffer);
> > 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..35d3beda 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;
> >
> > @@ -96,6 +98,7 @@ void V4L2Camera::requestComplete(Request *request)
> > if (ret != sizeof(data))
> > LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
> >
> > + request->reuse();
> > {
> > MutexLocker locker(bufferMutex_);
> > bufferAvailableCount_++;
> > @@ -154,16 +157,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 +209,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;
> > }
> > @@ -206,8 +223,12 @@ int V4L2Camera::streamOn()
> >
> > int V4L2Camera::streamOff()
> > {
> > - if (!isRunning_)
> > + if (!isRunning_) {
> > + for (std::unique_ptr<Request> &req : requestPool_)
> > + req->reuse();
> > +
> > return 0;
> > + }
> >
> > pendingRequests_.clear();
> >
> > @@ -226,12 +247,11 @@ 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();
> >
> > Stream *stream = config_->at(0).stream();
> > FrameBuffer *buffer = bufferAllocator_->buffers(stream)[index].get();
> > @@ -242,11 +262,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..72ce7b79 100644
> > --- a/test/camera/buffer_import.cpp
> > +++ b/test/camera/buffer_import.cpp
> > @@ -58,7 +58,7 @@ protected:
> > const Stream *stream = buffers.begin()->first;
> > FrameBuffer *buffer = buffers.begin()->second;
> >
> > - request = camera_->createRequest();
> > + request->reuse();
> > 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..c0770801 100644
> > --- a/test/camera/capture.cpp
> > +++ b/test/camera/capture.cpp
> > @@ -52,7 +52,7 @@ protected:
> > const Stream *stream = buffers.begin()->first;
> > FrameBuffer *buffer = buffers.begin()->second;
> >
> > - request = camera_->createRequest();
> > + request->reuse();
> > 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
- Previous message (by thread): [libcamera-devel] [PATCH v6] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
- Next message (by thread): [libcamera-devel] [PATCH v6] libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
More information about the libcamera-devel
mailing list