From patchwork Fri Jul 3 14:54:04 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kieran Bingham X-Patchwork-Id: 8589 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 5AF7ABE905 for ; Fri, 3 Jul 2020 14:54:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id CB0F860C57; Fri, 3 Jul 2020 16:54:15 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="Ven/Dpnd"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 217CF603AE for ; Fri, 3 Jul 2020 16:54:15 +0200 (CEST) Received: from Q.local (cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7BA7029E; Fri, 3 Jul 2020 16:54:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1593788054; bh=6SSTFk9rhZ9i+iI5UOPrSzWaCHz0gAYr9gjEat/a1Qg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Ven/DpndHftOQ3JrkMKlkKQamLtTbsmQiPwTY178v8N3kXpeWm/mXmFOBWbOZ9JPn 71BlIAOWCsTAuL8Hwgq6auQx3PY/tTtl+01Tp3fRvQoiw2/aGKAs8Flu+TL0D+kBb4 g94B1uj1lXhQwIa8k6oxMEygqFXin+BHMAJJJBIE= From: Kieran Bingham To: libcamera devel Date: Fri, 3 Jul 2020 15:54:04 +0100 Message-Id: <20200703145404.2246129-1-kieran.bingham@ideasonboard.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200703123919.2223048-8-kieran.bingham@ideasonboard.com> References: <20200703123919.2223048-8-kieran.bingham@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3.1 7/8] android: camera_device: Add buffers for each stream to Requests 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" Construct a FrameBuffer for every buffer given in the camera3Request and add it to the libcamera Request on the appropriate stream. The correct stream is obtained from the private data of the camera3_stream associated with the camera3_buffer. Comments regarding supporting only one buffer are now removed. Signed-off-by: Kieran Bingham Reviewed-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- src/android/camera_device.cpp | 58 ++++++++++++++++++++--------------- 1 file changed, 33 insertions(+), 25 deletions(-) Rather than resend a v4 just for the change in this patch - sending a v3.1 for direct review of this one. Within requestComplete(), the 'first' buffer pointer, which is needed to extract the timestamp is moved to directly before it is used, and a comment added to highlight that this is using a single buffer purely for the timestamp. Then, after calling the process_capture_result callback to the android hal, we iterate all buffers in the request and delete them to prevent leaks. diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 28334751a26e..72be4ab86e50 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -991,6 +991,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) /* Maintain internal state of all stream mappings. */ streams_[i].index = streamIndex++; + stream->priv = static_cast(&streams_[i]); } switch (config_->validate()) { @@ -1049,9 +1050,6 @@ FrameBuffer *CameraDevice::createFrameBuffer(const buffer_handle_t camera3buffer int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request) { - StreamConfiguration *streamConfiguration = &config_->at(0); - Stream *stream = streamConfiguration->stream(); - if (camera3Request->num_output_buffers != 1) { LOG(HAL, Error) << "Invalid number of output buffers: " << camera3Request->num_output_buffers; @@ -1089,30 +1087,32 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques camera_->createRequest(reinterpret_cast(descriptor)); for (unsigned int i = 0; i < descriptor->numBuffers; ++i) { + CameraStream *cameraStream = static_cast(camera3Buffers[i].stream->priv); + /* * Keep track of which stream the request belongs to and store * the native buffer handles. - * - * \todo Currently we only support one capture buffer. Copy - * all of them to be ready once we'll support more. */ descriptor->buffers[i].stream = camera3Buffers[i].stream; descriptor->buffers[i].buffer = camera3Buffers[i].buffer; - } - /* - * Create a libcamera buffer using the dmabuf descriptors of the first - * and (currently) only supported request buffer. - */ - FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer); - if (!buffer) { - LOG(HAL, Error) << "Failed to create buffer"; - delete request; - delete descriptor; - return -ENOMEM; - } + /* + * Create a libcamera buffer using the dmabuf descriptors of the + * first and (currently) only supported request buffer. + */ + FrameBuffer *buffer = createFrameBuffer(*camera3Buffers[0].buffer); + if (!buffer) { + LOG(HAL, Error) << "Failed to create buffer"; + delete request; + delete descriptor; + return -ENOMEM; + } - request->addBuffer(stream, buffer); + StreamConfiguration *streamConfiguration = &config_->at(cameraStream->index); + Stream *stream = streamConfiguration->stream(); + + request->addBuffer(stream, buffer); + } int ret = camera_->queueRequest(request); if (ret) { @@ -1128,7 +1128,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques void CameraDevice::requestComplete(Request *request) { const std::map &buffers = request->buffers(); - FrameBuffer *buffer = buffers.begin()->second; camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; std::unique_ptr resultMetadata; @@ -1146,10 +1145,6 @@ void CameraDevice::requestComplete(Request *request) captureResult.frame_number = descriptor->frameNumber; captureResult.num_output_buffers = descriptor->numBuffers; for (unsigned int i = 0; i < descriptor->numBuffers; ++i) { - /* - * \todo Currently we only support one capture buffer. Prepare - * all of them to be ready once we'll support more. - */ descriptor->buffers[i].acquire_fence = -1; descriptor->buffers[i].release_fence = -1; descriptor->buffers[i].status = status; @@ -1157,6 +1152,14 @@ void CameraDevice::requestComplete(Request *request) captureResult.output_buffers = const_cast(descriptor->buffers); + /* + * \todo The timestamp used for the metadata is currently always taken + * from the first buffer (which may be the first stream) in the Request. + * It might be appropriate to return a 'correct' (as determined by + * pipeline handlers) timestamp in the Request itself. + */ + FrameBuffer *buffer = buffers.begin()->second; + if (status == CAMERA3_BUFFER_STATUS_OK) { notifyShutter(descriptor->frameNumber, buffer->metadata().timestamp); @@ -1180,8 +1183,13 @@ void CameraDevice::requestComplete(Request *request) callbacks_->process_capture_result(callbacks_, &captureResult); + /* Release all buffers created with createFrameBuffer(). */ + for (auto it : buffers) { + FrameBuffer *buffer = it.second; + delete buffer; + } + delete descriptor; - delete buffer; } std::string CameraDevice::logPrefix() const