From patchwork Mon Apr 26 08:38:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 12110 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 EB437BDC99 for ; Mon, 26 Apr 2021 08:38:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AE2B2688B6; Mon, 26 Apr 2021 10:38:41 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="iw/S7OvC"; dkim-atps=neutral Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 02614605BE for ; Mon, 26 Apr 2021 10:38:39 +0200 (CEST) Received: by mail-pj1-x102c.google.com with SMTP id j6-20020a17090adc86b02900cbfe6f2c96so4858657pjv.1 for ; Mon, 26 Apr 2021 01:38:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=4zlrEa4ha9TdBM0OP0ylkX7d3YyewkL0ZRW5RuTbEl4=; b=iw/S7OvC0lVn6MD3L0B/Y57TT4dGYDGxq/EyCO9MpQQswgLAtaiT6Lj4ci4bcMZA/f r8zFt0kiWiTP+QJdxYR7relu9O9S6OlMCeIvZxerCXYEpFpROE4leneH1SIpes7sinpT jyhp2wAdxrSKLQPDFzfpbgIckeGVjdatiLw/g= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=4zlrEa4ha9TdBM0OP0ylkX7d3YyewkL0ZRW5RuTbEl4=; b=JUJKrLxU7QghJr1c7/MIiA7Z+XHzBpijTb9PGQ5BrsgY1RF9JiyTA7xqOBVAhQ7G3X ONtE5gXYlfeL4+KZsC7njF+Pr/VfTPpUovVtSNafHwIDifyYoPGOYiiUW9Id1eI4XIEB ea0vKI1Hq6weN4WaUDgX6C2kwd8bZh2eAoE9RHJbwK+UNKJPQWwZB7z/7Hjei69PHlWo fKLOfvtPOpukLDKG/i85eti3jOKKEgXXCCVwTciLnH6ezIvEIOD7cvVlg45NmhwExXSA zYKrXHP33emPdtY7VijRQpebnLbVqlWi2A86f7Lk8JzY0Bpd21I8um0V4yNMWRvzTNxu nyaw== X-Gm-Message-State: AOAM532mKN4QxRHq9FtppGambnasA7PoHJ+Chor5V7vjIjhcsKKlYzlC 6MLVJ+BH9DEVMTsdEL0v5dhbWrAm+DZVmg== X-Google-Smtp-Source: ABdhPJxIFGC84LDul1+MTQdgLlhCPyYy9N2dFEofIKHGcasRHuo1VG+6OddqACwO1BgXuI4P1/10gQ== X-Received: by 2002:a17:90a:6585:: with SMTP id k5mr19612685pjj.40.1619426317458; Mon, 26 Apr 2021 01:38:37 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:a407:b8c2:d8b:a449]) by smtp.gmail.com with ESMTPSA id e18sm4873728pgr.8.2021.04.26.01.38.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Apr 2021 01:38:37 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Mon, 26 Apr 2021 17:38:28 +0900 Message-Id: <20210426083830.2965095-2-hiroh@chromium.org> X-Mailer: git-send-email 2.31.1.498.g6c1eba8ee3d-goog In-Reply-To: <20210426083830.2965095-1-hiroh@chromium.org> References: <20210426083830.2965095-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/3] android: CameraDevice: Separate CaptureRequest from Camera3RequestDescriptor 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" Camera3RequestDescriptors can be destroyed in stop(). Since Camera3RequestDescriptor owns CaptureRequest, the CaptureRequest should not be used anymore. But requestComplete() cannot check if the associated descriptor with a request is valid without calling CaptureRequest::cookie(). This resolves the problem by separating CaptureRequest lifetime from Camera3RequestDescriptor. Thus CaptureRequest always is destroyed in requestComplete(). This guarantees that a request is valid until requestComplete() is called. Signed-off-by: Hirokazu Honda --- src/android/camera_device.cpp | 27 +++++++++++++++------------ src/android/camera_device.h | 9 +++++---- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index a71aee2f..b3def04b 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -361,7 +361,8 @@ bool validateCropRotate(const camera3_stream_configuration_t &streamList) */ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( - Camera *camera, const camera3_capture_request_t *camera3Request) + const camera3_capture_request_t *camera3Request, CaptureRequest *request) + : request_(request) { frameNumber_ = camera3Request->frame_number; @@ -379,12 +380,6 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( /* Clone the controls associated with the camera3 request. */ settings_ = CameraMetadata(camera3Request->settings); - - /* - * Create the CaptureRequest, stored as a unique_ptr<> to tie its - * lifetime to the descriptor. - */ - request_ = std::make_unique(camera); } /* @@ -1930,7 +1925,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * The descriptor and the associated memory reserved here are freed * at request complete time. */ - Camera3RequestDescriptor descriptor(camera_.get(), camera3Request); + auto request = std::make_unique(camera_.get()); + Camera3RequestDescriptor descriptor(camera3Request, request.get()); /* * \todo The Android request model is incremental, settings passed in @@ -2013,11 +2009,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (ret) return ret; - worker_.queueRequest(descriptor.request_.get()); + worker_.queueRequest(descriptor.request_); { std::scoped_lock lock(mutex_); descriptors_[descriptor.request_->cookie()] = std::move(descriptor); + requests_[request->cookie()] = std::move(request); } return 0; @@ -2030,15 +2027,21 @@ void CameraDevice::requestComplete(Request *request) std::unique_ptr resultMetadata; decltype(descriptors_)::node_type node; + std::unique_ptr captureRequest; { std::scoped_lock lock(mutex_); - auto it = descriptors_.find(request->cookie()); - if (it == descriptors_.end()) { + auto requestIt = requests_.find(request->cookie()); + if (requestIt == requests_.end()) { LOG(HAL, Fatal) << "Unknown request: " << request->cookie(); - status = CAMERA3_BUFFER_STATUS_ERROR; return; } + captureRequest = std::move(requestIt->second); + requests_.erase(requestIt); + + auto it = descriptors_.find(request->cookie()); + if (it == descriptors_.end()) + return; node = descriptors_.extract(it); } diff --git a/src/android/camera_device.h b/src/android/camera_device.h index c63e8e21..95d77890 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -72,15 +72,15 @@ private: struct Camera3RequestDescriptor { Camera3RequestDescriptor() = default; ~Camera3RequestDescriptor() = default; - Camera3RequestDescriptor(libcamera::Camera *camera, - const camera3_capture_request_t *camera3Request); + Camera3RequestDescriptor(const camera3_capture_request_t *camera3Request, + CaptureRequest *request); Camera3RequestDescriptor &operator=(Camera3RequestDescriptor &&) = default; uint32_t frameNumber_ = 0; std::vector buffers_; std::vector> frameBuffers_; CameraMetadata settings_; - std::unique_ptr request_; + CaptureRequest *request_ = nullptr; }; struct Camera3StreamConfiguration { @@ -127,8 +127,9 @@ private: std::map formatsMap_; std::vector streams_; - std::mutex mutex_; /* Protect descriptors_ */ + std::mutex mutex_; /* Protect descriptors_ and requests_. */ std::map descriptors_; + std::map> requests_; std::string maker_; std::string model_;