Skip to content
Snippets Groups Projects
Commit 5416fd8a authored by Robert Mader's avatar Robert Mader
Browse files

temp/libcamera: backport more swISP patches 

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
parent d3e6de36
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 289 additions and 1 deletion
temp/libcamera/0005-libcamera-pipeline_handler-Provide-cancelRequest.patch 0 → 100644
+ 79
− 0
View file @ 5416fd8a
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
temp/libcamera/0006-libcamera-software_isp-Clean-up-pending-requests-on-.patch 0 → 100644
+ 81
− 0
View file @ 5416fd8a
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
temp/libcamera/0007-pipeline-simple-Consider-output-sizes-when-choosing-.patch 0 → 100644
+ 41
− 0
View file @ 5416fd8a
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
temp/libcamera/0008-pipeline-simple-Increase-buffer-count-to-four.patch 0 → 100644
+ 50
− 0
View file @ 5416fd8a
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
temp/libcamera/0009-pipeline-simple-Increase-internal-buffer-count-to-fo.patch 0 → 100644
+ 27
− 0
View file @ 5416fd8a
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
temp/libcamera/APKBUILD
+ 11
− 1
View file @ 5416fd8a
......@@ -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
"
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment