From patchwork Sat Nov 20 11:13:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 14668 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 96D86BDB13 for ; Sat, 20 Nov 2021 11:12:24 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CD8A56032C; Sat, 20 Nov 2021 12:12:23 +0100 (CET) Received: from relay11.mail.gandi.net (relay11.mail.gandi.net [217.70.178.231]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 747E26032C for ; Sat, 20 Nov 2021 12:12:22 +0100 (CET) Received: (Authenticated sender: jacopo@jmondi.org) by relay11.mail.gandi.net (Postfix) with ESMTPSA id D6CF7100005; Sat, 20 Nov 2021 11:12:21 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Sat, 20 Nov 2021 12:13:01 +0100 Message-Id: <20211120111313.106621-1-jacopo@jmondi.org> X-Mailer: git-send-email 2.33.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 00/12] [WIP] libcamera: Add support for Fence X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Marked as WIP as I've not completed the documentation yet, as I wish to validate the API first. Also, I'm sure I've missed pieces here and there but I want this out to discuss it before getting it into a mergeable state. Reworked the API as suggested and discussed in v1. v1->v2: Major changes: - removed notifiers and timers from Fence. A Fence is now an wrapper that owns a file descriptor - Add timers and notifiers to Request::Private and add the Request::Private::prepare interface Smaller changes: - Expand Request::Private to move all internal fields - Remove Fence move semantic. A Fence lives in a Framebuffer. - A few minor patches on top Fence application API: 1 unique_ptr fence = make_unique(FileDescriptor &fd); 2 request->addBuffer(..., move(fence)) 3 frameBuffer.set(fence) 4 camera->queueRequest(request) 5 pipelineHandler.queueRequest() 6 request.prepare() 7 request.prepared.emit() 8 pipelineHandler.doQueueRequest() 9 if (request.privateState.Ready()) 10 doQueueRequestDevice(request) 11 else 12 request->cancel() 13 requestCompleted() 14 if buffer.fence() 15 FileDescriptor fd = std::move(buffer.fence.fd()) What I don't like - It's a clunky API FileDescriptor fenceFd = FileDescriptor(std::move(buffer.fence)); std::unique_ptr fence = std::make_unique(fenceFd); descriptor->request_->addBuffer(cameraStream->stream(), frameBuffer, std::move(fence)); - Fence creation with a FileDescriptor & does not enforce that the FileDescriptor should not have duplicated the integer file descriptor - Fence constructor takes a FileDescriptor &, but moves the FileDescriptor internally not to duplicate the fd. The FileDescriptor passed as reference is invalidated but nothing in the API suggests so - Request::addBuffer() moves the Fence to the Framebuffer not to the request even if the function prototype suggest differently - The API to extract the FileDescritptor from a completed request requires the user to now that it has to move the FileDescritpro out, nothing suggest that - Request now has Request::Private::Status and Request::Status - Fence are an empty wrapper around a FileDescriptor. That's ok-ish as it allows to support more synchronization methods in future without changing the API towards the Fence class users - Timeouts are per-request not per fence. If we need to have it per-fence the core infrastructure need to be changed Buildbot https://buildbot.libcamera.org/#/builders/1/builds/54 CTS: ======================================================= =============== Summary =============== Total Run time: 19m 58s 1/1 modules completed Total Tests : 231 PASSED : 230 FAILED : 1 ============== End of Results ============== ============================================ Thanks j Jacopo Mondi (11): libcamera: fence: Introduce Fence libcamera: framebuffer: private: Add Fence libcamera: request: Add Fence to Request::addBuffer() test: fence: Add test for the Fence class libcamera: pipeline_handler: Split request queueing libcamera: pipeline: Introduce stopDevice() libcamera: request: Add Request::Private::prepare() libcamera: pipeline_handler: Prepare Request android: Remove CameraWorker test: camera: Fix trivial spelling mistaken libcamera: Add tracing to meson summary Laurent Pinchart (1): libcamera: request: Make Request class Extensible include/libcamera/fence.h | 36 ++ include/libcamera/framebuffer.h | 3 + include/libcamera/internal/fence.h | 37 ++ include/libcamera/internal/framebuffer.h | 9 + include/libcamera/internal/meson.build | 2 + include/libcamera/internal/pipeline_handler.h | 9 +- include/libcamera/internal/request.h | 72 +++ .../libcamera/internal/tracepoints/request.tp | 18 +- include/libcamera/meson.build | 1 + include/libcamera/request.h | 22 +- meson.build | 1 + src/android/camera_device.cpp | 43 +- src/android/camera_device.h | 3 - src/android/camera_request.cpp | 3 +- src/android/camera_request.h | 3 +- src/android/camera_worker.cpp | 129 ----- src/android/camera_worker.h | 72 --- src/android/meson.build | 1 - src/libcamera/fence.cpp | 104 ++++ src/libcamera/framebuffer.cpp | 26 + src/libcamera/meson.build | 4 + src/libcamera/pipeline/ipu3/ipu3.cpp | 4 +- .../pipeline/raspberrypi/raspberrypi.cpp | 4 +- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +- src/libcamera/pipeline/simple/simple.cpp | 4 +- src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 4 +- src/libcamera/pipeline/vimc/vimc.cpp | 4 +- src/libcamera/pipeline_handler.cpp | 97 +++- src/libcamera/request.cpp | 462 ++++++++++++++---- test/camera/capture.cpp | 2 +- test/fence.cpp | 339 +++++++++++++ test/meson.build | 1 + 32 files changed, 1160 insertions(+), 363 deletions(-) create mode 100644 include/libcamera/fence.h create mode 100644 include/libcamera/internal/fence.h create mode 100644 include/libcamera/internal/request.h delete mode 100644 src/android/camera_worker.cpp delete mode 100644 src/android/camera_worker.h create mode 100644 src/libcamera/fence.cpp create mode 100644 test/fence.cpp --- 2.33.1