From patchwork Mon Mar 29 06:11:18 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11758 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 247BBC32EF for ; Mon, 29 Mar 2021 06:11:38 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id D835A68784; Mon, 29 Mar 2021 08:11:37 +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="nIbxirn7"; dkim-atps=neutral Received: from mail-pf1-x42e.google.com (mail-pf1-x42e.google.com [IPv6:2607:f8b0:4864:20::42e]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id AA1BA602D2 for ; Mon, 29 Mar 2021 08:11:36 +0200 (CEST) Received: by mail-pf1-x42e.google.com with SMTP id v10so3841340pfn.5 for ; Sun, 28 Mar 2021 23:11:36 -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=O3rAouR/WcVO9ATvKCqEXZXKFK3P+Zkr39OzYNkKz5E=; b=nIbxirn7hUFnEj4XfszecVsOjgF2cFZC+or2w3vSOD6lXb2AJAoxjw4dDpq6sgeRNN ZNwgg9e5HDOWXt+Z0sFdXmbn41uHCr56ID1rD2HQck1L1CJiF24WRENp6r96wCshDBJF w+bawznfODUCDG+U7ostmzvedL1sTUL9VcuMI= 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=O3rAouR/WcVO9ATvKCqEXZXKFK3P+Zkr39OzYNkKz5E=; b=um6kkRrnPmpckEIM7DyWL+YLv4Yw95EdUK7rO8+xEv85D7gxvYz0/D1IPefWVOJDAy Wu2ClCv1S8KSp78Fee1/zbbE8Q9Ewudz6YSeoBWTYUKrEmV4hb3LRMB7+YHhF5/P9YMl 84wsIVB04wZpwzD/ebRaSWaMf89ZuoXY8FWF7QEnnWbuLtng97KYieR74cY6DRgyGRjZ PXyxGHuQgz8U+6N6YcDGpRt6V6iPY5+doblEoNsZeag4aZ2+89H6z8evI8Xtw8B2WJGo ZgIvQ5M0+JFZQNzeyKOFIVKC6KNTgaCuioeTBUJ1uLFWyxV41RHfXHcQ7ri1yFrK1g46 K8hA== X-Gm-Message-State: AOAM531HGiJFrBOhHmHPAEWv6x5nbBMatxO2wRBPZ2yz1jb7EMZ/2Ojl uczdfzlgtPLrrYU03BCrfEtSMWge+p4nig== X-Google-Smtp-Source: ABdhPJwPdCLMOKfcBYA+1jWrNAE7VIoSSv1RgnIuAwu+Oql4Gc81vGzHJn04rT9O5+MR5Ktr15CmDA== X-Received: by 2002:a63:c647:: with SMTP id x7mr22640470pgg.15.1616998286720; Sun, 28 Mar 2021 23:11:26 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:1db7:ae60:9288:b906]) by smtp.gmail.com with ESMTPSA id f2sm16445975pfq.129.2021.03.28.23.11.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 28 Mar 2021 23:11:26 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Mar 2021 15:11:18 +0900 Message-Id: <20210329061119.135513-2-hiroh@chromium.org> X-Mailer: git-send-email 2.31.0.291.g576ba9dcdaf-goog In-Reply-To: <20210329061119.135513-1-hiroh@chromium.org> References: <20210329061119.135513-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 1/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 CameraRequestDescriptor. Signed-off-by: Hirokazu Honda --- src/android/camera_device.cpp | 67 +++++++++++++++++++---------------- src/android/camera_device.h | 8 +++-- src/android/camera_worker.cpp | 4 +-- src/android/camera_worker.h | 2 +- 4 files changed, 45 insertions(+), 36 deletions(-) -- 2.31.0.291.g576ba9dcdaf-goog diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index ae693664..02d3bfb2 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..5a2d22d6 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -69,11 +69,13 @@ 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_; + uint32_t frameNumber_ = 0; std::vector buffers_; std::vector> frameBuffers_; CameraMetadata settings_; @@ -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(); } From patchwork Mon Mar 29 06:11:19 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hirokazu Honda X-Patchwork-Id: 11759 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 A749AC32EF for ; Mon, 29 Mar 2021 06:11:41 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 61C3C6877D; Mon, 29 Mar 2021 08:11: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="jWwSpxw9"; dkim-atps=neutral Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B4E54602D2 for ; Mon, 29 Mar 2021 08:11:38 +0200 (CEST) Received: by mail-pf1-x42d.google.com with SMTP id 11so9182297pfn.9 for ; Sun, 28 Mar 2021 23:11: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=S0BbWN1J8xV7FErOAojsZ+fHvtyZlbls2qp4CgRYcQ4=; b=jWwSpxw90Kjo08Ou2TUbPCIq0bd0COdC+9xh76Hgung+cnjRCY9hA/gKI8hoTKqoQa GaMPKy600FaENYsYe8PeXJ86Avpj+Y1/d+kyzxDBQ5F3vPp64dVa4JuRLYcWKp+j8s1i 7tG3JAiQGgLoHiwqPg5hcbVSEP16cluJI/fkM= 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=S0BbWN1J8xV7FErOAojsZ+fHvtyZlbls2qp4CgRYcQ4=; b=eS11pEWLNPxHpsS5xgSNALMB0heOjNPAdNLiHSgGOUqMUd7LaqGeFWpIAeDq9PEdqq xQtLjxWkZYTDmZOFnLe5V6qaJREqRSMgV7bzm9FCn/X5z2KOHn/A+bAJZIfXID/ZUvzD wrUXUqufN8dX2K1NilDairnoL+qrjLJ5Jyx9welWtRoPmTaYdwpffzPvTSYEjYrkvji6 cnO4s3E15Zz7/gUs4eBnuKwxhUAeODFLX9bXjsoFbZ0Bn3CyAJFxv1XwUZ3TqyZ4IBu4 C0Nqdlx5lkqYuKIqqDlrBSZruYeRjeYruSEz5Edf8UYgnJ7HFN3rZxvWd7UTAdU04d81 TArg== X-Gm-Message-State: AOAM530bgL6oM7Ihxzr6MsDPwZI+vBqCKq37IEBiykO/oA8w5EpcravY ZQpKy33wcW/RS5V3fEzD6hfXFPBQsYisDA== X-Google-Smtp-Source: ABdhPJwOpgZl0M8F6EAcKF4dayMie13R5+QQgBhxiROQhzXwr1mMQ44bbmAiYl3el/LDbE5M00fxng== X-Received: by 2002:aa7:9431:0:b029:1f1:52fd:5444 with SMTP id y17-20020aa794310000b02901f152fd5444mr23377479pfo.47.1616998288304; Sun, 28 Mar 2021 23:11:28 -0700 (PDT) Received: from hiroh2.tok.corp.google.com ([2401:fa00:8f:2:1db7:ae60:9288:b906]) by smtp.gmail.com with ESMTPSA id f2sm16445975pfq.129.2021.03.28.23.11.27 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 28 Mar 2021 23:11:27 -0700 (PDT) From: Hirokazu Honda To: libcamera-devel@lists.libcamera.org Date: Mon, 29 Mar 2021 15:11:19 +0900 Message-Id: <20210329061119.135513-3-hiroh@chromium.org> X-Mailer: git-send-email 2.31.0.291.g576ba9dcdaf-goog In-Reply-To: <20210329061119.135513-1-hiroh@chromium.org> References: <20210329061119.135513-1-hiroh@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 2/2] android: CameraDevice: Add stop() 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" This adds CameraDevice::stop(), which cleans up the member variables of CameraDevice. It is called in CameraDevice::close() and CameraDevice::configureStreams(). Signed-off-by: Hirokazu Honda --- src/android/camera_device.cpp | 17 +++++++++-------- src/android/camera_device.h | 2 ++ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 02d3bfb2..d5447d8e 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -659,12 +659,17 @@ int CameraDevice::open(const hw_module_t *hardwareModule) void CameraDevice::close() { - streams_.clear(); + stop(); + + camera_->release(); +} +void CameraDevice::stop() +{ worker_.stop(); camera_->stop(); - camera_->release(); + descriptors_.clear(); running_ = false; } @@ -1547,12 +1552,8 @@ PixelFormat CameraDevice::toPixelFormat(int format) const */ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) { - /* Before any configuration attempt, stop the camera if it's running. */ - if (running_) { - worker_.stop(); - camera_->stop(); - running_ = false; - } + /* Before any configuration attempt, stop the camera. */ + stop(); /* * Generate an empty configuration, and construct a StreamConfiguration diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 5a2d22d6..cc12fe20 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -87,6 +87,8 @@ private: int androidFormat; }; + void stop(); + int initializeStreamConfigurations(); std::vector getYUVResolutions(libcamera::CameraConfiguration *cameraConfig,