[{"id":19045,"web_url":"https://patchwork.libcamera.org/comment/19045/","msgid":"","date":"2021-08-25T09:26:39","subject":"Re: [libcamera-devel] [PATCH v8 03/17] libcamera: pipeline:\n\traspberrypi: Don't rely on bufferCount","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Nicolas,\n\nThank you for your work!\nThe change looks good, just some (mostly cosmetic related) comments from me\nbelow.\n\nOn Tue, 24 Aug 2021 at 20:57, Nícolas F. R. A. Prado <\nnfraprado@collabora.com> wrote:\n\n> Currently the raspberrypi pipeline handler relies on bufferCount to\n> decide on the number of buffers to allocate internally and for the\n> number of V4L2 buffer slots to reserve. Instead, the number of internal\n> buffers should be the minimum required by the pipeline to keep the\n> requests flowing, in order to avoid wasting memory, while the number of\n> V4L2 buffer slots should be greater than the expected number of requests\n> queued, in order to avoid thrashing dmabuf mappings, which would degrade\n> performance.\n>\n> Extend the Stream class to receive the number of internal buffers that\n> should be allocated, and optionally the number of buffer slots to\n> reserve. Use these new member variables to specify better suited numbers\n> for internal buffers and buffer slots for each stream individually,\n> which also allows us to remove bufferCount.\n>\n> Signed-off-by: Nícolas F. R. A. Prado \n>\n> ---\n>\n> Changes in v8:\n> - Reworked buffer allocation handling in the raspberrypi pipeline handler\n> - New\n>\n> .../pipeline/raspberrypi/raspberrypi.cpp | 45 +++++++++++--------\n> .../pipeline/raspberrypi/rpi_stream.cpp | 28 +++++++++---\n> .../pipeline/raspberrypi/rpi_stream.h | 24 ++++++++--\n> 3 files changed, 69 insertions(+), 28 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 8a1040aa46be..a7c1cc1d5001 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -262,6 +262,25 @@ public:\n> bool match(DeviceEnumerator *enumerator) override;\n>\n> private:\n> + /*\n> + * The number of buffers to allocate internally for transferring\n> raw\n> + * frames from the Unicam Image stream to the ISP Input stream. It\n> is\n> + * defined such that:\n> + * - one buffer is being written to in Unicam Image\n> + * - one buffer is being processed in ISP Input\n> + * - one extra buffer is queued to Unicam Image\n> + */\n> + static constexpr unsigned int kInternalRawBufferCount = 3;\n>\n+\n> + /*\n> + * The number of buffers to allocate internally for the external\n>\n+ * streams. They're used to drop the first few frames while the IPA\n> + * algorithms converge. It is defined such that:\n> + * - one buffer is being used on the stream\n> + * - one extra buffer is queued\n> + */\n>\n\ns/external/ISP since the Unicam RAW image can also be an external stream.\n\nAdditionally, I would add that not only are these buffers used to drop the\nfirst few\nframes, but they are also used when the user does not supply a buffer in\nthe Request.\nFor example, if a low res is not requested by the application, we use an\ninternal\nbuffer since low res images are used for our colour denoise algorithm.\n\n\n> + static constexpr unsigned int kInternalDropoutBufferCount = 2;\n> +\n>\n\nI would probably rename these variables like:\n\ns/kInternalRawBufferCount/unicamInternalBufferCount/\ns/kInternalDropoutBufferCount/ispInternalBufferCount/\n\nI know that other libcamera source files use the \"k\" const prefix, but\nRaspberry Pi\nsource files don't, so, I would drop it to be consistent. The reason for\nthe rename is\nto more closely match the buffer usage as per the above comment.\n\n\n> RPiCameraData *cameraData(Camera *camera)\n> {\n> return static_cast(camera->_d());\n> @@ -971,14 +990,14 @@ bool PipelineHandlerRPi::match(DeviceEnumerator\n> *enumerator)\n> return false;\n>\n> /* Locate and open the unicam video streams. */\n> - data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\",\n> unicam_->getEntityByName(\"unicam-embedded\"));\n> - data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\",\n> unicam_->getEntityByName(\"unicam-image\"));\n> + data->unicam_[Unicam::Embedded] = RPi::Stream(\"Unicam Embedded\",\n> unicam_->getEntityByName(\"unicam-embedded\"), kInternalRawBufferCount);\n> + data->unicam_[Unicam::Image] = RPi::Stream(\"Unicam Image\",\n> unicam_->getEntityByName(\"unicam-image\"), kInternalRawBufferCount);\n>\n> /* Tag the ISP input stream as an import stream. */\n> - data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\",\n> isp_->getEntityByName(\"bcm2835-isp0-output0\"), true);\n> - data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture1\"));\n> - data->isp_[Isp::Output1] = RPi::Stream(\"ISP Output1\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture2\"));\n> - data->isp_[Isp::Stats] = RPi::Stream(\"ISP Stats\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture3\"));\n> + data->isp_[Isp::Input] = RPi::Stream(\"ISP Input\",\n> isp_->getEntityByName(\"bcm2835-isp0-output0\"), 0, kInternalRawBufferCount,\n> true);\n> + data->isp_[Isp::Output0] = RPi::Stream(\"ISP Output0\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture1\"),\n> kInternalDropoutBufferCount);\n> + data->isp_[Isp::Output1] = RPi::Stream(\"ISP Output1\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture2\"),\n> kInternalDropoutBufferCount);\n> + data->isp_[Isp::Stats] = RPi::Stream(\"ISP Stats\",\n> isp_->getEntityByName(\"bcm2835-isp0-capture3\"),\n> kInternalDropoutBufferCount);\n>\n>\nSome of these lines are over the 120 character limit and will need\nwrapping.\n\n\n> /* Wire up all the buffer connections. */\n> data->unicam_[Unicam::Image].dev()->frameStart.connect(data.get(),\n> &RPiCameraData::frameStarted);\n> @@ -1153,19 +1172,9 @@ int PipelineHandlerRPi::prepareBuffers(Camera\n> *camera)\n> RPiCameraData *data = cameraData(camera);\n> int ret;\n>\n> - /*\n> - * Decide how many internal buffers to allocate. For now, simply\n> look\n> - * at how many external buffers will be provided. We'll need to\n> improve\n> - * this logic. However, we really must have all streams allocate\n> the same\n> - * number of buffers to simplify error handling in\n> queueRequestDevice().\n> - */\n> - unsigned int maxBuffers = 0;\n> - for (const Stream *s : camera->streams())\n> - if (static_cast(s)->isExternal())\n> - maxBuffers = std::max(maxBuffers,\n> s->configuration().bufferCount);\n> -\n> + /* Allocate internal buffers. */\n> for (auto const stream : data->streams_) {\n> - ret = stream->prepareBuffers(maxBuffers);\n> + ret = stream->prepareBuffers();\n> if (ret < 0)\n> return ret;\n> }\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> index b3265d0e8aab..98572abc2f61 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -84,14 +84,24 @@ void Stream::removeExternalBuffer(FrameBuffer *buffer)\n> bufferMap_.erase(id);\n> }\n>\n> -int Stream::prepareBuffers(unsigned int count)\n> +int Stream::prepareBuffers()\n> {\n> + unsigned int slotCount;\n> int ret;\n>\n> if (!importOnly_) {\n> - if (count) {\n> + /*\n> + * External streams overallocate buffer slots in order to\n> handle\n> + * the buffers queued from applications without degrading\n> + * performance. Internal streams only need to have enough\n> buffer\n> + * slots to have the internal buffers queued.\n> + */\n> + slotCount = external_ ? kRPIExternalBufferSlotCount\n> + : internalBufferCount_;\n> +\n> + if (internalBufferCount_) {\n> /* Export some frame buffers for internal use. */\n> - ret = dev_->exportBuffers(count,\n> &internalBuffers_);\n> + ret = dev_->exportBuffers(internalBufferCount_,\n> &internalBuffers_);\n> if (ret < 0)\n> return ret;\n>\n> @@ -102,12 +112,16 @@ int Stream::prepareBuffers(unsigned int count)\n> for (auto const &buffer : internalBuffers_)\n> availableBuffers_.push(buffer.get());\n> }\n> -\n> - /* We must import all internal/external exported buffers.\n> */\n> - count = bufferMap_.size();\n> + } else {\n> + /*\n> + * An importOnly stream doesn't have its own internal\n> buffers,\n> + * so it needs to have the number of buffer slots to\n> reserve\n> + * explicitly declared.\n> + */\n> + slotCount = bufferSlotCount_;\n> }\n>\n> - return dev_->importBuffers(count);\n> + return dev_->importBuffers(slotCount);\n> }\n>\n> int Stream::queueBuffer(FrameBuffer *buffer)\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> index f1ac715f4221..7310ba1cc371 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -36,9 +36,13 @@ public:\n> {\n> }\n>\n> - Stream(const char *name, MediaEntity *dev, bool importOnly = false)\n> + Stream(const char *name, MediaEntity *dev,\n> + unsigned int internalBufferCount,\n> + unsigned int bufferSlotCount = 0, bool importOnly = false)\n> : external_(false), importOnly_(importOnly), name_(name),\n> - dev_(std::make_unique(dev)),\n> id_(ipa::RPi::MaskID)\n> + dev_(std::make_unique(dev)),\n> id_(ipa::RPi::MaskID),\n> + internalBufferCount_(internalBufferCount),\n> + bufferSlotCount_(bufferSlotCount)\n>\n\nI wonder if we can simplify this constructor by removing \"bool importOnly =\nfalse\"?\nIn your patch, If a non-zero bufferSlotCount is provided, it implies\nimportOnly == true.\nCould we use that to remove the latter? Perhaps renaming bufferSlotCount to\nimportSlots to be more clear?\n\n\n> {\n> }\n>\n> @@ -57,7 +61,7 @@ public:\n> void setExternalBuffer(FrameBuffer *buffer);\n> void removeExternalBuffer(FrameBuffer *buffer);\n>\n> - int prepareBuffers(unsigned int count);\n> + int prepareBuffers();\n> int queueBuffer(FrameBuffer *buffer);\n> void returnBuffer(FrameBuffer *buffer);\n>\n> @@ -65,6 +69,8 @@ public:\n> void releaseBuffers();\n>\n> private:\n> + static constexpr unsigned int kRPIExternalBufferSlotCount = 16;\n> +\n>\n\nSimilar to earlier, I would remove the \"k\" prefix.\n\nRegards,\nNaush\n\n\n> class IdGenerator\n> {\n> public:\n> @@ -127,6 +133,18 @@ private:\n> /* All frame buffers associated with this device stream. */\n> BufferMap bufferMap_;\n>\n> + /*\n> + * The number of buffers that should be allocated internally for\n> this\n> + * stream.\n> + */\n> + unsigned int internalBufferCount_;\n> +\n> + /*\n> + * The number of buffer slots that should be reserved for this\n> stream.\n> + * Only relevant for importOnly streams.\n> + */\n> + unsigned int bufferSlotCount_;\n> +\n> /*\n> * List of frame buffers that we can use if none have been\n> provided by\n> * the application for external streams. This is populated by the\n> --\n> 2.33.0\n>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6FCCEBD87C\n\tfor ;\n\tWed, 25 Aug 2021 09:26:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2BAB688A3;\n\tWed, 25 Aug 2021 11:26:58 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ED3CE60259\n\tfor ;\n\tWed, 25 Aug 2021 11:26:56 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id i9so51510961lfg.10\n\tfor ;\n\tWed, 25 Aug 2021 02:26:56 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"niaIcTfs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=HhckMq5cNY5xhZ5E511FTmsFAk4CIWF1VE3MWp1eTMI=;\n\tb=niaIcTfssqNUW5OwkLFgsYBshmlaflqJkw9BeaI9KP/jGVZrBuyXwX7S4awWVToaq/\n\tQU+Qs/s4fLJOkZNnlsQMeSIhWFsg0mnYz1AB2c4FFQNZqk9pSzKg12CPJPjrEqbzi7Qx\n\t1hbJ9+DVhD0ll8Rg36X40TUbc4X6hIBK51tlnc1tzOY1iPdizqk+2rC++kfJEQXfbfX0\n\t+ERCTc3r9f9oJTZDQgZEWhXyan3tK2MioQDNRBUqIk4vL/scqCQU5KLFJQ53ZUB78l6c\n\tDoaj1738us1t4UqEuR4KqVybIkJ9IdIwAdNMfgUG7eOOTkUhXgoLQyun1KfWqG9aIevd\n\t1Mwg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=HhckMq5cNY5xhZ5E511FTmsFAk4CIWF1VE3MWp1eTMI=;\n\tb=sKIyKnBCGGZYoO44cyohHpb6Pf1QYecjjz7vQBwOJjXXv3k9pNX5TDTyHlJ6dRh+ND\n\tdfaTqi4L6sv9cQ3KCpZVmrKPTyecmA2oTsQo+TBtVUBe9183nqc2wYRQGQf9HbWYSvOb\n\tDhnpOLEMabkA5WJzvFDBTcZaN6nJWwTPY/pcvfUGxhnXWf7m6mQvO1pc+qoj9o7VTEC6\n\tHPcOCffxwoJTbaCW9RhdaYjcyxN4o4Dz25fKhoCvaB4zKGbOsyRHbaV0hTpU6EVInbs/\n\tlhub+8rTJwVgabrz7T+YzR+ElHJDrwJJz2Vgb4DHQisXkQnhWfvLBwpkpTLe+sCTjUYu\n\t0Mlg==","X-Gm-Message-State":"AOAM532ejmVcq7pbgGKLKXuwx/PQMYBj22/4l3er7lvEve3FZMkkncsg\n\tVZLcbFvXfULUiwQLmNoexUxdwwCn22njbBPetXvuOw==","X-Google-Smtp-Source":"ABdhPJyj+2ffDL8t4Zixe5cjHpw9qCCLl9F+YViPpN+EQszIesTnEgZyUrzlbtOwQZWPWM/DI3PA+00AWYEBDktiE8Y=","X-Received":"by 2002:ac2:5597:: with SMTP id\n\tv23mr3720359lfg.617.1629883616148; \n\tWed, 25 Aug 2021 02:26:56 -0700 (PDT)","MIME-Version":"1.0","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-4-nfraprado@collabora.com>","In-Reply-To":"<20210824195636.1110845-4-nfraprado@collabora.com>","From":"Naushir Patuck ","Date":"Wed, 25 Aug 2021 10:26:39 +0100","Message-ID":"","To":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= ","Content-Type":"multipart/alternative; boundary=\"000000000000f7591c05ca5ed955\"","Subject":"Re: [libcamera-devel] [PATCH v8 03/17] libcamera: pipeline:\n\traspberrypi: Don't rely on bufferCount","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ,\n\tkernel@collabora.com, =?utf-8?q?Andr=C3=A9_Almeida?=\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25931,"web_url":"https://patchwork.libcamera.org/comment/25931/","msgid":"","date":"2022-11-30T11:19:16","subject":"Re: [libcamera-devel] [PATCH v8 02/17] libcamera:\n\tframebuffer_allocator: Make allocate() require count","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:21PM -0300, Nícolas F. R. A. Prado wrote:\n> Make FrameBufferAllocator::allocate() require a 'count' argument for the\n> number of buffers to be allocated.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - Updated application-developer and pipeline-handler guides with new allocate()\n> API and MinimumRequests property\n> - Added handling for when allocate() returns less buffers than needed in cam and\n> the capture unit test\n> - Improved FrameBufferAllocator::allocate() description\n> \n> Changes in v6:\n> - Changed static_cast to convert 'count' to unsigned int instead of\n> 'bufferCount' to int when comparing\n> \n> Changes in v5:\n> - Made sure that qcam allocates at least 2 buffers\n> \n> Documentation/guides/application-developer.rst | 9 +++++++--\n> Documentation/guides/pipeline-handler.rst | 3 +--\n> include/libcamera/camera.h | 2 +-\n> include/libcamera/framebuffer_allocator.h | 2 +-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> src/android/camera_stream.cpp | 5 ++++-\n> src/cam/camera_session.cpp | 12 ++++++++----\n> src/gstreamer/gstlibcameraallocator.cpp | 4 +++-\n> src/lc-compliance/simple_capture.cpp | 7 +++++--\n> src/libcamera/camera.cpp | 4 ++--\n> src/libcamera/framebuffer_allocator.cpp | 9 +++++++--\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 4 ++--\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 4 ++--\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 ++--\n> src/libcamera/pipeline/simple/simple.cpp | 4 ++--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 7 ++++---\n> src/libcamera/pipeline/vimc/vimc.cpp | 7 ++++---\n> src/libcamera/pipeline_handler.cpp | 1 +\n> src/qcam/main_window.cpp | 9 ++++++++-\n> src/v4l2/v4l2_camera.cpp | 2 +-\n> test/camera/camera_reconfigure.cpp | 4 +++-\n> test/camera/capture.cpp | 6 ++++--\n> test/camera/statemachine.cpp | 4 +++-\n> test/mapped-buffer.cpp | 4 +++-\n> 24 files changed, 79 insertions(+), 40 deletions(-)\n> \n> diff --git a/Documentation/guides/application-developer.rst b/Documentation/guides/application-developer.rst\n> index fb6a80a57eb7..4dc8ac402ec6 100644\n> --- a/Documentation/guides/application-developer.rst\n> +++ b/Documentation/guides/application-developer.rst\n> @@ -31,6 +31,7 @@ defined names and types without the need of prefixing them.\n> #include \n> \n> #include \n> + #include \n> \n> using namespace libcamera;\n> \n> @@ -260,7 +261,10 @@ Using the libcamera ``FrameBufferAllocator``\n> \n> Applications create a ``FrameBufferAllocator`` for a Camera and use it\n> to allocate buffers for streams of a ``CameraConfiguration`` with the\n> -``allocate()`` function.\n> +``allocate()`` function. The number of buffers to be allocated needs to be\n> +specified, and should be at least equal to the value of the ``MinimumRequests``\n> +property in order for the pipeline to have enough requests to be able to\n> +capture without frame drops.\n> \n> The list of allocated buffers can be retrieved using the ``Stream`` instance\n> as the parameter of the ``FrameBufferAllocator::buffers()`` function.\n> @@ -268,9 +272,10 @@ as the parameter of the ``FrameBufferAllocator::buffers()`` function.\n> .. code:: cpp\n> \n> FrameBufferAllocator *allocator = new FrameBufferAllocator(camera);\n> + unsigned int bufferCount = camera->properties().get(properties::MinimumRequests);\n> \n> for (StreamConfiguration &cfg : *config) {\n> - int ret = allocator->allocate(cfg.stream());\n> + int ret = allocator->allocate(cfg.stream(), bufferCount);\n> if (ret < 0) {\n> std::cerr << \"Can't allocate buffers\" << std::endl;\n> return -ENOMEM;\n> diff --git a/Documentation/guides/pipeline-handler.rst b/Documentation/guides/pipeline-handler.rst\n> index 32e3fc518d91..2a69ef7d7461 100644\n> --- a/Documentation/guides/pipeline-handler.rst\n> +++ b/Documentation/guides/pipeline-handler.rst\n> @@ -206,7 +206,7 @@ implementations for the overridden class members.\n> const StreamRoles &roles) override;\n> int configure(Camera *camera, CameraConfiguration *config) override;\n> \n> - int exportFrameBuffers(Camera *camera, Stream *stream,\n> + int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> std::vector> *buffers) override;\n> \n> int start(Camera *camera, const ControlList *controls) override;\n> @@ -1186,7 +1186,6 @@ handle this:\n> \n> .. code-block:: cpp\n> \n> - unsigned int count = stream->configuration().bufferCount;\n> VividCameraData *data = cameraData(camera);\n> \n> return data->video_->exportBuffers(count, buffers);\n> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\n> index 05cdab724b10..d52867db5137 100644\n> --- a/include/libcamera/camera.h\n> +++ b/include/libcamera/camera.h\n> @@ -116,7 +116,7 @@ private:\n> \tvoid requestComplete(Request *request);\n> \n> \tfriend class FrameBufferAllocator;\n> -\tint exportFrameBuffers(Stream *stream,\n> +\tint exportFrameBuffers(Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers);\n> };\n> \n> diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h\n> index cbc9ce101889..2d5a6e98e10c 100644\n> --- a/include/libcamera/framebuffer_allocator.h\n> +++ b/include/libcamera/framebuffer_allocator.h\n> @@ -25,7 +25,7 @@ public:\n> \tFrameBufferAllocator(std::shared_ptr camera);\n> \t~FrameBufferAllocator();\n> \n> -\tint allocate(Stream *stream);\n> +\tint allocate(Stream *stream, unsigned int count);\n> \tint free(Stream *stream);\n> \n> \tbool allocated() const { return !buffers_.empty(); }\n> diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h\n> index 41cba44d990f..c6b3af18b1d3 100644\n> --- a/include/libcamera/internal/pipeline_handler.h\n> +++ b/include/libcamera/internal/pipeline_handler.h\n> @@ -50,7 +50,7 @@ public:\n> \t\tconst StreamRoles &roles) = 0;\n> \tvirtual int configure(Camera *camera, CameraConfiguration *config) = 0;\n> \n> -\tvirtual int exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tvirtual int exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t\t std::vector> *buffers) = 0;\n> \n> \tvirtual int start(Camera *camera, const ControlList *controls) = 0;\n> diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp\n> index 61b441830e18..29be1ac5ca4f 100644\n> --- a/src/android/camera_stream.cpp\n> +++ b/src/android/camera_stream.cpp\n> @@ -15,6 +15,7 @@\n> #include \"jpeg/post_processor_jpeg.h\"\n> \n> #include \n> +#include \n> \n> using namespace libcamera;\n> \n> @@ -83,8 +84,10 @@ int CameraStream::configure()\n> \t\t\treturn ret;\n> \t}\n> \n> +\tunsigned int bufferCount = cameraDevice_->camera()->properties().get(properties::MinimumRequests);\n> +\n> \tif (allocator_) {\n> -\t\tint ret = allocator_->allocate(stream());\n> +\t\tint ret = allocator_->allocate(stream(), bufferCount);\n> \t\tif (ret < 0)\n> \t\t\treturn ret;\n> \n> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp\n> index 60d640f2b15c..d9a8500f390f 100644\n> --- a/src/cam/camera_session.cpp\n> +++ b/src/cam/camera_session.cpp\n> @@ -237,17 +237,21 @@ int CameraSession::startCapture()\n> {\n> \tint ret;\n> \n> -\t/* Identify the stream with the least number of buffers. */\n> -\tunsigned int nbuffers = UINT_MAX;\n> +\tunsigned int nbuffers = camera_->properties().get(properties::MinimumRequests);\n> +\n> \tfor (StreamConfiguration &cfg : *config_) {\n> -\t\tret = allocator_->allocate(cfg.stream());\n> +\t\tret = allocator_->allocate(cfg.stream(), nbuffers);\n> \t\tif (ret < 0) {\n> \t\t\tstd::cerr << \"Can't allocate buffers\" << std::endl;\n> \t\t\treturn -ENOMEM;\n> \t\t}\n> \n> \t\tunsigned int allocated = allocator_->buffers(cfg.stream()).size();\n> -\t\tnbuffers = std::min(nbuffers, allocated);\n> +\t\tif (allocated < nbuffers) {\n> +\t\t\tstd::cerr << \"Unable to allocate enough buffers\"\n> +\t\t\t\t << std::endl;\n> +\t\t\treturn -ENOMEM;\n> +\t\t}\n> \t}\n> \n> \t/*\n> diff --git a/src/gstreamer/gstlibcameraallocator.cpp b/src/gstreamer/gstlibcameraallocator.cpp\n> index 7bd8ba2db71d..8cc42edfaf61 100644\n> --- a/src/gstreamer/gstlibcameraallocator.cpp\n> +++ b/src/gstreamer/gstlibcameraallocator.cpp\n> @@ -10,6 +10,7 @@\n> \n> #include \n> #include \n> +#include \n> #include \n> \n> #include \"gstlibcamera-utils.h\"\n> @@ -188,13 +189,14 @@ gst_libcamera_allocator_new(std::shared_ptr camera,\n> {\n> \tauto *self = GST_LIBCAMERA_ALLOCATOR(g_object_new(GST_TYPE_LIBCAMERA_ALLOCATOR,\n> \t\t\t\t\t\t\t nullptr));\n> +\tunsigned int bufferCount = camera->properties().get(properties::MinimumRequests);\n> \n> \tself->fb_allocator = new FrameBufferAllocator(camera);\n> \tfor (StreamConfiguration &streamCfg : *config_) {\n> \t\tStream *stream = streamCfg.stream();\n> \t\tgint ret;\n> \n> -\t\tret = self->fb_allocator->allocate(stream);\n> +\t\tret = self->fb_allocator->allocate(stream, bufferCount);\n> \t\tif (ret == 0)\n> \t\t\treturn nullptr;\n> \n> diff --git a/src/lc-compliance/simple_capture.cpp b/src/lc-compliance/simple_capture.cpp\n> index 25097f28a603..d87f30cbeb1b 100644\n> --- a/src/lc-compliance/simple_capture.cpp\n> +++ b/src/lc-compliance/simple_capture.cpp\n> @@ -7,6 +7,8 @@\n> \n> #include \n> \n> +#include \n> +\n> #include \"simple_capture.h\"\n> \n> using namespace libcamera;\n> @@ -44,11 +46,12 @@ void SimpleCapture::configure(StreamRole role)\n> \n> void SimpleCapture::start()\n> {\n> +\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);\n> \tStream *stream = config_->at(0).stream();\n> -\tint count = allocator_->allocate(stream);\n> +\tint count = allocator_->allocate(stream, bufferCount);\n> \n> \tASSERT_GE(count, 0) << \"Failed to allocate buffers\";\n> -\tEXPECT_EQ(count, config_->at(0).bufferCount) << \"Allocated less buffers than expected\";\n> +\tEXPECT_EQ(count, bufferCount) << \"Allocated less buffers than expected\";\n> \n> \tcamera_->requestCompleted.connect(this, &SimpleCapture::requestComplete);\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index c20e05014fb9..bea0d04ebd4e 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -691,7 +691,7 @@ void Camera::disconnect()\n> \tdisconnected.emit(this);\n> }\n> \n> -int Camera::exportFrameBuffers(Stream *stream,\n> +int Camera::exportFrameBuffers(Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers)\n> {\n> \tPrivate *const d = _d();\n> @@ -708,7 +708,7 @@ int Camera::exportFrameBuffers(Stream *stream,\n> \n> \treturn d->pipe_->invokeMethod(&PipelineHandler::exportFrameBuffers,\n> \t\t\t\t ConnectionTypeBlocking, this, stream,\n> -\t\t\t\t buffers);\n> +\t\t\t\t count, buffers);\n> }\n> \n> /**\n> diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp\n> index 4df27cac0e94..8538240b4ae6 100644\n> --- a/src/libcamera/framebuffer_allocator.cpp\n> +++ b/src/libcamera/framebuffer_allocator.cpp\n> @@ -71,6 +71,7 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> /**\n> * \\brief Allocate buffers for a configured stream\n> * \\param[in] stream The stream to allocate buffers for\n> + * \\param[in] count The number of buffers to allocate\n> *\n> * Allocate buffers suitable for capturing frames from the \\a stream. The Camera\n> * shall have been previously configured with Camera::configure() and shall be\n> @@ -79,6 +80,10 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> * Upon successful allocation, the allocated buffers can be retrieved with the\n> * buffers() function.\n> *\n> + * This function may allocate less buffers than requested, due to memory and\n> + * other system constraints. The caller shall always check the return value to\n> + * verify if the number of allocate buffers matches its needs.\n> + *\n> * \\return The number of allocated buffers on success or a negative error code\n> * otherwise\n> * \\retval -EACCES The camera is not in a state where buffers can be allocated\n> @@ -86,14 +91,14 @@ FrameBufferAllocator::~FrameBufferAllocator()\n> * not part of the active camera configuration\n> * \\retval -EBUSY Buffers are already allocated for the \\a stream\n> */\n> -int FrameBufferAllocator::allocate(Stream *stream)\n> +int FrameBufferAllocator::allocate(Stream *stream, unsigned int count)\n> {\n> \tif (buffers_.count(stream)) {\n> \t\tLOG(Allocator, Error) << \"Buffers already allocated for stream\";\n> \t\treturn -EBUSY;\n> \t}\n> \n> -\tint ret = camera_->exportFrameBuffers(stream, &buffers_[stream]);\n> +\tint ret = camera_->exportFrameBuffers(stream, count, &buffers_[stream]);\n> \tif (ret == -EINVAL)\n> \t\tLOG(Allocator, Error)\n> \t\t\t<< \"Stream is not part of \" << camera_->id()\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 2309609093cc..eaa503a33208 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -137,7 +137,7 @@ public:\n> \t\tconst StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -670,10 +670,10 @@ int PipelineHandlerIPU3::configure(Camera *camera, CameraConfiguration *c)\n> }\n> \n> int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \n> \tif (stream == &data->outStream_)\n> \t\treturn data->imgu_->output_->exportBuffers(count, buffers);\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index d35b9714f0c3..8a1040aa46be 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -251,7 +251,7 @@ public:\n> \tCameraConfiguration *generateConfiguration(Camera *camera, const StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -795,10 +795,10 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)\n> }\n> \n> int PipelineHandlerRPi::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tRPi::Stream *s = static_cast(stream);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \tint ret = s->dev()->exportBuffers(count, buffers);\n> \n> \ts->setExportedBuffers(buffers);\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index cc663c983b3b..853a0ef42ba3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -143,7 +143,7 @@ public:\n> \t\tconst StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -674,10 +674,10 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> }\n> \n> int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tRkISP1CameraData *data = cameraData(camera);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \n> \tif (stream == &data->mainPathStream_)\n> \t\treturn mainPath_.exportBuffers(count, buffers);\n> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp\n> index 26c59e601a76..737815452bae 100644\n> --- a/src/libcamera/pipeline/simple/simple.cpp\n> +++ b/src/libcamera/pipeline/simple/simple.cpp\n> @@ -236,7 +236,7 @@ public:\n> \t\t\t\t\t\t const StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -808,10 +808,10 @@ int SimplePipelineHandler::configure(Camera *camera, CameraConfiguration *c)\n> }\n> \n> int SimplePipelineHandler::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tSimpleCameraData *data = cameraData(camera);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \n> \t/*\n> \t * Export buffers on the converter or capture video node, depending on\n> diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> index 75d57300555c..7821cacfa883 100644\n> --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp\n> @@ -70,7 +70,7 @@ public:\n> \t\tconst StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -223,11 +223,12 @@ int PipelineHandlerUVC::configure(Camera *camera, CameraConfiguration *config)\n> \treturn 0;\n> }\n> \n> -int PipelineHandlerUVC::exportFrameBuffers(Camera *camera, Stream *stream,\n> +int PipelineHandlerUVC::exportFrameBuffers(Camera *camera,\n> +\t\t\t\t\t [[maybe_unused]] Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tUVCCameraData *data = cameraData(camera);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \n> \treturn data->video_->exportBuffers(count, buffers);\n> }\n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index a0d8fd6c48c3..eebdfd1a4c01 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -88,7 +88,7 @@ public:\n> \t\tconst StreamRoles &roles) override;\n> \tint configure(Camera *camera, CameraConfiguration *config) override;\n> \n> -\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream, unsigned int count,\n> \t\t\t std::vector> *buffers) override;\n> \n> \tint start(Camera *camera, const ControlList *controls) override;\n> @@ -318,11 +318,12 @@ int PipelineHandlerVimc::configure(Camera *camera, CameraConfiguration *config)\n> \treturn 0;\n> }\n> \n> -int PipelineHandlerVimc::exportFrameBuffers(Camera *camera, Stream *stream,\n> +int PipelineHandlerVimc::exportFrameBuffers(Camera *camera,\n> +\t\t\t\t\t [[maybe_unused]] Stream *stream,\n> +\t\t\t\t\t unsigned int count,\n> \t\t\t\t\t std::vector> *buffers)\n> {\n> \tVimcCameraData *data = cameraData(camera);\n> -\tunsigned int count = stream->configuration().bufferCount;\n> \n> \treturn data->video_->exportBuffers(count, buffers);\n> }\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 597d4c6a578a..ed4d62ae5c34 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -230,6 +230,7 @@ void PipelineHandler::unlock()\n> * \\brief Allocate and export buffers for \\a stream\n> * \\param[in] camera The camera\n> * \\param[in] stream The stream to allocate buffers for\n> + * \\param[in] count The number of buffers to allocate\n> * \\param[out] buffers Array of buffers successfully allocated\n> *\n> * This function allocates buffers for the \\a stream from the devices associated\n> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp\n> index 1adaae60d83b..4089e47269a4 100644\n> --- a/src/qcam/main_window.cpp\n> +++ b/src/qcam/main_window.cpp\n> @@ -25,6 +25,7 @@\n> #include \n> \n> #include \n> +#include \n> #include \n> \n> #include \"dng_writer.h\"\n> @@ -463,7 +464,13 @@ int MainWindow::startCapture()\n> \tfor (StreamConfiguration &config : *config_) {\n> \t\tStream *stream = config.stream();\n> \n> -\t\tret = allocator_->allocate(stream);\n> +\t\t/*\n> +\t\t * We hold on to a buffer for display, so need one extra from\n> +\t\t * the minimum required for capture.\n> +\t\t */\n> +\t\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests) + 1;\n> +\n> +\t\tret = allocator_->allocate(stream, bufferCount);\n> \t\tif (ret < 0) {\n> \t\t\tqWarning() << \"Failed to allocate capture buffers\";\n> \t\t\tgoto error;\n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index 157ab94e0544..d01eacfa2b84 100644\n> --- a/src/v4l2/v4l2_camera.cpp\n> +++ b/src/v4l2/v4l2_camera.cpp\n> @@ -161,7 +161,7 @@ int V4L2Camera::allocBuffers(unsigned int count)\n> {\n> \tStream *stream = config_->at(0).stream();\n> \n> -\tint ret = bufferAllocator_->allocate(stream);\n> +\tint ret = bufferAllocator_->allocate(stream, count);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp\n> index 5adef16e1c9e..00e6b113341b 100644\n> --- a/test/camera/camera_reconfigure.cpp\n> +++ b/test/camera/camera_reconfigure.cpp\n> @@ -17,6 +17,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \"camera_test.h\"\n> #include \"test.h\"\n> @@ -76,7 +77,8 @@ private:\n> \t\t * same buffer allocation for each run.\n> \t\t */\n> \t\tif (!allocated_) {\n> -\t\t\tint ret = allocator_->allocate(stream);\n> +\t\t\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);\n> +\t\t\tint ret = allocator_->allocate(stream, bufferCount);\n> \t\t\tif (ret < 0) {\n> \t\t\t\tcerr << \"Failed to allocate buffers\" << endl;\n> \t\t\t\treturn TestFail;\n> diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp\n> index 238d98dbba16..8d921fcb38f6 100644\n> --- a/test/camera/capture.cpp\n> +++ b/test/camera/capture.cpp\n> @@ -8,6 +8,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -96,8 +97,9 @@ protected:\n> \n> \t\tStream *stream = cfg.stream();\n> \n> -\t\tint ret = allocator_->allocate(stream);\n> -\t\tif (ret < 0)\n> +\t\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);\n> +\t\tint ret = allocator_->allocate(stream, bufferCount);\n> +\t\tif (ret < static_cast(bufferCount))\n> \t\t\treturn TestFail;\n> \n> \t\tfor (const std::unique_ptr &buffer : allocator_->buffers(stream)) {\n> diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp\n> index 26fb5ca17139..70015bb56c74 100644\n> --- a/test/camera/statemachine.cpp\n> +++ b/test/camera/statemachine.cpp\n> @@ -8,6 +8,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \"camera_test.h\"\n> #include \"test.h\"\n> @@ -119,7 +120,8 @@ protected:\n> \t\t/* Use internally allocated buffers. */\n> \t\tallocator_ = new FrameBufferAllocator(camera_);\n> \t\tStream *stream = *camera_->streams().begin();\n> -\t\tif (allocator_->allocate(stream) < 0)\n> +\t\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);\n> +\t\tif (allocator_->allocate(stream, bufferCount) < 0)\n> \t\t\treturn TestFail;\n> \n> \t\tif (camera_->start())\n> diff --git a/test/mapped-buffer.cpp b/test/mapped-buffer.cpp\n> index 97571945cbca..a5c56715a169 100644\n> --- a/test/mapped-buffer.cpp\n> +++ b/test/mapped-buffer.cpp\n> @@ -8,6 +8,7 @@\n> #include \n> \n> #include \n> +#include \n> \n> #include \"libcamera/internal/mapped_framebuffer.h\"\n> \n> @@ -54,7 +55,8 @@ protected:\n> \n> \t\tstream_ = cfg.stream();\n> \n> -\t\tint ret = allocator_->allocate(stream_);\n> +\t\tunsigned int bufferCount = camera_->properties().get(properties::MinimumRequests);\n> +\t\tint ret = allocator_->allocate(stream_, bufferCount);\n> \t\tif (ret < 0)\n> \t\t\treturn TestFail;\n> \n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F0E1CBDE6B\n\tfor ;\n\tWed, 30 Nov 2022 11:19:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA36663336;\n\tWed, 30 Nov 2022 12:19:25 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C9ED661F23\n\tfor ;\n\tWed, 30 Nov 2022 12:19:23 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1360D13EA;\n\tWed, 30 Nov 2022 12:19:21 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669807165;\n\tbh=i4R1vq0MMlj4AzECUkNKWqtJTvjhbisiKuONu/lIDPI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=RRMV8B9NGIsfNN7DQscLfDraSbxfVZfD6O5mVZwdBy4FJ5kzuLsYfVgB7Kjp1Hh+Y\n\tt5ZMZBxVgW8V5Ew87wy1GfSfN/bRIIsr71Q9n4XvlBKP9KzZBvkyTaYSw2GmSESpkg\n\tqsZ6RvriFScltzqNlkVRQX1QypbsYcPtdSxcwNp0t74L4pilgADXXWQqd3hcpt9Yd+\n\tSLN59jpTdOeF1iEvQYOMqC2lFDF8dWfgNAuMdYXe5UxqaTZgwqpaodS3uICPn0ryPZ\n\tsXEr1CsK4LaIl7qb4b/sOQIVZHxJWY6pNCJFf+N7aKqJK5OPhFfokp4mzaX6I9IogV\n\tWgQL0e7eok1Iw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669807163;\n\tbh=i4R1vq0MMlj4AzECUkNKWqtJTvjhbisiKuONu/lIDPI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=nQ2h3/XVY+gSTlGFKCqO/SSluk5nW1wckOi2sPN4R//BGiXmGQAuN8oo4Xsn/edlQ\n\t6UvnGVVWuJc/k2VJKOWlYWQ+TddPlWq8zBZdSZBJ7RARQojYbzk3z4dtR780ZdLWh6\n\tn63I7vmX409dNwDtrNNwGKOg/qHokl3m4NL3UXUI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nQ2h3/XV\"; dkim-atps=neutral","Date":"Wed, 30 Nov 2022 20:19:16 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-3-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-3-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 02/17] libcamera:\n\tframebuffer_allocator: Make allocate() require count","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25933,"web_url":"https://patchwork.libcamera.org/comment/25933/","msgid":"","date":"2022-11-30T11:22:26","subject":"Re: [libcamera-devel] [PATCH v8 10/17] lc-compliance: Fix source\n\tfile ordering in meson.build","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:29PM -0300, Nícolas F. R. A. Prado wrote:\n> The capture_test.cpp file was added in the source list of meson in the\n> wrong place. Fix it so the list is alphabetically sorted.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - New\n> \n> src/lc-compliance/meson.build | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> index aa5852f6cb87..4be14b694426 100644\n> --- a/src/lc-compliance/meson.build\n> +++ b/src/lc-compliance/meson.build\n> @@ -13,10 +13,10 @@ lc_compliance_enabled = true\n> lc_compliance_sources = files([\n> '../cam/event_loop.cpp',\n> '../cam/options.cpp',\n> + 'capture_test.cpp',\n> 'environment.cpp',\n> 'main.cpp',\n> 'simple_capture.cpp',\n> - 'capture_test.cpp',\n> ])\n> \n> lc_compliance = executable('lc-compliance', lc_compliance_sources,\n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 16FC4BE08B\n\tfor ;\n\tWed, 30 Nov 2022 11:22:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7D68863336;\n\tWed, 30 Nov 2022 12:22:35 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F88861F23\n\tfor ;\n\tWed, 30 Nov 2022 12:22:34 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C4C213EA;\n\tWed, 30 Nov 2022 12:22:31 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669807355;\n\tbh=ruNH0TL9GCeR9nSHlKmsAc+fi8Coc4nVcMQKVLR4GFM=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=K+CsGPhI4KapkRgqs81FsfNIv/7AAKa0d0mTOixFqJyJDH8CD1UHxrVi53cVfxRhd\n\tznH2M/4wac3ZoEHwkN86tXMCIJTyUAwKXNa/oaS1L5tOUVuVvEutuR152L2YrULv4N\n\tSa5OaNl8c7DAodEKhLJPfC+ohLIvrTTXajTxdtvpD0dEu0SG1v/Z85xGMf9fMQOjvh\n\tNhUCZqc6SqphcacdvRbUk3/y25u1Da73OhmuC2HEBBnDRu5sMsoexvmox/rhJpy40C\n\tmdfqoh4RuOUJEV8loljA5+1oSLG+A7VkRSeP0GP/Vqg3v9sXcBnOd4DlTNeqLXSDKc\n\tMdFoDSzKYFKbA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669807353;\n\tbh=ruNH0TL9GCeR9nSHlKmsAc+fi8Coc4nVcMQKVLR4GFM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FeXgmvWJ+JiLxDFGOWtHgmnWU169tcOBYp4vTAF4ohm6jzfL9MRld55tFxofQgTA2\n\trA4QACimCxbrDlwiGMk9pfRbczMJiZicbPTa+uzSuvOSTGMP6snQ+ZNtNuGaobTbek\n\ty7XJ0UdMfsioJzXEDCnxSLq+mrUtvaAXguj+Ckpw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FeXgmvWJ\"; dkim-atps=neutral","Date":"Wed, 30 Nov 2022 20:22:26 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-11-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-11-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 10/17] lc-compliance: Fix source\n\tfile ordering in meson.build","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25958,"web_url":"https://patchwork.libcamera.org/comment/25958/","msgid":"","date":"2022-12-01T11:32:00","subject":"Re: [libcamera-devel] [PATCH v8 17/17] lc-compliance: Add test to\n\tensure MinimumRequests is valid","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:36PM -0300, Nícolas F. R. A. Prado wrote:\n> Add a test in lc-compliance to check that the MinimumRequests property\n> is set and valid, that is, greater than 0.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - Moved RequiredProperties test to property_test.cpp\n> - Moved CameraTests to new test_base.{cpp,h} files\n> \n> Changes in v7:\n> - New\n> \n> src/lc-compliance/meson.build | 1 +\n> src/lc-compliance/property_test.cpp | 24 ++++++++++++++++++++++++\n> src/lc-compliance/test_base.cpp | 10 ++++++++++\n> src/lc-compliance/test_base.h | 7 +++++++\n> 4 files changed, 42 insertions(+)\n> create mode 100644 src/lc-compliance/property_test.cpp\n> \n> diff --git a/src/lc-compliance/meson.build b/src/lc-compliance/meson.build\n> index db593a6ec201..0c7e750b8264 100644\n> --- a/src/lc-compliance/meson.build\n> +++ b/src/lc-compliance/meson.build\n> @@ -16,6 +16,7 @@ lc_compliance_sources = files([\n> 'capture_test.cpp',\n> 'environment.cpp',\n> 'main.cpp',\n> + 'property_test.cpp',\n> 'simple_capture.cpp',\n> 'test_base.cpp',\n> ])\n> diff --git a/src/lc-compliance/property_test.cpp b/src/lc-compliance/property_test.cpp\n> new file mode 100644\n> index 000000000000..6a0951b2ce53\n> --- /dev/null\n> +++ b/src/lc-compliance/property_test.cpp\n> @@ -0,0 +1,24 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2021, Collabora Ltd.\n> + *\n> + * property_test.cpp - Test camera properties\n> + */\n> +\n> +#include \n> +\n> +#include \n> +\n> +#include \"test_base.h\"\n> +\n> +using namespace libcamera;\n> +\n> +TEST_F(CameraTests, RequiredProperties)\n> +{\n> +\tconst ControlList &properties = camera_->properties();\n> +\n> +\tusing namespace properties;\n> +\n> +\tEXPECT_GT(properties.get(MinimumRequests), 0)\n> +\t\t<< \"Camera should have a positive value for MinimumRequests property\";\n> +}\n> diff --git a/src/lc-compliance/test_base.cpp b/src/lc-compliance/test_base.cpp\n> index c9957b9efd36..a5e64c8b8144 100644\n> --- a/src/lc-compliance/test_base.cpp\n> +++ b/src/lc-compliance/test_base.cpp\n> @@ -26,3 +26,13 @@ void CameraHolder::releaseCamera()\n> \tcamera_->release();\n> \tcamera_.reset();\n> }\n> +\n> +void CameraTests::SetUp()\n> +{\n> +\tacquireCamera();\n> +}\n> +\n> +void CameraTests::TearDown()\n> +{\n> +\treleaseCamera();\n> +}\n> diff --git a/src/lc-compliance/test_base.h b/src/lc-compliance/test_base.h\n> index 52347749ab10..8125e1c60af7 100644\n> --- a/src/lc-compliance/test_base.h\n> +++ b/src/lc-compliance/test_base.h\n> @@ -21,4 +21,11 @@ protected:\n> \tstd::shared_ptr camera_;\n> };\n> \n> +class CameraTests : public ::testing::Test, public CameraHolder\n> +{\n> +protected:\n> +\tvoid SetUp() override;\n> +\tvoid TearDown() override;\n> +};\n> +\n> #endif /* __LC_COMPLIANCE_TEST_BASE_H__ */\n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 16E39BE08B\n\tfor ;\n\tThu, 1 Dec 2022 11:32:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C3EB963336;\n\tThu, 1 Dec 2022 12:32:07 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B53E363335\n\tfor ;\n\tThu, 1 Dec 2022 12:32:06 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 11F5133F;\n\tThu, 1 Dec 2022 12:32:04 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669894327;\n\tbh=lirOsk8DLBbkAZQy/V1FyjQlpGvv64XSZZ1NJIKOGVc=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=H4vGyxxxkCJ9M06rzOe/kv+uwZQMRAPPIfj29q9QKyt7qBFHa0TiqTFzGNhFMc51L\n\t+uR9cS9GiN7IqqANRRWOYDGWGM9cu/919Krk4qrKI6PTqn6tMPKzMOeBeWe7F+APX4\n\t6EEdB9+BxgzoouV2aetlhrJO/W6D9j9ZvtG/F8Q8r2jK18C1QbhuC31ZZ84YAZ02ZT\n\tA+9gEEtvi4/zMayCoSf7B/5MEzHqh731i2hvtXxLcV8yrP6Szg8nLXWPNJ5i4dJ3QN\n\tLpwIeFAvZH4DKk5Hsn4EQ7Fg3nzivwK78Zq/EK3gRF5+HxjIXALcjd95+oxRSLcPqV\n\tCzrBqTexnBkAg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669894326;\n\tbh=lirOsk8DLBbkAZQy/V1FyjQlpGvv64XSZZ1NJIKOGVc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VBWPhQLnuCMcjoHE3YvvumQKY6r8WGoeQtbHTUV8qLgJGKfBqSYxa7Tq5pV9maodz\n\tDzA3n85+cnUJZr/yUO8Zd/ZQ/C86AvNF0vgLsXjK4oTxjrXTJDaSzCevZRI3ydMbaQ\n\tIbX9m4DhjdBGbiFz7NxIGSUmXDuPihiOpCg2G3A0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VBWPhQLn\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 20:32:00 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-18-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-18-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 17/17] lc-compliance: Add test to\n\tensure MinimumRequests is valid","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25960,"web_url":"https://patchwork.libcamera.org/comment/25960/","msgid":"","date":"2022-12-01T12:25:09","subject":"Re: [libcamera-devel] [PATCH v8 06/17] libcamera: pipeline: rkisp1:\n\tDon't rely on bufferCount","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:25PM -0300, Nícolas F. R. A. Prado wrote:\n> Currently the rkisp1 pipeline handler relies on bufferCount to decide on\n> the number of parameter and statistics buffers to allocate internally\n> and for the number of V4L2 buffer slots to reserve. Instead, the number\n> of internal buffers should be the minimum required by the pipeline to\n> keep the requests flowing, in order to avoid wasting memory, while the\n> number of V4L2 buffer slots should be greater than the expected number\n> of requests queued by the application, in order to avoid thrashing\n> dmabuf mappings, which would degrade performance.\n> \n> Stop relying on bufferCount for these numbers and instead set them to\n> appropriate, and independent, constants.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - New\n> \n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 19 ++++++++++++-------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 3 +--\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 2 ++\n> 3 files changed, 15 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 853a0ef42ba3..03a8d1d26e30 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -188,6 +188,16 @@ private:\n> \tstd::queue availableStatBuffers_;\n> \n> \tCamera *activeCamera_;\n> +\n> +\t/*\n> +\t * This many internal buffers (or rather parameter and statistics buffer\n> +\t * pairs) ensures that the pipeline runs smoothly, without frame drops.\n> +\t * This number considers:\n> +\t * - three buffers queued to the driver, which is also the minimum\n> +\t * required to start streaming\n> +\t * - one buffer being processed on the IPA\n> +\t */\n> +\tstatic constexpr unsigned int kRkISP1InternalBufferCount = 4;\n> };\n> \n> RkISP1Frames::RkISP1Frames(PipelineHandler *pipe)\n> @@ -693,16 +703,11 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> \tunsigned int ipaBufferId = 1;\n> \tint ret;\n> \n> -\tunsigned int maxCount = std::max({\n> -\t\tdata->mainPathStream_.configuration().bufferCount,\n> -\t\tdata->selfPathStream_.configuration().bufferCount,\n> -\t});\n> -\n> -\tret = param_->allocateBuffers(maxCount, ¶mBuffers_);\n> +\tret = param_->allocateBuffers(kRkISP1InternalBufferCount, ¶mBuffers_);\n> \tif (ret < 0)\n> \t\tgoto error;\n> \n> -\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n> +\tret = stat_->allocateBuffers(kRkISP1InternalBufferCount, &statBuffers_);\n> \tif (ret < 0)\n> \t\tgoto error;\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 25f482eb8d8e..515f4be16d7e 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -171,8 +171,7 @@ int RkISP1Path::start()\n> \tif (running_)\n> \t\treturn -EBUSY;\n> \n> -\t/* \\todo Make buffer count user configurable. */\n> -\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n> +\tret = video_->importBuffers(kRkISP1BufferSlotCount);\n> \tif (ret)\n> \t\treturn ret;\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index 91757600ccdc..267d8f988ace 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -69,6 +69,8 @@ private:\n> \tstd::unique_ptr resizer_;\n> \tstd::unique_ptr video_;\n> \tMediaLink *link_;\n> +\n> +\tstatic constexpr unsigned int kRkISP1BufferSlotCount = 16;\n> };\n> \n> class RkISP1MainPath : public RkISP1Path\n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 531A3BE08B\n\tfor ;\n\tThu, 1 Dec 2022 12:25:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2D1A63336;\n\tThu, 1 Dec 2022 13:25:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EBD1E63335\n\tfor ;\n\tThu, 1 Dec 2022 13:25:15 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1B77F33F;\n\tThu, 1 Dec 2022 13:25:13 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669897517;\n\tbh=zSw+3WiDwnax8oJWwZfJwsmSHbzu7wiN5sI/Q9JdjlI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=pFrn72Q7o5E7WObuzoDV3DNFJRcHDOXHob9Q7aI5lNIqMbijqHUfP8IIy//Xzmet3\n\tRXuNC9zZCA0YdUal/4NPQOqf/wCl418M7sRBP56l6lyu/Zh4k7ucp51N/Vm29Erhnf\n\tJLeFz5PfnDZakjTAb/sJgqpnZrkUc96tLxOJmVCDmq72zVA5jiF63EfZtZNE3T4e0C\n\tAy5b7JU5x9Ga0BbpE8Mz+qxnCnygNodAgBDpeum+gs2KUF5ye11fVbrfexVqrTtHUC\n\tHjfI8M1K34d4Sygz19aJD+G1JKPMFECFp6uHQjh3V2tV/pOOajm/SY6/9bQom1q+iB\n\tq1L702AoawxvA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669897515;\n\tbh=zSw+3WiDwnax8oJWwZfJwsmSHbzu7wiN5sI/Q9JdjlI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=fctM4wydPA7VUKMb/KBA29xbkzivnfESvwCi/hmBCdhc6VEmgb/gML2lIVmzbARQk\n\t6UztHq50Pe4ylILDsUd5Vlnt8Qvf1pdiwHMuIS87VaOOyztSrFBfZpFk1K3dKq1VhJ\n\tC1MWi3BlPz4H76+8y9Tj4i6STTraYA71kqMIAqO4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"fctM4wyd\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 21:25:09 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-7-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-7-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 06/17] libcamera: pipeline: rkisp1:\n\tDon't rely on bufferCount","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25961,"web_url":"https://patchwork.libcamera.org/comment/25961/","msgid":"","date":"2022-12-01T12:28:33","subject":"Re: [libcamera-devel] [PATCH v8 04/17] libcamera: pipeline: ipu3:\n\tDon't rely on bufferCount","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Tue, Aug 24, 2021 at 04:56:23PM -0300, Nícolas F. R. A. Prado wrote:\n> Currently the ipu3 pipeline handler relies on bufferCount to decide on\n> the number of V4L2 buffer slots to reserve as well as for the number of\n> buffers to allocate internally for the CIO2 raw buffers and\n> parameter-statistics ImgU buffer pairs. Instead, the number of internal\n> buffers should be the minimum required by the pipeline to keep the\n> requests flowing without frame drops, in order to avoid wasting memory,\n> while the number of V4L2 buffer slots should be greater than the\n> expected number of requests queued by the application, in order to avoid\n> thrashing dmabuf mappings, which would degrade performance.\n> \n> Stop relying on bufferCount for these numbers and instead set them to\n> appropriate, and independent, constants.\n> \n> Signed-off-by: Nícolas F. R. A. Prado \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> \n> Changes in v8:\n> - New\n> \n> src/libcamera/pipeline/ipu3/cio2.cpp | 6 +++---\n> src/libcamera/pipeline/ipu3/cio2.h | 18 +++++++++++++++++-\n> src/libcamera/pipeline/ipu3/imgu.cpp | 12 ++++++------\n> src/libcamera/pipeline/ipu3/imgu.h | 15 ++++++++++++++-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 20 ++++++++------------\n> 5 files changed, 48 insertions(+), 23 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp\n> index 1bcd580e251c..b940a0f6d7d6 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.cpp\n> +++ b/src/libcamera/pipeline/ipu3/cio2.cpp\n> @@ -225,13 +225,13 @@ int CIO2Device::exportBuffers(unsigned int count,\n> \treturn output_->exportBuffers(count, buffers);\n> }\n> \n> -int CIO2Device::start()\n> +int CIO2Device::start(unsigned int bufferSlotCount)\n> {\n> -\tint ret = output_->exportBuffers(CIO2_BUFFER_COUNT, &buffers_);\n> +\tint ret = output_->exportBuffers(kCIO2InternalBufferCount, &buffers_);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> -\tret = output_->importBuffers(CIO2_BUFFER_COUNT);\n> +\tret = output_->importBuffers(bufferSlotCount);\n> \tif (ret)\n> \t\tLOG(IPU3, Error) << \"Failed to import CIO2 buffers\";\n> \n> diff --git a/src/libcamera/pipeline/ipu3/cio2.h b/src/libcamera/pipeline/ipu3/cio2.h\n> index f28e9f1ddf42..ab915b6a16fa 100644\n> --- a/src/libcamera/pipeline/ipu3/cio2.h\n> +++ b/src/libcamera/pipeline/ipu3/cio2.h\n> @@ -45,7 +45,7 @@ public:\n> \tint exportBuffers(unsigned int count,\n> \t\t\t std::vector> *buffers);\n> \n> -\tint start();\n> +\tint start(unsigned int bufferSlotCount);\n> \tint stop();\n> \n> \tCameraSensor *sensor() { return sensor_.get(); }\n> @@ -69,6 +69,22 @@ private:\n> \n> \tstd::vector> buffers_;\n> \tstd::queue availableBuffers_;\n> +\n> +\t/*\n> +\t * This many internal buffers for the CIO2 ensures that the pipeline\n> +\t * runs smoothly, without frame drops. This number considers:\n> +\t * - one buffer being DMA'ed to in CIO2\n> +\t * - one buffer programmed by the CIO2 as the next buffer\n> +\t * - one buffer under processing in ImgU\n> +\t * - one extra idle buffer queued to CIO2, to account for possible\n> +\t * delays in requeuing the buffer from ImgU back to CIO2\n> +\t *\n> +\t * Transient situations can arise when one of the parts, CIO2 or ImgU,\n> +\t * finishes its processing first and experiences a lack of buffers, but\n> +\t * they will shortly after return to the state described above as the\n> +\t * other part catches up.\n> +\t */\n> +\tstatic constexpr unsigned int kCIO2InternalBufferCount = 4;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.cpp b/src/libcamera/pipeline/ipu3/imgu.cpp\n> index 317e482a1498..3484b378c189 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.cpp\n> +++ b/src/libcamera/pipeline/ipu3/imgu.cpp\n> @@ -593,22 +593,22 @@ int ImgUDevice::configureVideoDevice(V4L2VideoDevice *dev, unsigned int pad,\n> /**\n> * \\brief Allocate buffers for all the ImgU video devices\n> */\n> -int ImgUDevice::allocateBuffers(unsigned int bufferCount)\n> +int ImgUDevice::allocateBuffers(unsigned int bufferSlotCount)\n> {\n> \t/* Share buffers between CIO2 output and ImgU input. */\n> -\tint ret = input_->importBuffers(bufferCount);\n> +\tint ret = input_->importBuffers(bufferSlotCount);\n> \tif (ret) {\n> \t\tLOG(IPU3, Error) << \"Failed to import ImgU input buffers\";\n> \t\treturn ret;\n> \t}\n> \n> -\tret = param_->allocateBuffers(bufferCount, ¶mBuffers_);\n> +\tret = param_->allocateBuffers(kImgUInternalBufferCount, ¶mBuffers_);\n> \tif (ret < 0) {\n> \t\tLOG(IPU3, Error) << \"Failed to allocate ImgU param buffers\";\n> \t\tgoto error;\n> \t}\n> \n> -\tret = stat_->allocateBuffers(bufferCount, &statBuffers_);\n> +\tret = stat_->allocateBuffers(kImgUInternalBufferCount, &statBuffers_);\n> \tif (ret < 0) {\n> \t\tLOG(IPU3, Error) << \"Failed to allocate ImgU stat buffers\";\n> \t\tgoto error;\n> @@ -619,13 +619,13 @@ int ImgUDevice::allocateBuffers(unsigned int bufferCount)\n> \t * corresponding stream is active or inactive, as the driver needs\n> \t * buffers to be requested on the V4L2 devices in order to operate.\n> \t */\n> -\tret = output_->importBuffers(bufferCount);\n> +\tret = output_->importBuffers(bufferSlotCount);\n> \tif (ret < 0) {\n> \t\tLOG(IPU3, Error) << \"Failed to import ImgU output buffers\";\n> \t\tgoto error;\n> \t}\n> \n> -\tret = viewfinder_->importBuffers(bufferCount);\n> +\tret = viewfinder_->importBuffers(bufferSlotCount);\n> \tif (ret < 0) {\n> \t\tLOG(IPU3, Error) << \"Failed to import ImgU viewfinder buffers\";\n> \t\tgoto error;\n> diff --git a/src/libcamera/pipeline/ipu3/imgu.h b/src/libcamera/pipeline/ipu3/imgu.h\n> index 9d4915116087..61ccd17c2e31 100644\n> --- a/src/libcamera/pipeline/ipu3/imgu.h\n> +++ b/src/libcamera/pipeline/ipu3/imgu.h\n> @@ -61,7 +61,7 @@ public:\n> \t\t\t\t\t outputFormat);\n> \t}\n> \n> -\tint allocateBuffers(unsigned int bufferCount);\n> +\tint allocateBuffers(unsigned int bufferSlotCount);\n> \tvoid freeBuffers();\n> \n> \tint start();\n> @@ -96,6 +96,19 @@ private:\n> \n> \tstd::string name_;\n> \tMediaDevice *media_;\n> +\n> +\t/*\n> +\t * This many internal buffers (or rather parameter and statistics buffer\n> +\t * pairs) for the ImgU ensures that the pipeline runs smoothly, without\n> +\t * frame drops. This number considers:\n> +\t * - three buffers queued to the CIO2 (Since these buffers are bound to\n> +\t * CIO2 buffers before queuing to the CIO2)\n> +\t * - one buffer under processing in ImgU\n> +\t *\n> +\t * \\todo Update this number when we make these buffers only get added to\n> +\t * the FrameInfo after the raw buffers are dequeued from CIO2.\n> +\t */\n> +\tstatic constexpr unsigned int kImgUInternalBufferCount = 4;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index eaa503a33208..cc519ae6adbe 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -156,7 +156,7 @@ private:\n> \tint initControls(IPU3CameraData *data);\n> \tint registerCameras();\n> \n> -\tint allocateBuffers(Camera *camera);\n> +\tint allocateBuffers(Camera *camera, unsigned int bufferSlotCount);\n> \tint freeBuffers(Camera *camera);\n> \n> \tImgUDevice imgu0_;\n> @@ -165,6 +165,8 @@ private:\n> \tMediaDevice *imguMediaDev_;\n> \n> \tstd::vector ipaBuffers_;\n> +\n> +\tstatic constexpr unsigned int kIPU3BufferSlotCount = 16;\n> };\n> \n> IPU3CameraConfiguration::IPU3CameraConfiguration(IPU3CameraData *data)\n> @@ -693,20 +695,14 @@ int PipelineHandlerIPU3::exportFrameBuffers(Camera *camera, Stream *stream,\n> * In order to be able to start the 'viewfinder' and 'stat' nodes, we need\n> * memory to be reserved.\n> */\n> -int PipelineHandlerIPU3::allocateBuffers(Camera *camera)\n> +int PipelineHandlerIPU3::allocateBuffers(Camera *camera,\n> +\t\t\t\t\t unsigned int bufferSlotCount)\n> {\n> \tIPU3CameraData *data = cameraData(camera);\n> \tImgUDevice *imgu = data->imgu_;\n> -\tunsigned int bufferCount;\n> \tint ret;\n> \n> -\tbufferCount = std::max({\n> -\t\tdata->outStream_.configuration().bufferCount,\n> -\t\tdata->vfStream_.configuration().bufferCount,\n> -\t\tdata->rawStream_.configuration().bufferCount,\n> -\t});\n> -\n> -\tret = imgu->allocateBuffers(bufferCount);\n> +\tret = imgu->allocateBuffers(bufferSlotCount);\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> @@ -758,7 +754,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \tint ret;\n> \n> \t/* Allocate buffers for internal pipeline usage. */\n> -\tret = allocateBuffers(camera);\n> +\tret = allocateBuffers(camera, kIPU3BufferSlotCount);\n> \tif (ret)\n> \t\treturn ret;\n> \n> @@ -770,7 +766,7 @@ int PipelineHandlerIPU3::start(Camera *camera, [[maybe_unused]] const ControlLis\n> \t * Start the ImgU video devices, buffers will be queued to the\n> \t * ImgU output and viewfinder when requests will be queued.\n> \t */\n> -\tret = cio2->start();\n> +\tret = cio2->start(kIPU3BufferSlotCount);\n> \tif (ret)\n> \t\tgoto error;\n> \n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 1290BBE08B\n\tfor ;\n\tThu, 1 Dec 2022 12:28:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E4246333F;\n\tThu, 1 Dec 2022 13:28:42 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 32CF663335\n\tfor ;\n\tThu, 1 Dec 2022 13:28:41 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A7AD33F;\n\tThu, 1 Dec 2022 13:28:39 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669897722;\n\tbh=KV0SAAwmCE2NcGRqu6tYtxOyq4A5oTNOa8MrQywBjMg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=sx7tk/EkzkFYEhGBxypu0KTBa7PgDTBKR0XHnCrGQ089irha1WNpd2XVO6w4Y7t7R\n\tdalX61iP/nGzJBQd11fl5W22nxhGHBcuymcPCgmViczwTEmYxgdVbHk6UaXMN71MhR\n\t2dCHx+wSmUXE9MeAv5sz2nv9BB2SNhbnQRKFmHsSTRbuWGiTiQTtsMqSoXmq3ssLfw\n\tgJ0m4+DEbEUrdJXwic0uevHSzjBslbfjSreU4y5Qc/SuziI+5OSKdLPqyZ+GeVmsvL\n\th3FOGLTma0/N1xYULtyN9rII2gbEJS8TTBZVHTYtqP6WkppyCXAotfS2gXHbl9bKLg\n\t7UoTRuiogQigQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669897720;\n\tbh=KV0SAAwmCE2NcGRqu6tYtxOyq4A5oTNOa8MrQywBjMg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FwpHHSF48WjCuZrfwhl8Rr8/p9SZ2wnmfaE+pxEoeUIOJBknfdneLp2MzSiWwZ/7r\n\t50Fr6hOoJ/KjulxBdS1wdusN7j+e8zHFSuAzeDbsViAnJhNWEHwztHV3rlWNSwfZuS\n\tHweyhDQQpcLHjEJj+RUmLI9WtQIkeRKPjTxdpKT0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"FwpHHSF4\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 21:28:33 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t<20210824195636.1110845-5-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-5-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 04/17] libcamera: pipeline: ipu3:\n\tDon't rely on bufferCount","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25963,"web_url":"https://patchwork.libcamera.org/comment/25963/","msgid":"","date":"2022-12-01T12:32:02","subject":"Re: [libcamera-devel] [PATCH v8 00/17] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Nícolas,\n\nOn Tue, Aug 24, 2021 at 04:56:19PM -0300, Nícolas F. R. A. Prado wrote:\n> The purpose of this series is to add a new test to lc-compliance that tests\n> queuing a lot of requests at once in order to ensure that pipeline handlers are\n> able to handle more requests than they have resources for (like internal buffers\n> and V4L2 buffer slots) [1].\n\nSorry for getting to this so late. I was considering that I would rebase\nthis and send it again, since it's been so long; is that fine with you?\n\n\nThanks,\n\nPaul\n\n> \n> [1] https://bugs.libcamera.org/show_bug.cgi?id=24\n> \n> In order to achieve this, the FrameBufferAllocator had to be adapted to allow an\n> arbitrary number of buffers to be allocated. But there's also the issue of\n> reporting the minimum number of requests required by the pipeline handler, which\n> was solved by creating a new MinimumRequests property.\n> \n> Briefly, patches 1 through 9 rework the core and pipeline handlers to use\n> MinimumRequests and remove bufferCount, while patches 10 through 17 rework\n> lc-compliance to add the new test and an additional test for MinimumRequests.\n> \n> Patch 1 adds the new MinimumRequests property to report the minimum number of\n> requests needed by the pipeline handler.\n> \n> Patch 2 adds a count argument to allocate() so that the number of buffers to\n> allocate needs to be specified, as it is no longer assumed through bufferCount.\n> \n> Patches 3-7 decouple the number of internal buffers and V4L2 buffer slots from\n> bufferCount in each pipeline handler.\n> \n> Patch 8 reworks the V4L2 compatibility layer to not depend on bufferCount.\n> \n> Patch 9 removes bufferCount from the StreamConfiguration and everywhere it was\n> still used, as it is no longer needed.\n> \n> Patch 10 fixes a file ordering issue in lc-compliance's meson.build.\n> \n> Patches 11-14 does some refactoring in lc-compliance in order to reduce code\n> duplication.\n> \n> Patch 15 adds the test to lc-compliance.\n> \n> Patch 16 adds checks in lc-compliance to ensure that requests which failed to be\n> enqueued are reported as test failure.\n> \n> Patch 17 adds another, very short, test to lc-compliance to make sure that the\n> MinimumRequests property is set in the pipeline handler.\n> \n> I've run this new lc-compliance test on the raspberrypi, rkisp1, uvcvideo and\n> vimc pipelines. raspberrypi already handles it well, while the other three run\n> successfully after applying the series in [2]. The ipu3 should run fine as well\n> since the series in [2] was based on the internal queue already present there.\n> A patch for the simple pipeline is still pending.\n> \n> [2] https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022508.html\n> \n> The number of buffers to allocate in the lc-compliance test (patch 15) was\n> hardcoded to 8 since more than that would cause errors when allocating.\n> \n> v7: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022577.html\n> v6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022356.html\n> v5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-July/022098.html\n> v4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020150.html\n> v3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019613.html\n> v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019398.html\n> v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019139.html\n> \n> Changes in v8:\n> (thanks to Laurent and Kieran)\n> - Changed internal buffer count constants for pipeline handlers to better values\n> - Reordered patches to group non-lc-compliance changes together\n> - Split buffer allocation changes into separate commits for each pipeline\n> handler (patches 3-7)\n> - Changed the MinimumRequests property meaning to require that frames aren't\n> dropped\n> - Set MinimumRequests on SimplePipeline depending on device and usage of\n> converter\n> - Undid definition of default MinimumRequests on CameraSensor constructor\n> - Updated application-developer and pipeline-handler guides with new allocate()\n> API and MinimumRequests property\n> - Added handling for when allocate() returns less buffers than needed in cam and\n> the capture unit test\n> - Reworked buffer allocation handling in the raspberrypi pipeline handler\n> - Moved V4L2 compatibility layer changes to separate commit\n> - Added patch 10 to fix wrong file order in lc-compliance's meson.build\n> - Added requests_ member to SimpleCapture to hold ownership of queued\n> requests during capture\n> - Moved CameraHolder to new test_base.{cpp,h} files\n> - Fixed issue in UnbalancedStop test where requests cancelled due to stop() call\n> were failing the test\n> - Moved RequiredProperties test to property_test.cpp\n> - Moved CameraTests to new test_base.{cpp,h} files\n> \n> Changes in v7:\n> (thanks to Kieran and Jacopo)\n> - Renamed property from MinNumRequests to MinimumRequests\n> - Changed MinimumRequests property's description\n> - Added patch 11 to test if MinimumRequests is valid\n> \n> Changes in v6:\n> (thanks to Naushir)\n> - Fixed style issues\n> - Changed static_cast to unsigned int when comparing buffer count in\n> lc-compliance\n> - Added pipeline prefix to INTERNAL_BUFFER_COUNT and BUFFER_SLOT_COUNT constants\n> - Removed comment from Raspberrypi MinNumRequests setting\n> - Switched queueRequests()'s 'buffers' and 'requests' parameters order, since\n> 'requests' is an output variable\n> - Added comment to runCaptureSession()\n> \n> Changes in v5:\n> - Rebased on master (now that lc-compliance was refactored to use Googletest)\n> - Added patches 3, 5, 6 and 8\n> - Fixed qcam to use at least two buffers\n> - Made sure that qcam allocates at least 2 buffers\n> \n> Changes in v4:\n> (thanks to Laurent and Niklas)\n> - Renamed QueueDepth property to MinNumRequests and better documented it\n> - Changed patch 6 to also remove bufferCount from android\n> - Added patch 3 to factor common code in lc-compliance\n> - Added patch 5 to remove pipeline dependency on bufferCount\n> \n> Changes in v3:\n> - Added patches 1 and 4 to add the QueueDepth property and remove bufferCount\n> - Made the count argument required in patch 2\n> - Added previously missing changes to the gstreamer and V4L2 compatibility\n> layers\n> \n> Changes in v2:\n> - Renamed and reworded commits and series\n> - Dropped patches 2 and 3, which were hacks to test, and added patch 1 to add\n> count to FrameBufferAllocator\n> - Thanks to Niklas:\n> - Created new standalone test instead of looping over the other tests\n> \n> Nícolas F. R. A. Prado (17):\n> libcamera: property: Add MinimumRequests property\n> libcamera: framebuffer_allocator: Make allocate() require count\n> libcamera: pipeline: raspberrypi: Don't rely on bufferCount\n> libcamera: pipeline: ipu3: Don't rely on bufferCount\n> libcamera: pipeline: simple: Don't rely on bufferCount\n> libcamera: pipeline: rkisp1: Don't rely on bufferCount\n> libcamera: pipeline: vimc, uvcvideo: Don't rely on bufferCount\n> v4l2: Allocate buffers based on requested count and MinimumRequests\n> libcamera: stream: Remove bufferCount\n> lc-compliance: Fix source file ordering in meson.build\n> lc-compliance: Move buffer allocation to separate function\n> lc-compliance: Factor common capture code into SimpleCapture\n> lc-compliance: Move camera setup to CameraHolder class\n> lc-compliance: Move role to string conversion to its own function\n> lc-compliance: Add test to queue more requests than hardware depth\n> lc-compliance: Check that requests complete successfully\n> lc-compliance: Add test to ensure MinimumRequests is valid\n> \n> .../guides/application-developer.rst | 9 +-\n> Documentation/guides/pipeline-handler.rst | 40 +++--\n> include/libcamera/camera.h | 2 +-\n> include/libcamera/framebuffer_allocator.h | 2 +-\n> include/libcamera/internal/pipeline_handler.h | 2 +-\n> include/libcamera/stream.h | 2 -\n> src/android/camera_stream.cpp | 7 +-\n> src/cam/camera_session.cpp | 12 +-\n> src/gstreamer/gstlibcameraallocator.cpp | 4 +-\n> src/lc-compliance/capture_test.cpp | 90 ++++++++---\n> src/lc-compliance/meson.build | 4 +-\n> src/lc-compliance/property_test.cpp | 24 +++\n> src/lc-compliance/simple_capture.cpp | 147 ++++++++++++------\n> src/lc-compliance/simple_capture.h | 26 +++-\n> src/lc-compliance/test_base.cpp | 38 +++++\n> src/lc-compliance/test_base.h | 31 ++++\n> src/libcamera/camera.cpp | 4 +-\n> src/libcamera/framebuffer_allocator.cpp | 9 +-\n> src/libcamera/pipeline/ipu3/cio2.cpp | 7 +-\n> src/libcamera/pipeline/ipu3/cio2.h | 20 ++-\n> src/libcamera/pipeline/ipu3/imgu.cpp | 12 +-\n> src/libcamera/pipeline/ipu3/imgu.h | 15 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 34 ++--\n> .../pipeline/raspberrypi/raspberrypi.cpp | 57 +++----\n> .../pipeline/raspberrypi/rpi_stream.cpp | 28 +++-\n> .../pipeline/raspberrypi/rpi_stream.h | 24 ++-\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 25 +--\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 5 +-\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 4 +-\n> src/libcamera/pipeline/simple/converter.cpp | 15 +-\n> src/libcamera/pipeline/simple/converter.h | 9 +-\n> src/libcamera/pipeline/simple/simple.cpp | 70 +++++++--\n> src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 17 +-\n> src/libcamera/pipeline/vimc/vimc.cpp | 17 +-\n> src/libcamera/pipeline_handler.cpp | 1 +\n> src/libcamera/property_ids.yaml | 21 +++\n> src/libcamera/stream.cpp | 12 +-\n> src/qcam/main_window.cpp | 9 +-\n> src/v4l2/v4l2_camera.cpp | 22 ++-\n> src/v4l2/v4l2_camera.h | 5 +-\n> src/v4l2/v4l2_camera_proxy.cpp | 11 +-\n> test/camera/buffer_import.cpp | 10 +-\n> test/camera/camera_reconfigure.cpp | 4 +-\n> test/camera/capture.cpp | 6 +-\n> test/camera/statemachine.cpp | 4 +-\n> test/libtest/buffer_source.cpp | 4 +-\n> test/libtest/buffer_source.h | 2 +-\n> test/mapped-buffer.cpp | 4 +-\n> test/v4l2_videodevice/buffer_cache.cpp | 3 +-\n> 49 files changed, 655 insertions(+), 275 deletions(-)\n> create mode 100644 src/lc-compliance/property_test.cpp\n> create mode 100644 src/lc-compliance/test_base.cpp\n> create mode 100644 src/lc-compliance/test_base.h\n> \n> -- \n> 2.33.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 22FE8BDE6B\n\tfor ;\n\tThu, 1 Dec 2022 12:32:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 97FE363336;\n\tThu, 1 Dec 2022 13:32:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11E1D63335\n\tfor ;\n\tThu, 1 Dec 2022 13:32:10 +0100 (CET)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C1F633F;\n\tThu, 1 Dec 2022 13:32:07 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669897931;\n\tbh=wmlwuq0p8OD4UnOpNe6N7+ED0qypQRpNMBWqGk4jBr0=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=u7vurVqaVqNF8OZuveYefBheh7mB/DBPJ2UwKmmRrBMFc1yYlWynCk1RokLClc+yp\n\tZw49iV0ariEM+V1RI9mG8Qg7+bxvFxn+SKmRw5kTz26YTiva9vB0dbpr/pmOCesfP0\n\tPPN6X27zqW/EO7oHxD9cScyKwBdJernsktu4lEoFBI14Agb4hq3fdfTpbf6M/3db1d\n\tZE/SFeDJBwFGdqi8QfQwvV8/jGEaf3Igy0f5oj2pb55gDu4+FvjT1VadwinM/STUo2\n\t0LO28iqCzCy2O71SglzcoQ7xqqx5e5z54U8gn8b7vJ/RlDEWRfnDbS34ZhITJrrdSH\n\ty92q9HL3ahSxA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669897929;\n\tbh=wmlwuq0p8OD4UnOpNe6N7+ED0qypQRpNMBWqGk4jBr0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BuiT0jkY2ofsXL93WYPt69Y3qYJyWxqp4swFuW+yVO0i2JVxZOSutvJWdC5LfC/Kj\n\ttIjRTcwUOpeephh0rUyGRyPB2hXPbVp7dW5EZ4Uhzgn7YoubuKJ0KroN/OJhrjOVIp\n\thBFhSrZdVGSkR2uKjzhNBDuuwOeN1cxn4ip639lY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BuiT0jkY\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 21:32:02 +0900","To":"=?iso-8859-1?q?N=EDcolas_F=2E_R=2E_A=2E?= Prado ","Message-ID":"","References":"<20210824195636.1110845-1-nfraprado@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20210824195636.1110845-1-nfraprado@collabora.com>","Subject":"Re: [libcamera-devel] [PATCH v8 00/17] lc-compliance: Add test to\n\tqueue more requests than hardware depth","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Paul Elder via libcamera-devel ","Reply-To":"Paul Elder ","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com,\n\t=?iso-8859-1?q?Andr=E9?= Almeida ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25966,"web_url":"https://patchwork.libcamera.org/comment/25966/","msgid":"<20221201150042.lioi2zopptfaae36@notapiano>","date":"2022-12-01T15:02:46","subject":"Re: [libcamera-devel] [PATCH v8 00/17] lc-compliance: Add test to\n\tqueue more requests than hardware depth","submitter":{"id":84,"url":"https://patchwork.libcamera.org/api/people/84/","name":"Nícolas F. R. A. Prado","email":"nfraprado@collabora.com"},"content":"On Thu, Dec 01, 2022 at 09:32:02PM +0900, Paul Elder wrote:\n> Hi Nícolas,\n> \n> On Tue, Aug 24, 2021 at 04:56:19PM -0300, Nícolas F. R. A. Prado wrote:\n> > The purpose of this series is to add a new test to lc-compliance that tests\n> > queuing a lot of requests at once in order to ensure that pipeline handlers are\n> > able to handle more requests than they have resources for (like internal buffers\n> > and V4L2 buffer slots) [1].\n> \n> Sorry for getting to this so late. I was considering that I would rebase\n> this and send it again, since it's been so long; is that fine with you?\n\nHi,\n\nyes that would be fine by me, although I don't think I would have the time to\nwork on this series further myself (and my brain has since let go of a lot of\nthe concepts needed here)...\n\nThanks,\nNícolas","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5EDAABDE6B\n\tfor ;\n\tThu, 1 Dec 2022 15:02:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BC1FA63336;\n\tThu, 1 Dec 2022 16:02:52 +0100 (CET)","from madras.collabora.co.uk (madras.collabora.co.uk\n\t[46.235.227.172])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F030463335\n\tfor ;\n\tThu, 1 Dec 2022 16:02:50 +0100 (CET)","from notapiano (unknown\n\t[IPv6:2600:4041:5b1a:cd00:524d:e95d:1a9c:492a])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange ECDHE (P-256) server-signature RSA-PSS (4096 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\t(Authenticated sender: nfraprado)\n\tby madras.collabora.co.uk (Postfix) with ESMTPSA id 2C3FF66025DF;\n\tThu, 1 Dec 2022 15:02:50 +0000 (GMT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669906972;\n\tbh=OOvb1M8ksYgz8Ts+NPO5XWXbfycIcdwxDRYpn3qs5uI=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=rbKzkdTpU+BnKReXgqG4BnulKJ24SZ/+0Gq3kv6M8UIXLm//jE2jxICwxnIUM8/4W\n\t+DNLQI6UVN1fgN6OfDF8YYzq40t3RARNptzdN4FFfiWyOBtw4D8akGavhTEQkQDDEp\n\tgEHVmOfdCZELkqKAnaMpddf2pXhp07I02Ejy340UWs7vU+8igtIDssJ6dCM6Kwm4ET\n\tJmHB39VrCECo4ctjADfarOMU4l3Zn4duPxB5Uo+drlrvJ0ul7iN4TuJAS97KbXK4CX\n\t0/fA2LxSY7cVtQUrO+W/omA+pyQahoA7OwSoo2h8wOdnbRLZdW89Rbp6eoXHNVrMXy\n\tP2x41IVQLeNaA==","v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com;\n\ts=mail; t=1669906970;\n\tbh=OOvb1M8ksYgz8Ts+NPO5XWXbfycIcdwxDRYpn3qs5uI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=IMPS7krYmsbTIzLY0BFP27AHdPCJOJWwU3Xk75hRmnsq7dqap2dlaqp0TsJPsKlUA\n\tD7qRI2xwP1Vuhf8SMmHfyR9x/ShR6/mW97zivH2uVH48t+KZsg+Q0D0l/ZWj982kaK\n\tGll52RckVhCS/5Z0T7J5dQgVxOoyisMuCVsaIx2fw/ewHGiDs6RQMcGzKa7mKddnxf\n\txszHyI6ZN4sKlt1Bc7MhHiZL8bv3qzv/FuAiXeMb0BZjQbxRhu6g1VLHpgKn1ULah2\n\tsyjIPze6McVLIKTDcoJilmXTalhF67X+0Gg3IqwM36tV2UDbwi+2R/SqqkiclxY0SO\n\tagrUiiCC+Vjdw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=collabora.com\n\theader.i=@collabora.com\n\theader.b=\"IMPS7krY\"; dkim-atps=neutral","Date":"Thu, 1 Dec 2022 10:02:46 -0500","To":"Paul Elder ","Message-ID":"<20221201150042.lioi2zopptfaae36@notapiano>","References":"<20210824195636.1110845-1-nfraprado@collabora.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v8 00/17] lc-compliance: Add test to\n\tqueue more requests than hardware depth","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"=?utf-8?q?N=C3=ADcolas_F=2E_R=2E_A=2E_Prado_via_libcamera-devel?=\n\t","Reply-To":"=?utf-8?b?TsOtY29sYXMgRi4gUi4gQS4=?= Prado\n\t","Cc":"libcamera-devel@lists.libcamera.org, kernel@collabora.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]