From patchwork Fri Jul 17 08:54:08 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 8846 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 9EA78C0109 for ; Fri, 17 Jul 2020 08:54:27 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6D00A6053D; Fri, 17 Jul 2020 10:54:27 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="VylYYNF0"; dkim-atps=neutral Received: from mail-wr1-x434.google.com (mail-wr1-x434.google.com [IPv6:2a00:1450:4864:20::434]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 931EF6053B for ; Fri, 17 Jul 2020 10:54:23 +0200 (CEST) Received: by mail-wr1-x434.google.com with SMTP id b6so10127695wrs.11 for ; Fri, 17 Jul 2020 01:54:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=qfb6/FkqmhJZigVXV/YSoZj/h9o6nTV54RylKnKZHpo=; b=VylYYNF03KvMg2f79/WBne/jGL1ineHDUOo1kYZuUWNl3fzW9uBtUEFOVse1++8l57 BFNGELLZx4n/vmsvU9hjnEJAyZai51VGMhlRZOCF1vPcsz/2VKmpHBAN+dlPPGtLhARr SogL/gaDXytkIuyP9sIbSN4FqQBwG9ZG5p+6Dv3mFR4jZH6R9Vg8FzI4+6n0u4lXh06M /GanyNp8KBPS50xtf0nD68omm5C402eRW+S1gh3rCyf4w9x4n/Ek9ET2wzic/bfEtpAX Qee9hqjfmymd0PgSYTRLocVvMtx+c+LoNu/rVtrMqxB0qYPqxzEEW5+52D/9oClLvJBa 5MfQ== 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=qfb6/FkqmhJZigVXV/YSoZj/h9o6nTV54RylKnKZHpo=; b=B+9s53DYmad/0kq6DfcGpsM7l5AW1PxsdLSKw8VuY7raj4FoxpzrOmC/UjCxAoQLrR YhWMwaN/2E8Y+ka6mPBvos/IZlG+E297VXXm9SbQb0ijSrFQNSwqygvymiqMwaMwm32w IlBx+LilkyQs8EjYtHUiLe/C4JG0QRTg0KUNVuevsdEy6WjbPE2XsEx8eK3G8vg26DKt MdBPPAVjQUsVxw5E4B65eJo2y/8l+8NG69wWpYCxgbuW/tjbjwDopBvvgHYr3mgoHosw OIzqtgyTozyXfkGltrmCQPAqVjtO6888qmLpZJbyxJ9bM40GzgRagsZ+ouBDrehtu80C YyDw== X-Gm-Message-State: AOAM532+3ZvLzCrrrMQqLtWDcKIvENvJB8GGoGbmrJqNgo3/DWvQ5AHn y1wwowOUKzpwcKJtuC2aqJ2GTcwIQkE= X-Google-Smtp-Source: ABdhPJx2NFN6AXPvbiM3o1/3MXrBCsRaKSmsXa8c3ADn7stp6kRS1PKCO346MLQcusCgW8yg9iDtvg== X-Received: by 2002:a5d:684f:: with SMTP id o15mr9401338wrw.148.1594976062667; Fri, 17 Jul 2020 01:54:22 -0700 (PDT) Received: from naushir-VirtualBox.patuck.local ([88.97.76.4]) by smtp.gmail.com with ESMTPSA id 92sm14582375wrr.96.2020.07.17.01.54.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Jul 2020 01:54:22 -0700 (PDT) From: Naushir Patuck To: libcamera-devel@lists.libcamera.org Date: Fri, 17 Jul 2020 09:54:08 +0100 Message-Id: <20200717085410.732308-9-naush@raspberrypi.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200717085410.732308-1-naush@raspberrypi.com> References: <20200717085410.732308-1-naush@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 08/10] libcamera: pipeline: raspberrypi: Add more robust stream buffer logic 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" Add further queueing into the RPiStream object to ensure that we always follow the buffer ordering (be it internal or external) given by incoming Requests. This is essential, otherwise we risk dropping frames that are meant to be part of a Request, and can cause the pipeline to stall indefinitely. This also prevents any possibility of mismatched frame buffers going through the pipeline and out to the application. Signed-off-by: Naushir Patuck Reviewed-by: Niklas Söderlund --- .../pipeline/raspberrypi/rpi_stream.cpp | 73 ++++++++++++++++--- .../pipeline/raspberrypi/rpi_stream.h | 12 ++- 2 files changed, 73 insertions(+), 12 deletions(-) diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp index 53a335e3..4818117e 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp @@ -112,25 +112,34 @@ int RPiStream::queueBuffer(FrameBuffer *buffer) */ if (!buffer) { if (availableBuffers_.empty()) { - LOG(RPISTREAM, Warning) << "No buffers available for " + LOG(RPISTREAM, Info) << "No buffers available for " << name_; - return -EINVAL; + /* + * Note that we need to requeue an internal buffer as soon + * as one becomes available. + */ + requeueBuffers_.push(nullptr); + return 0; } buffer = availableBuffers_.front(); availableBuffers_.pop(); } - LOG(RPISTREAM, Debug) << "Queuing buffer " << buffer->cookie() - << " for " << name_; - - int ret = dev_->queueBuffer(buffer); - if (ret) { - LOG(RPISTREAM, Error) << "Failed to queue buffer for " - << name_; + if (requeueBuffers_.empty()) { + /* + * No earlier requests are pending to be queued, so we can + * go ahead and queue the buffer into the device. + */ + return queueToDevice(buffer); + } else { + /* + * There are earlier buffers to be queued, so this buffer + * must go on the waiting list. + */ + requeueBuffers_.push(buffer); + return 0; } - - return ret; } void RPiStream::returnBuffer(FrameBuffer *buffer) @@ -138,7 +147,36 @@ void RPiStream::returnBuffer(FrameBuffer *buffer) /* This can only be called for external streams. */ assert(external_); + /* Push this buffer back into the queue to be used again. */ availableBuffers_.push(buffer); + + /* + * Do we have any buffers that are waiting to be queued? + * If so, do it now as availableBuffers_ will not be empty. + */ + while (!requeueBuffers_.empty()) { + FrameBuffer *buffer = requeueBuffers_.front(); + + if (!buffer && availableBuffers_.empty()) { + /* + * We want to queue an internal buffer, but none + * are available. Can't do anything, quit the loop. + */ + break; + } + + if (!buffer) { + /* + * We want to queue an internal buffer, and at least one + * is available. + */ + buffer = availableBuffers_.front(); + availableBuffers_.pop(); + } + + requeueBuffers_.pop(); + queueToDevice(buffer); + } } bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const @@ -155,10 +193,23 @@ bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const void RPiStream::clearBuffers() { availableBuffers_ = std::queue{}; + requeueBuffers_ = std::queue{}; internalBuffers_.clear(); bufferList_.clear(); } +int RPiStream::queueToDevice(FrameBuffer *buffer) +{ + LOG(RPISTREAM, Debug) << "Queuing buffer " << buffer->cookie() + << " for " << name_; + + int ret = dev_->queueBuffer(buffer); + if (ret) + LOG(RPISTREAM, Error) << "Failed to queue buffer for " + << name_; + return ret; +} + } /* namespace RPi */ } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h index 019e236d..6c8dfa83 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h @@ -51,6 +51,7 @@ public: private: void clearBuffers(); + int queueToDevice(FrameBuffer *buffer); /* * Indicates that this stream is active externally, i.e. the buffers * might be provided by (and returned to) the application. @@ -62,7 +63,7 @@ private: std::string name_; /* The actual device stream. */ std::unique_ptr dev_; - /* All framebuffers associated with this device stream. */ + /* All frame buffers associated with this device stream. */ std::vector bufferList_; /* * List of frame buffer that we can use if none have been provided by @@ -70,6 +71,15 @@ private: * buffers exported internally. */ std::queue availableBuffers_; + /* + * List of frame buffer that are to be re-queued into the device. + * A nullptr indicates any internal buffer can be used (from availableBuffers_), + * whereas a valid pointer indicates an external buffer to be queued. + * + * Ordering buffers to be queued is important here as it must match the + * requests coming from the application. + */ + std::queue requeueBuffers_; /* * This is a list of buffers exported internally. Need to keep this around * as the stream needs to maintain ownership of these buffers.