From patchwork Thu Mar 25 11:13:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11706 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 A6BBCBDC66 for ; Thu, 25 Mar 2021 11:14:06 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E698168D65; Thu, 25 Mar 2021 12:14:05 +0100 (CET) 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="XRSBdCAv"; dkim-atps=neutral Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CDA42602D5 for ; Thu, 25 Mar 2021 12:14:04 +0100 (CET) Received: by mail-pg1-x52b.google.com with SMTP id u19so1427695pgh.10 for ; Thu, 25 Mar 2021 04:14:04 -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:mime-version :content-transfer-encoding; bh=swpYWXv8pZADKxhv3P2NWUwDpKqWYN1hW5XFmXO4vPk=; b=XRSBdCAvbdF007UwP3zl9/oD/xgjheZjw9k7zYDeJPrWFwIU5Z6jnvYVZo29mIDrTf /mo1rA8Eh5yvBeupEkwuM5iFoEiZLocfpxJt/dSH2TgsWVnKBpk7BQ3vzx3W1GqnTzLt AXPkQDHilqa7Tdata0xZsLsT7eTWidMI9rW0k= 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:mime-version :content-transfer-encoding; bh=swpYWXv8pZADKxhv3P2NWUwDpKqWYN1hW5XFmXO4vPk=; b=oYCt0kpygqFslnNGpb7xoSriek9MHu51jLzw/rt3bRe1k/9vRKFZtTzsB0a1R/Gh32 AekHmnxKtzrjyCTs6I3rk53ArnQ74ri4EMMRnOMGO2byVOBPjZTgPLlR/EXl9Tn1yI1S okeoxdv0oq7KFY7CuiUbFSNIDRKxWh8bz9YkElZm8yAhLuO5e2creFpSSN+8+ki2HcQC PcUNjHUzfT1iZcVMIuXN2hmOHOgWMsDpJ6Ayt/l2jnynSRefnljaU+1BixqSLo4cE/3Y NGHoVwKp1moMwsxN9Wbbo0IENrl1TUEVvdKovfCVYlQO+9/Dj1PaAdXkd8aSk9804coS UPSA== X-Gm-Message-State: AOAM532YX9pjVeiozcNytrPhULhrgU3LZsLXTuhoqKvSCeX0X75H2j39 WRfn9Wb29CQlv4r2GNEUnVolgLpVnjcl1A== X-Google-Smtp-Source: ABdhPJxfkU8KZ7YC4utiRteDq44Et/yWHyCms2ePgAGu3jfRFLBVlTp1JpaBiJIG6VU/T3qu2Yx0eA== X-Received: by 2002:a17:902:7682:b029:e6:2bc5:f005 with SMTP id m2-20020a1709027682b02900e62bc5f005mr9131613pll.32.1616670843146; Thu, 25 Mar 2021 04:14:03 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:8de:2f19:4b87:ef01]) by smtp.gmail.com with ESMTPSA id 202sm5503803pfu.46.2021.03.25.04.14.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Mar 2021 04:14:02 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 25 Mar 2021 20:13:56 +0900 Message-Id: <20210325111357.3862847-1-hiroh@chromium.org> X-Mailer: git-send-email 2.31.0.291.g576ba9dcdaf-goog MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 1/2] android: CameraDevice: Mark getResultMetadata() const function 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" CameraDevice::getResultMetadata() doesn't change either |descriptor| and member variables. It should be marked as a const function and |descriptor| should be passed with const lvalue reference. Signed-off-by: Hirokazu Honda Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/android/camera_device.cpp | 10 +++++----- src/android/camera_device.h | 3 ++- 2 files changed, 7 insertions(+), 6 deletions(-) -- 2.31.0.291.g576ba9dcdaf-goog diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 5fbf6f82..ae693664 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1924,7 +1924,7 @@ void CameraDevice::requestComplete(Request *request) * pipeline handlers) timestamp in the Request itself. */ uint64_t timestamp = buffers.begin()->second->metadata().timestamp; - resultMetadata = getResultMetadata(descriptor, timestamp); + resultMetadata = getResultMetadata(*descriptor, timestamp); /* Handle any JPEG compression. */ for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { @@ -2030,11 +2030,11 @@ void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream) * Produce a set of fixed result metadata. */ std::unique_ptr -CameraDevice::getResultMetadata(Camera3RequestDescriptor *descriptor, - int64_t timestamp) +CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor, + int64_t timestamp) const { - const ControlList &metadata = descriptor->request_->metadata(); - const CameraMetadata &settings = descriptor->settings_; + const ControlList &metadata = descriptor.request_->metadata(); + const CameraMetadata &settings = descriptor.settings_; camera_metadata_ro_entry_t entry; bool found; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 09c395ff..11bdfec8 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -102,7 +102,8 @@ private: libcamera::PixelFormat toPixelFormat(int format) const; int processControls(Camera3RequestDescriptor *descriptor); std::unique_ptr getResultMetadata( - Camera3RequestDescriptor *descriptor, int64_t timestamp); + const Camera3RequestDescriptor &descriptor, + int64_t timestamp) const; unsigned int id_; camera3_device_t camera3Device_; From patchwork Thu Mar 25 11:13:57 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11707 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 010E4BDC66 for ; Thu, 25 Mar 2021 11:14:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id AFF2B602D9; Thu, 25 Mar 2021 12:14:08 +0100 (CET) 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="SUoGsa69"; dkim-atps=neutral Received: from mail-pf1-x436.google.com (mail-pf1-x436.google.com [IPv6:2607:f8b0:4864:20::436]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 5C05C68D6A for ; Thu, 25 Mar 2021 12:14:06 +0100 (CET) Received: by mail-pf1-x436.google.com with SMTP id c17so1699571pfn.6 for ; Thu, 25 Mar 2021 04:14:06 -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=hSRtvNF41uec4k/kiSAF8ebI/JlRa3mzKNJUJKvB9tE=; b=SUoGsa69pDlshLSYI6GhI0T5nGNIeWNZMjW052qBxGe0yetEnAN+DEDGboBLQnEs47 XXNk2+W6BHpm0WQCCipoWWreoLYb2VKxPXOG5mXSM5xl21vqzj7fTvtY7nqLMYOwQHrd cXPuE8jCjoYlDqwum4G0OgmDWlRlxM15+6KKU= 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=hSRtvNF41uec4k/kiSAF8ebI/JlRa3mzKNJUJKvB9tE=; b=rZh/wxSUtvbiK4yFXxeYNZkOB5FvifefLeyTGm7LE6eZykGS9UJr4wTOReD/Wt2f6d 8p5VdZwCp4bRHUS8s3syT/EcdqTtnrcjzU3rzkeJMwhf5+UtQZrL0MTokuwUb4R5+iAY wkHK8p0Raw99wBmrued6AZtfI+qs1Lo2RQH8o9XgxkzRl4VXR2HmQGj8LSO1Y7JCYzaS 9znf2F44zuwJ0gLZdJleO8G90RyfcSA9M39AlyJyqHdQ4AWBTo+HQP6mpihuWz7/vP+n gi4PLVHWIPJ9hzsQJ3h1soNGNJN7iFN/Zrkt/crv0E4xlIZyuJnDiyXqLVoL2numWI+t 6AxA== X-Gm-Message-State: AOAM532oc5tr+flFEBf72K1RDsGPYBu9g1g+lYUcL5S4xeCFv7L61RIK AX80tG3btjzqwYRU9IY2pA8yPSEMca+EPg== X-Google-Smtp-Source: ABdhPJy41G1eiDHNfGTyzyljlO3/rGlCuWdtZcf7dqeUbdJgmTwccKZZmxoBoeOuHT1htLbTgegPCA== X-Received: by 2002:a17:902:8ec9:b029:e6:c5e:cf18 with SMTP id x9-20020a1709028ec9b02900e60c5ecf18mr9066996plo.47.1616670844661; Thu, 25 Mar 2021 04:14:04 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:8de:2f19:4b87:ef01]) by smtp.gmail.com with ESMTPSA id 202sm5503803pfu.46.2021.03.25.04.14.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 25 Mar 2021 04:14:04 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Thu, 25 Mar 2021 20:13:57 +0900 Message-Id: <20210325111357.3862847-2-hiroh@chromium.org> X-Mailer: git-send-email 2.31.0.291.g576ba9dcdaf-goog In-Reply-To: <20210325111357.3862847-1-hiroh@chromium.org> References: <20210325111357.3862847-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 2/2] android: CameraDevice: Fix Camera3RequestDescriptor leakage 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" CameraDevice creates Camera3RequestDescriptor in processCaptureRequest() and disallocates in requestComplete(). Camera3RequestDescriptor can never be destroyed if requestComplete() is never called. This avoid the memory leakage by storing them in map CamerarRequestDescriptor. Signed-off-by: Hirokazu Honda --- src/android/camera_device.cpp | 67 +++++++++++++++++++---------------- src/android/camera_device.h | 6 +++- src/android/camera_worker.cpp | 4 +-- src/android/camera_worker.h | 2 +- 4 files changed, 44 insertions(+), 35 deletions(-) -- 2.31.0.291.g576ba9dcdaf-goog diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ae693664..50b017a3 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -287,15 +287,17 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( /* * Create the libcamera::Request unique_ptr<> to tie its lifetime - * to the descriptor's one. Set the descriptor's address as the - * request's cookie to retrieve it at completion time. + * to the descriptor's one. */ - request_ = std::make_unique(camera, - reinterpret_cast(this)); + request_ = std::make_unique(camera); } +CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor() = default; CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor() = default; +CameraDevice::Camera3RequestDescriptor& +CameraDevice::Camera3RequestDescriptor::operator=(Camera3RequestDescriptor&&) = default; + /* * \class CameraDevice * @@ -1811,8 +1813,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 = - new Camera3RequestDescriptor(camera_.get(), camera3Request); + Camera3RequestDescriptor descriptor(camera_.get(), camera3Request); + /* * \todo The Android request model is incremental, settings passed in * previous requests are to be effective until overridden explicitly in @@ -1822,12 +1824,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (camera3Request->settings) lastSettings_ = camera3Request->settings; else - descriptor->settings_ = lastSettings_; + descriptor.settings_ = lastSettings_; - LOG(HAL, Debug) << "Queueing request " << descriptor->request_->cookie() - << " with " << descriptor->buffers_.size() << " streams"; - for (unsigned int i = 0; i < descriptor->buffers_.size(); ++i) { - const camera3_stream_buffer_t &camera3Buffer = descriptor->buffers_[i]; + LOG(HAL, Debug) << "Queueing request " << descriptor.request_->cookie() + << " with " << descriptor.buffers_.size() << " streams"; + for (unsigned int i = 0; i < descriptor.buffers_.size(); ++i) { + const camera3_stream_buffer_t &camera3Buffer = descriptor.buffers_[i]; camera3_stream *camera3Stream = camera3Buffer.stream; CameraStream *cameraStream = static_cast(camera3Stream->priv); @@ -1860,7 +1862,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * lifetime management only. */ buffer = createFrameBuffer(*camera3Buffer.buffer); - descriptor->frameBuffers_.emplace_back(buffer); + descriptor.frameBuffers_.emplace_back(buffer); LOG(HAL, Debug) << ss.str() << " (direct)"; break; @@ -1879,11 +1881,10 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (!buffer) { LOG(HAL, Error) << "Failed to create buffer"; - delete descriptor; return -ENOMEM; } - descriptor->request_->addBuffer(cameraStream->stream(), buffer, + descriptor.request_->addBuffer(cameraStream->stream(), buffer, camera3Buffer.acquire_fence); } @@ -1891,11 +1892,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * Translate controls from Android to libcamera and queue the request * to the CameraWorker thread. */ - int ret = processControls(descriptor); + int ret = processControls(&descriptor); if (ret) return ret; - worker_.queueRequest(descriptor->request_.get()); + worker_.queueRequest(descriptor.request_.get()); + descriptors_[descriptor.request_->cookie()] = std::move(descriptor); return 0; } @@ -1905,8 +1907,13 @@ void CameraDevice::requestComplete(Request *request) const Request::BufferMap &buffers = request->buffers(); camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; std::unique_ptr resultMetadata; - Camera3RequestDescriptor *descriptor = - reinterpret_cast(request->cookie()); + auto node = descriptors_.extract(request->cookie()); + if (node.empty()) { + LOG(HAL, Error) << "Unknown request: " << request->cookie(); + status = CAMERA3_BUFFER_STATUS_ERROR; + return; + } + Camera3RequestDescriptor& descriptor = node.mapped(); if (request->status() != Request::RequestComplete) { LOG(HAL, Error) << "Request not successfully completed: " @@ -1915,7 +1922,7 @@ void CameraDevice::requestComplete(Request *request) } LOG(HAL, Debug) << "Request " << request->cookie() << " completed with " - << descriptor->buffers_.size() << " streams"; + << descriptor.buffers_.size() << " streams"; /* * \todo The timestamp used for the metadata is currently always taken @@ -1924,10 +1931,10 @@ void CameraDevice::requestComplete(Request *request) * pipeline handlers) timestamp in the Request itself. */ uint64_t timestamp = buffers.begin()->second->metadata().timestamp; - resultMetadata = getResultMetadata(*descriptor, timestamp); + resultMetadata = getResultMetadata(descriptor, timestamp); /* Handle any JPEG compression. */ - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { CameraStream *cameraStream = static_cast(buffer.stream->priv); @@ -1942,7 +1949,7 @@ void CameraDevice::requestComplete(Request *request) int ret = cameraStream->process(*src, *buffer.buffer, - descriptor->settings_, + descriptor.settings_, resultMetadata.get()); if (ret) { status = CAMERA3_BUFFER_STATUS_ERROR; @@ -1959,17 +1966,17 @@ void CameraDevice::requestComplete(Request *request) /* Prepare to call back the Android camera stack. */ camera3_capture_result_t captureResult = {}; - captureResult.frame_number = descriptor->frameNumber_; - captureResult.num_output_buffers = descriptor->buffers_.size(); - for (camera3_stream_buffer_t &buffer : descriptor->buffers_) { + captureResult.frame_number = descriptor.frameNumber_; + captureResult.num_output_buffers = descriptor.buffers_.size(); + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { buffer.acquire_fence = -1; buffer.release_fence = -1; buffer.status = status; } - captureResult.output_buffers = descriptor->buffers_.data(); + captureResult.output_buffers = descriptor.buffers_.data(); if (status == CAMERA3_BUFFER_STATUS_OK) { - notifyShutter(descriptor->frameNumber_, timestamp); + notifyShutter(descriptor.frameNumber_, timestamp); captureResult.partial_result = 1; captureResult.result = resultMetadata->get(); @@ -1982,13 +1989,11 @@ void CameraDevice::requestComplete(Request *request) * is here signalled. Make sure the error path plays well with * the camera stack state machine. */ - notifyError(descriptor->frameNumber_, - descriptor->buffers_[0].stream); + notifyError(descriptor.frameNumber_, + descriptor.buffers_[0].stream); } callbacks_->process_capture_result(callbacks_, &captureResult); - - delete descriptor; } std::string CameraDevice::logPrefix() const diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 11bdfec8..54d7c319 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -69,9 +69,11 @@ private: CameraDevice(unsigned int id, std::shared_ptr camera); struct Camera3RequestDescriptor { + Camera3RequestDescriptor(); + ~Camera3RequestDescriptor(); Camera3RequestDescriptor(libcamera::Camera *camera, const camera3_capture_request_t *camera3Request); - ~Camera3RequestDescriptor(); + Camera3RequestDescriptor& operator=(Camera3RequestDescriptor&&); uint32_t frameNumber_; std::vector buffers_; @@ -122,6 +124,8 @@ private: std::map formatsMap_; std::vector streams_; + std::map descriptors_; + std::string maker_; std::string model_; diff --git a/src/android/camera_worker.cpp b/src/android/camera_worker.cpp index df436460..300ddde0 100644 --- a/src/android/camera_worker.cpp +++ b/src/android/camera_worker.cpp @@ -26,10 +26,10 @@ LOG_DECLARE_CATEGORY(HAL) * by the CameraWorker which queues it to the libcamera::Camera after handling * fences. */ -CaptureRequest::CaptureRequest(libcamera::Camera *camera, uint64_t cookie) +CaptureRequest::CaptureRequest(libcamera::Camera *camera) : camera_(camera) { - request_ = camera_->createRequest(cookie); + request_ = camera_->createRequest(reinterpret_cast(this)); } void CaptureRequest::addBuffer(Stream *stream, FrameBuffer *buffer, int fence) diff --git a/src/android/camera_worker.h b/src/android/camera_worker.h index d7060576..64b1658b 100644 --- a/src/android/camera_worker.h +++ b/src/android/camera_worker.h @@ -22,7 +22,7 @@ class CameraDevice; class CaptureRequest { public: - CaptureRequest(libcamera::Camera *camera, uint64_t cookie); + CaptureRequest(libcamera::Camera *camera); const std::vector &fences() const { return acquireFences_; } libcamera::ControlList &controls() { return request_->controls(); }