| Message ID | 20250526214224.13631-3-stefan.klug@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Stefan Klug (2025-05-26 22:42:16) > Allow pipeline handler classes to limit the maximum number of requests that > get queued using queueRequestDevice(). > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > include/libcamera/internal/pipeline_handler.h | 5 +++++ > src/libcamera/pipeline_handler.cpp | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > index dedc29e815fb..86b86d5971dc 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -79,6 +79,11 @@ protected: > virtual bool acquireDevice(Camera *camera); > virtual void releaseDevice(Camera *camera); > > + virtual unsigned int maxQueuedRequestsDevice() const > + { > + return std::numeric_limits<unsigned int>::max(); I wonder if something more small yet big enough would be sane like '16'. But there's not much difference between 'a number big enough' and the 'biggest number' if it should always be overridden. Something like this should come with a corresponding update to the pipeline handler writers guide to clarify what should be considered here. > + } > + > CameraManager *manager_; > > private: > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 14d8602e67d8..1853bca71371 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests() > > Camera::Private *data = camera->_d(); > while (!data->waitingRequests_.empty()) { Perhaps some verbose explanation here would be helpful to a reader: /* * Limit the number of requests being processed * by the device at one time to keep pipeline * internal depths controlled. The Camera holds * on to requests until the pipeline is ready to * process them. */ > + if (data->queuedRequests_.size() == maxQueuedRequestsDevice()) Would if (data->queuedRequests_.size() >= maxQueuedRequestsDevice()) break; be over cautious given the request can only be added below? > + break; > + > Request *request = data->waitingRequests_.front(); > if (!request->_d()->prepared_) > break; > -- > 2.43.0 >
Hi Stefan, On 5/26/25 23:42, Stefan Klug wrote: > Allow pipeline handler classes to limit the maximum number of requests that > get queued using queueRequestDevice(). > > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> > --- > include/libcamera/internal/pipeline_handler.h | 5 +++++ > src/libcamera/pipeline_handler.cpp | 3 +++ > 2 files changed, 8 insertions(+) > > diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h > index dedc29e815fb..86b86d5971dc 100644 > --- a/include/libcamera/internal/pipeline_handler.h > +++ b/include/libcamera/internal/pipeline_handler.h > @@ -79,6 +79,11 @@ protected: > virtual bool acquireDevice(Camera *camera); > virtual void releaseDevice(Camera *camera); > > + virtual unsigned int maxQueuedRequestsDevice() const > + { > + return std::numeric_limits<unsigned int>::max(); > + } > + is there a benefit of having a virtual function instead of a variable (initialized to max by default and potentially changed in the subclass constructors)? > CameraManager *manager_; > > private: > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp > index 14d8602e67d8..1853bca71371 100644 > --- a/src/libcamera/pipeline_handler.cpp > +++ b/src/libcamera/pipeline_handler.cpp > @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests() > > Camera::Private *data = camera->_d(); > while (!data->waitingRequests_.empty()) { > + if (data->queuedRequests_.size() == maxQueuedRequestsDevice()) > + break; > + > Request *request = data->waitingRequests_.front(); > if (!request->_d()->prepared_) > break;
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h index dedc29e815fb..86b86d5971dc 100644 --- a/include/libcamera/internal/pipeline_handler.h +++ b/include/libcamera/internal/pipeline_handler.h @@ -79,6 +79,11 @@ protected: virtual bool acquireDevice(Camera *camera); virtual void releaseDevice(Camera *camera); + virtual unsigned int maxQueuedRequestsDevice() const + { + return std::numeric_limits<unsigned int>::max(); + } + CameraManager *manager_; private: diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 14d8602e67d8..1853bca71371 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -490,6 +490,9 @@ void PipelineHandler::doQueueRequests() Camera::Private *data = camera->_d(); while (!data->waitingRequests_.empty()) { + if (data->queuedRequests_.size() == maxQueuedRequestsDevice()) + break; + Request *request = data->waitingRequests_.front(); if (!request->_d()->prepared_) break;
Allow pipeline handler classes to limit the maximum number of requests that get queued using queueRequestDevice(). Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> --- include/libcamera/internal/pipeline_handler.h | 5 +++++ src/libcamera/pipeline_handler.cpp | 3 +++ 2 files changed, 8 insertions(+)