temp/libcamera: backport more swISP patches (MR 5710)
Fixing or related to: - https://bugs.libcamera.org/show_bug.cgi?id=234 - https://bugs.libcamera.org/show_bug.cgi?id=235 - https://bugs.libcamera.org/show_bug.cgi?id=236 [ci:skip-build]: already built successfully in CI
This commit is contained in:
Robert Mader
Pablo Correa Gómez
parent
31aa6e8a2b
commit
58625e8394
|
|
@ -0,0 +1,79 @@
|
|||
From f74131fa00580e69776215a631217c8b20cc8189 Mon Sep 17 00:00:00 2001
|
||||
From: Milan Zamazal <mzamazal@redhat.com>
|
||||
Date: Wed, 9 Oct 2024 19:21:07 +0200
|
||||
Subject: [PATCH 1/3] libcamera: pipeline_handler: Provide cancelRequest
|
||||
|
||||
Let's extract the two occurrences of canceling a request to a common
|
||||
helper. This is especially useful for the followup patch, which needs
|
||||
to cancel a request from outside.
|
||||
|
||||
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
|
||||
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
||||
---
|
||||
include/libcamera/internal/pipeline_handler.h | 1 +
|
||||
src/libcamera/pipeline_handler.cpp | 23 +++++++++++++------
|
||||
2 files changed, 17 insertions(+), 7 deletions(-)
|
||||
|
||||
diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
|
||||
index 0d380803..fb28a18d 100644
|
||||
--- a/include/libcamera/internal/pipeline_handler.h
|
||||
+++ b/include/libcamera/internal/pipeline_handler.h
|
||||
@@ -60,6 +60,7 @@ public:
|
||||
|
||||
bool completeBuffer(Request *request, FrameBuffer *buffer);
|
||||
void completeRequest(Request *request);
|
||||
+ void cancelRequest(Request *request);
|
||||
|
||||
std::string configurationFile(const std::string &subdir,
|
||||
const std::string &name) const;
|
||||
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
|
||||
index e5940469..c9cb11f0 100644
|
||||
--- a/src/libcamera/pipeline_handler.cpp
|
||||
+++ b/src/libcamera/pipeline_handler.cpp
|
||||
@@ -367,9 +367,7 @@ void PipelineHandler::stop(Camera *camera)
|
||||
while (!waitingRequests_.empty()) {
|
||||
Request *request = waitingRequests_.front();
|
||||
waitingRequests_.pop();
|
||||
-
|
||||
- request->_d()->cancel();
|
||||
- completeRequest(request);
|
||||
+ cancelRequest(request);
|
||||
}
|
||||
|
||||
/* Make sure no requests are pending. */
|
||||
@@ -470,10 +468,8 @@ void PipelineHandler::doQueueRequest(Request *request)
|
||||
}
|
||||
|
||||
int ret = queueRequestDevice(camera, request);
|
||||
- if (ret) {
|
||||
- request->_d()->cancel();
|
||||
- completeRequest(request);
|
||||
- }
|
||||
+ if (ret)
|
||||
+ cancelRequest(request);
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -568,6 +564,19 @@ void PipelineHandler::completeRequest(Request *request)
|
||||
}
|
||||
}
|
||||
|
||||
+/**
|
||||
+ * \brief Cancel request and signal its completion
|
||||
+ * \param[in] request The request to cancel
|
||||
+ *
|
||||
+ * This function cancels the request in addition to its completion. The same
|
||||
+ * rules as for completeRequest() apply.
|
||||
+ */
|
||||
+void PipelineHandler::cancelRequest(Request *request)
|
||||
+{
|
||||
+ request->_d()->cancel();
|
||||
+ completeRequest(request);
|
||||
+}
|
||||
+
|
||||
/**
|
||||
* \brief Retrieve the absolute path to a platform configuration file
|
||||
* \param[in] subdir The pipeline handler specific subdirectory name
|
||||
--
|
||||
2.47.0
|
||||
|
||||
|
|
@ -0,0 +1,81 @@
|
|||
From 36c6eb6e9b665830f11876c125780af08b6217d0 Mon Sep 17 00:00:00 2001
|
||||
From: Milan Zamazal <mzamazal@redhat.com>
|
||||
Date: Wed, 9 Oct 2024 19:21:08 +0200
|
||||
Subject: [PATCH 2/3] libcamera: software_isp: Clean up pending requests on
|
||||
stop
|
||||
|
||||
PipelineHandler::stop() calls stopDevice() method to perform pipeline
|
||||
specific cleanup and then completes waiting requests. If any queued
|
||||
requests remain, an assertion error is raised.
|
||||
|
||||
Software ISP stores request buffers in
|
||||
SimpleCameraData::conversionQueue_ and queues them as V4L2 signals
|
||||
bufferReady. stopDevice() cleanup forgets to clean up the buffers and
|
||||
their requests from conversionQueue_, possibly resulting in the
|
||||
assertion error. This patch fixes the omission.
|
||||
|
||||
The problem wasn't very visible when
|
||||
SimplePipelineHandler::kNumInternalBuffers (the number of buffers
|
||||
allocated in V4L2) was equal to the number of buffers exported from
|
||||
software ISP. But when the number of the exported buffers was increased
|
||||
by one in commit abe2ec64f9e4e97bbdfe3a50372611bd7b5315c2, the assertion
|
||||
error started pop up in some environments. Increasing the number of the
|
||||
buffers much more, e.g. to 9, makes the problem very reproducible.
|
||||
|
||||
Each pipeline uses its own mechanism to track the requests to clean up
|
||||
and it can't be excluded that similar omissions are present in other
|
||||
places. But there is no obvious way to make a common cleanup for all
|
||||
the pipelines (except for doing it instead of raising the assertion
|
||||
error, which is probably undesirable, in order not to hide incomplete
|
||||
pipeline specific cleanups).
|
||||
|
||||
Bug: https://bugs.libcamera.org/show_bug.cgi?id=234
|
||||
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
|
||||
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
||||
---
|
||||
src/libcamera/pipeline/simple/simple.cpp | 15 +++++++++++++++
|
||||
1 file changed, 15 insertions(+)
|
||||
|
||||
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
|
||||
index 29320a26..5ee6a5c3 100644
|
||||
--- a/src/libcamera/pipeline/simple/simple.cpp
|
||||
+++ b/src/libcamera/pipeline/simple/simple.cpp
|
||||
@@ -281,6 +281,7 @@ public:
|
||||
std::vector<std::unique_ptr<FrameBuffer>> conversionBuffers_;
|
||||
std::queue<std::map<const Stream *, FrameBuffer *>> conversionQueue_;
|
||||
bool useConversion_;
|
||||
+ void clearIncompleteRequests();
|
||||
|
||||
std::unique_ptr<Converter> converter_;
|
||||
std::unique_ptr<SoftwareIsp> swIsp_;
|
||||
@@ -886,6 +887,19 @@ void SimpleCameraData::conversionOutputDone(FrameBuffer *buffer)
|
||||
pipe->completeRequest(request);
|
||||
}
|
||||
|
||||
+void SimpleCameraData::clearIncompleteRequests()
|
||||
+{
|
||||
+ while (!conversionQueue_.empty()) {
|
||||
+ for (auto &item : conversionQueue_.front()) {
|
||||
+ FrameBuffer *outputBuffer = item.second;
|
||||
+ Request *request = outputBuffer->request();
|
||||
+ if (request->status() == Request::RequestPending)
|
||||
+ pipe()->cancelRequest(request);
|
||||
+ }
|
||||
+ conversionQueue_.pop();
|
||||
+ }
|
||||
+}
|
||||
+
|
||||
void SimpleCameraData::ispStatsReady()
|
||||
{
|
||||
/* \todo Use the DelayedControls class */
|
||||
@@ -1382,6 +1396,7 @@ void SimplePipelineHandler::stopDevice(Camera *camera)
|
||||
|
||||
video->bufferReady.disconnect(data, &SimpleCameraData::bufferReady);
|
||||
|
||||
+ data->clearIncompleteRequests();
|
||||
data->conversionBuffers_.clear();
|
||||
|
||||
releasePipeline(data);
|
||||
--
|
||||
2.47.0
|
||||
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
From d07972cc4188a38816c8686f6b66467fe5b9a45d Mon Sep 17 00:00:00 2001
|
||||
From: Robert Mader <robert.mader@collabora.com>
|
||||
Date: Fri, 11 Oct 2024 20:13:24 +0200
|
||||
Subject: [PATCH 3/3] pipeline: simple: Consider output sizes when choosing
|
||||
pipe config
|
||||
|
||||
In order to avoid having to adjust the size further down below which
|
||||
again can break user assumptions. Notably, without this the capture size
|
||||
of 1920x1080 gets adjusted to 1912x1080 when used with the swISP using a
|
||||
bayer pattern width of 4, breaking users like Gstreamer down the line.
|
||||
|
||||
Closes https://bugs.libcamera.org/show_bug.cgi?id=236
|
||||
|
||||
Signed-off-by: Robert Mader <robert.mader@collabora.com>
|
||||
|
||||
---
|
||||
|
||||
I'm not really sure if this is the correct approach, but sending it out
|
||||
already for feedback. So far this gives me promissing results on tested
|
||||
devices.
|
||||
---
|
||||
src/libcamera/pipeline/simple/simple.cpp | 3 ++-
|
||||
1 file changed, 2 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
|
||||
index 5ee6a5c3..ffb59473 100644
|
||||
--- a/src/libcamera/pipeline/simple/simple.cpp
|
||||
+++ b/src/libcamera/pipeline/simple/simple.cpp
|
||||
@@ -1051,7 +1051,8 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
|
||||
const Size &size = pipeConfig->captureSize;
|
||||
|
||||
if (size.width >= maxStreamSize.width &&
|
||||
- size.height >= maxStreamSize.height) {
|
||||
+ size.height >= maxStreamSize.height &&
|
||||
+ pipeConfig->outputSizes.contains(maxStreamSize)) {
|
||||
if (!pipeConfig_ || size < pipeConfig_->captureSize)
|
||||
pipeConfig_ = pipeConfig;
|
||||
}
|
||||
--
|
||||
2.47.0
|
||||
|
||||
|
|
@ -0,0 +1,50 @@
|
|||
From 08a845788faeb8e38e54aca14ef739349771438e Mon Sep 17 00:00:00 2001
|
||||
From: Robert Mader <robert.mader@collabora.com>
|
||||
Date: Thu, 26 Sep 2024 23:07:39 +0200
|
||||
Subject: [PATCH 1/2] pipeline: simple: Increase buffer count to four
|
||||
|
||||
Which is not only what many other pipeline handlers use, but also a good
|
||||
lower limit when dealing with DRM and similar APIs. Even Mesas EGL and
|
||||
Vulkan WSI implementations use for the reason outlined in mesa commit
|
||||
992a2dbba80aba35efe83202e1013bd6143f0dba:
|
||||
> When the compositor is directly scanning out from the application's buffer it
|
||||
> may end up holding on to three buffers. These are the one that is is currently
|
||||
> scanning out from, one that has been given to DRM as the next buffer to flip
|
||||
> to, and one that has been attached and will be given to DRM as soon as the
|
||||
> previous flip completes. When we attach a fourth buffer to the compositor it
|
||||
> should replace that third buffer so we should get a release event immediately
|
||||
> after that. This patch therefore also changes the number of buffer slots to 4
|
||||
> so that we can accomodate that situation.
|
||||
|
||||
Given the popularity of this buffer number the bump should be unlikely
|
||||
to cause problems. At the same time it may help with performance or
|
||||
even work around glitches.
|
||||
|
||||
The previous number was introduced in commit
|
||||
a8964c28c80fb520ee3c7b10143371081d41405a without mentioning a specific
|
||||
reason against the change at hand.
|
||||
|
||||
Signed-off-by: Robert Mader <robert.mader@collabora.com>
|
||||
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
|
||||
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
||||
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
||||
---
|
||||
src/libcamera/pipeline/simple/simple.cpp | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
|
||||
index ffb59473..931a3c7d 100644
|
||||
--- a/src/libcamera/pipeline/simple/simple.cpp
|
||||
+++ b/src/libcamera/pipeline/simple/simple.cpp
|
||||
@@ -1149,7 +1149,7 @@ CameraConfiguration::Status SimpleCameraConfiguration::validate()
|
||||
cfg.frameSize = format.planes[0].size;
|
||||
}
|
||||
|
||||
- cfg.bufferCount = 3;
|
||||
+ cfg.bufferCount = 4;
|
||||
}
|
||||
|
||||
return status;
|
||||
--
|
||||
2.47.0
|
||||
|
||||
|
|
@ -0,0 +1,27 @@
|
|||
From e1ec2833fc7320eb9c6258fa17022cd920ba5cc1 Mon Sep 17 00:00:00 2001
|
||||
From: Robert Mader <robert.mader@collabora.com>
|
||||
Date: Sun, 13 Oct 2024 14:13:44 +0200
|
||||
Subject: [PATCH 2/2] pipeline: simple: Increase internal buffer count to four
|
||||
aswell
|
||||
|
||||
Signed-off-by: Robert Mader <robert.mader@collabora.com>
|
||||
---
|
||||
src/libcamera/pipeline/simple/simple.cpp | 2 +-
|
||||
1 file changed, 1 insertion(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
|
||||
index 931a3c7d..855a4de4 100644
|
||||
--- a/src/libcamera/pipeline/simple/simple.cpp
|
||||
+++ b/src/libcamera/pipeline/simple/simple.cpp
|
||||
@@ -352,7 +352,7 @@ protected:
|
||||
int queueRequestDevice(Camera *camera, Request *request) override;
|
||||
|
||||
private:
|
||||
- static constexpr unsigned int kNumInternalBuffers = 3;
|
||||
+ static constexpr unsigned int kNumInternalBuffers = 4;
|
||||
|
||||
struct EntityData {
|
||||
std::unique_ptr<V4L2VideoDevice> video;
|
||||
--
|
||||
2.47.0
|
||||
|
||||
|
|
@ -3,7 +3,7 @@
|
|||
pkgname=libcamera
|
||||
pkgver=9999
|
||||
_pkgver=0.3.2
|
||||
pkgrel=6
|
||||
pkgrel=7
|
||||
pkgdesc="Linux camera framework"
|
||||
url="https://libcamera.org/"
|
||||
arch="all"
|
||||
|
|
@ -46,6 +46,11 @@ source="https://gitlab.freedesktop.org/camera/libcamera/-/archive/v$_pkgver/libc
|
|||
0002-libcamera-simple-Force-disable-softwareISP-for-milli.patch
|
||||
0003-libcamera-simple-Enable-softISP-for-the-Pinephone.patch
|
||||
0004-libcamera-simple-Skip-hwISP-formats-if-swISP-is-acti.patch
|
||||
0005-libcamera-pipeline_handler-Provide-cancelRequest.patch
|
||||
0006-libcamera-software_isp-Clean-up-pending-requests-on-.patch
|
||||
0007-pipeline-simple-Consider-output-sizes-when-choosing-.patch
|
||||
0008-pipeline-simple-Increase-buffer-count-to-four.patch
|
||||
0009-pipeline-simple-Increase-internal-buffer-count-to-fo.patch
|
||||
qcam.desktop
|
||||
90-libcamera.rules
|
||||
"
|
||||
|
|
@ -150,6 +155,11 @@ ac7df3e4509ae874199810057f4d8416da71720c15534578cc352608a8ae228dfa4814f9eb995d55
|
|||
9b6da8bd11ff9d8400ed721fbbeb960ac8753c078fdd971d786a446a9f96fea19dfc55be2705dc44a152e11de996f88139c1d24637bffc257da5083d19fe80c9 0002-libcamera-simple-Force-disable-softwareISP-for-milli.patch
|
||||
0fc6a1108c4e905d2d422a664622dc25ec459f13765b5711ad009d4df0fd0cceda8cd067a18e5e54eb2346b292481952161e72deb03d416ba80b300256c25e40 0003-libcamera-simple-Enable-softISP-for-the-Pinephone.patch
|
||||
35c74746453f4c2e24a2185331afabcf64e3af01bec2462ec09940518cda0e91c4a1f33853b4b009e1f8352af3c606fbd4b4d3791ffbba0f610f19538380c4c8 0004-libcamera-simple-Skip-hwISP-formats-if-swISP-is-acti.patch
|
||||
d8460cb16ad7787f90450bd8bc85b18af14ec5b9add09b246ffd8737275b4681e670e5ce98e2151ba9343f51ecf10e3100846ca480e75654ba2989c28498e702 0005-libcamera-pipeline_handler-Provide-cancelRequest.patch
|
||||
caa441737da9dc1e9eaa2e27d23ae8d02a16b412deb4b75e144c75dd57ae2bb73b22e2062c593a247ade38a38992945e406679b7a69b7885a109b396808fb37f 0006-libcamera-software_isp-Clean-up-pending-requests-on-.patch
|
||||
3a969bb728c4d73f1bc99d97b749ae657ef5e4ff5f1e5f0ec73a470ec362b9d9039070a9eac52ccfcf0bf0a000b4149055fc3053d3bb4bbcc1633a373c041de1 0007-pipeline-simple-Consider-output-sizes-when-choosing-.patch
|
||||
396f741d6cbec8ba316f4651a912d45eb3b4ee8be79f653d2eab24be8dbb0b08e59d4b603d418cb9cc86251d22c151aef5cc5658a012c9b8d7c83862ae232cb1 0008-pipeline-simple-Increase-buffer-count-to-four.patch
|
||||
dbdaf5fdcd250d6591ce7981256cce3c1dc901b395b797b785602ceef5e3f0bf110ca2ec7edba6abdb6f6b434b8ff376c1121cf6f5c8d204d6eb4369684be627 0009-pipeline-simple-Increase-internal-buffer-count-to-fo.patch
|
||||
22167a4eceb6d1b40b0b7c45fdf116c71684f5340de7f767535cb8e160ad9d2ae0f00cb3d461f73a344520a48a4641cf46226841d78bee06bfbfd2a91337f754 qcam.desktop
|
||||
cb4eb19eec766f1b8667a8b7c9d5f7d44a2dce79fddfdf3b6e3d1849066cebe79f82566bdcf6659c7ddf4faaf233d5adac10cda636935785e5305e2b7e9b34a9 90-libcamera.rules
|
||||
"
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user