| Message ID | 20251010092226.41228-3-robert.mader@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Quoting Robert Mader (2025-10-10 10:22:26) > Now that the pipeline handler supports high buffer counts, apps may > queue more requests, exhausting e.g. frame contexts (see > ipa::soft::kMaxFrameContexts => 16). > > Thus limit the number of queued requests to 4, corresponding to the > the previous buffer limit. > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Signed-off-by: Robert Mader <robert.mader@collabora.com> Oddly enough - I think this series is backwards :D I think applying as 3/3 Restrict pipeline queuing (to 3?) Prevent ever overflowing 2/3 Increase buffering (to tested limits) 1/3 Open the flood gates to let more requests be queued safely Handles this in a truely bisectable/safe way. But that would require rewriting the commit messages and might be just overkill :D Anyway for this patch: Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/pipeline/simple/simple.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index c4cb7c391..dec9f6514 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -420,6 +420,7 @@ protected: > int queueRequestDevice(Camera *camera, Request *request) override; > > private: > + static constexpr unsigned int kMaxQueuedRequestsDevice = 4; > static constexpr unsigned int kNumInternalBuffers = 4; > > struct EntityData { > @@ -1287,7 +1288,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > */ > > SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) > - : PipelineHandler(manager), converter_(nullptr) > + : PipelineHandler(manager, kMaxQueuedRequestsDevice), > + converter_(nullptr) > { > } > > -- > 2.51.0 >
2025. 10. 10. 11:22 keltezéssel, Robert Mader írta: > Now that the pipeline handler supports high buffer counts, apps may > queue more requests, exhausting e.g. frame contexts (see > ipa::soft::kMaxFrameContexts => 16). > > Thus limit the number of queued requests to 4, corresponding to the > the previous buffer limit. > > Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > Signed-off-by: Robert Mader <robert.mader@collabora.com> > --- Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> As far as I could test, this avoids the fatal assertion when the number of quued buffers is > 16. > src/libcamera/pipeline/simple/simple.cpp | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp > index c4cb7c391..dec9f6514 100644 > --- a/src/libcamera/pipeline/simple/simple.cpp > +++ b/src/libcamera/pipeline/simple/simple.cpp > @@ -420,6 +420,7 @@ protected: > int queueRequestDevice(Camera *camera, Request *request) override; > > private: > + static constexpr unsigned int kMaxQueuedRequestsDevice = 4; > static constexpr unsigned int kNumInternalBuffers = 4; > > struct EntityData { > @@ -1287,7 +1288,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() > */ > > SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) > - : PipelineHandler(manager), converter_(nullptr) > + : PipelineHandler(manager, kMaxQueuedRequestsDevice), > + converter_(nullptr) > { > } >
On 10/10/25 14:07, Barnabás Pőcze wrote: > 2025. 10. 10. 11:22 keltezéssel, Robert Mader írta: >> Now that the pipeline handler supports high buffer counts, apps may >> queue more requests, exhausting e.g. frame contexts (see >> ipa::soft::kMaxFrameContexts => 16). >> >> Thus limit the number of queued requests to 4, corresponding to the >> the previous buffer limit. >> >> Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> Signed-off-by: Robert Mader <robert.mader@collabora.com> >> --- > > Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > > As far as I could test, this avoids the fatal assertion > when the number of quued buffers is > 16. Nice, thanks! > >> src/libcamera/pipeline/simple/simple.cpp | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp >> b/src/libcamera/pipeline/simple/simple.cpp >> index c4cb7c391..dec9f6514 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -420,6 +420,7 @@ protected: >> int queueRequestDevice(Camera *camera, Request *request) override; >> private: >> + static constexpr unsigned int kMaxQueuedRequestsDevice = 4; >> static constexpr unsigned int kNumInternalBuffers = 4; >> struct EntityData { >> @@ -1287,7 +1288,8 @@ CameraConfiguration::Status >> SimpleCameraConfiguration::validate() >> */ >> SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) >> - : PipelineHandler(manager), converter_(nullptr) >> + : PipelineHandler(manager, kMaxQueuedRequestsDevice), >> + converter_(nullptr) >> { >> } >
On 10/10/25 12:22, Kieran Bingham wrote: > Quoting Robert Mader (2025-10-10 10:22:26) >> Now that the pipeline handler supports high buffer counts, apps may >> queue more requests, exhausting e.g. frame contexts (see >> ipa::soft::kMaxFrameContexts => 16). >> >> Thus limit the number of queued requests to 4, corresponding to the >> the previous buffer limit. >> >> Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> Signed-off-by: Robert Mader <robert.mader@collabora.com> > Oddly enough - I think this series is backwards :D > > I think applying as > > 3/3 Restrict pipeline queuing (to 3?) Prevent ever overflowing > 2/3 Increase buffering (to tested limits) > 1/3 Open the flood gates to let more requests be queued safely > > Handles this in a truely bisectable/safe way. > > But that would require rewriting the commit messages and might be just > overkill :D I can spin a v5 over the weekend or on Monday, however feel free to already land with any suggestions to the commit message :) > > Anyway for this patch: > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > >> --- >> src/libcamera/pipeline/simple/simple.cpp | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp >> index c4cb7c391..dec9f6514 100644 >> --- a/src/libcamera/pipeline/simple/simple.cpp >> +++ b/src/libcamera/pipeline/simple/simple.cpp >> @@ -420,6 +420,7 @@ protected: >> int queueRequestDevice(Camera *camera, Request *request) override; >> >> private: >> + static constexpr unsigned int kMaxQueuedRequestsDevice = 4; >> static constexpr unsigned int kNumInternalBuffers = 4; >> >> struct EntityData { >> @@ -1287,7 +1288,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() >> */ >> >> SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) >> - : PipelineHandler(manager), converter_(nullptr) >> + : PipelineHandler(manager, kMaxQueuedRequestsDevice), >> + converter_(nullptr) >> { >> } >> >> -- >> 2.51.0 >>
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp index c4cb7c391..dec9f6514 100644 --- a/src/libcamera/pipeline/simple/simple.cpp +++ b/src/libcamera/pipeline/simple/simple.cpp @@ -420,6 +420,7 @@ protected: int queueRequestDevice(Camera *camera, Request *request) override; private: + static constexpr unsigned int kMaxQueuedRequestsDevice = 4; static constexpr unsigned int kNumInternalBuffers = 4; struct EntityData { @@ -1287,7 +1288,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate() */ SimplePipelineHandler::SimplePipelineHandler(CameraManager *manager) - : PipelineHandler(manager), converter_(nullptr) + : PipelineHandler(manager, kMaxQueuedRequestsDevice), + converter_(nullptr) { }
Now that the pipeline handler supports high buffer counts, apps may queue more requests, exhausting e.g. frame contexts (see ipa::soft::kMaxFrameContexts => 16). Thus limit the number of queued requests to 4, corresponding to the the previous buffer limit. Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Signed-off-by: Robert Mader <robert.mader@collabora.com> --- src/libcamera/pipeline/simple/simple.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)