{"id":14588,"url":"https://patchwork.libcamera.org/api/1.1/patches/14588/?format=json","web_url":"https://patchwork.libcamera.org/patch/14588/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20211112100305.2217099-3-naush@raspberrypi.com>","date":"2021-11-12T10:03:04","name":"[libcamera-devel,v2,2/3] pipeline: raspberrypi: Rework the internal buffer allocation scheme","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"a533d56a0112497ea48434ed0ed028308821abec","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/1.1/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/14588/mbox/","series":[{"id":2716,"url":"https://patchwork.libcamera.org/api/1.1/series/2716/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2716","date":"2021-11-12T10:03:02","name":"raspberrypi: Rework the internal buffer allocation scheme","version":2,"mbox":"https://patchwork.libcamera.org/series/2716/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/14588/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/14588/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 00F40BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 12 Nov 2021 10:03:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0B4B60362;\n\tFri, 12 Nov 2021 11:03:52 +0100 (CET)","from mail-wr1-x429.google.com (mail-wr1-x429.google.com\n\t[IPv6:2a00:1450:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7AA6060234\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Nov 2021 11:03:51 +0100 (CET)","by mail-wr1-x429.google.com with SMTP id n29so14477840wra.11\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 12 Nov 2021 02:03:51 -0800 (PST)","from naush-laptop.pitowers.org ([93.93.133.154])\n\tby smtp.gmail.com with ESMTPSA id\n\to9sm5283155wrs.4.2021.11.12.02.03.47\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 12 Nov 2021 02:03:50 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"lkk8ZtOs\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=P0NCeHwS7qg46ZoOFp+cM5MU1LaBQjc7ALSHiOuZ6cs=;\n\tb=lkk8ZtOsWFlyed0y5zMEU4v43p4KPuZAX+ovlMcnQM0yqsF7HKsYuHgtU1ZHDfwtLM\n\typxUyAbcnvnxGq31kPh70NGkY08gorEm2qHtFurDl3oBqdKAevvH/Dac/DDfVOL7kwIG\n\tVDeoOU58LOiC0G1KuVHocVGsw2JJDC574/UYPIOuDQDXlXhr6jTZqsezUydwOtFL1L/C\n\tuIwiPPPh862fkRYQspJhPpVmqtqtejUyEO927IAgn4ldPTcuwbDzzIWIJOKxOzfJDIZR\n\tege5h5D8siA5jyZvzcFgUfaRPCbe+8G49PiNdieNcKS+vI3h/Ycj8Q4kdbQjwcTSmjEN\n\tup5g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=P0NCeHwS7qg46ZoOFp+cM5MU1LaBQjc7ALSHiOuZ6cs=;\n\tb=v5oVqL1i0aC5Ikkz0lbh+nO+TgT57hPXd/q2HudwPWr9kcI8M8XopsY+M/m16cWlSP\n\tuqq4pqpimahR/qdIXzmyydptAwsX714sgxY8hm7XuRt0ppuBIeJYDK230yxeBAtJU853\n\tEcV74Eo8m138b+KFYGTdiE+ssFsQZTHBY+jcynvlZrAOjfK9MQHBlokoC3eaAyvriOxb\n\tohFgL7lGp2G0mbecuXPrRAOCwoUjT37nuQSHy6C1v0BbdSPFOiDTC/fIbQVucNAt1Rf3\n\tEX+AvT/C/N/e2QWI4UPkOvNsZb5Va0KZw85xabjqP+lPh8XsUusVBgWedkfmQE2B/7GZ\n\tVz8g==","X-Gm-Message-State":"AOAM532VreweANCI1AQpChmSON6JMG/akzeb+18Xl0byP3uEyctOzV5k\n\tlnAVQTovEVdzJ32WAW3J+LnUlCiTEmZrv4Fo","X-Google-Smtp-Source":"ABdhPJzDRXbhTyBpQFbfuTyqkWwC0nnDPr/sTstNgBk3/2lj/s7MOa66nDzr1sUqVqRaakNZuAwMrg==","X-Received":"by 2002:a5d:6c6b:: with SMTP id\n\tr11mr17054706wrz.231.1636711430994; \n\tFri, 12 Nov 2021 02:03:50 -0800 (PST)","From":"Naushir Patuck <naush@raspberrypi.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Fri, 12 Nov 2021 10:03:04 +0000","Message-Id":"<20211112100305.2217099-3-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20211112100305.2217099-1-naush@raspberrypi.com>","References":"<20211112100305.2217099-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 2/3] pipeline: raspberrypi: Rework the\n\tinternal buffer allocation scheme","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"Roman Stratiienko <r.stratiienko@gmail.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"For simplicity, the pipeline handler currently look at the maximum number of\nbuffers set in the StreamConfiguration by the user and allocate the same number\nof internal buffers for all device nodes. This would likely overallocate buffers\nfor some nodes. Rework this logic to try and minimise overallcations without\ncompromising performance.\n\nThe key change is to mostly decouple the number of internal buffers allocated\nfrom number of buffers requested by the user through the StreamConfiguration.\n\nFor ISP nodes, we only ever need 1 set of internal buffers, as the hardware runs\nsynchronous with the requests and IPA.\n\nFor Unicam nodes, allocate a minimum for 4 buffers (exported + internal), but\nalso require at least 2 internal buffers to minimise frame drops.\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\nReviewed-by: Umang Jain <umang.jain@ideasonboard.com>\n---\n .../pipeline/raspberrypi/raspberrypi.cpp      | 43 ++++++++++++++-----\n 1 file changed, 32 insertions(+), 11 deletions(-)","diff":"diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 11d3c2b120dd..4f6c699a4379 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -1211,21 +1211,42 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n {\n \tRPiCameraData *data = cameraData(camera);\n+\tunsigned int numRawBuffers = 0;\n \tint ret;\n \n-\t/*\n-\t * Decide how many internal buffers to allocate. For now, simply look\n-\t * at how many external buffers will be provided. We'll need to improve\n-\t * this logic. However, we really must have all streams allocate the same\n-\t * number of buffers to simplify error handling in queueRequestDevice().\n-\t */\n-\tunsigned int maxBuffers = 0;\n-\tfor (const Stream *s : camera->streams())\n-\t\tif (static_cast<const RPi::Stream *>(s)->isExternal())\n-\t\t\tmaxBuffers = std::max(maxBuffers, s->configuration().bufferCount);\n+\tfor (Stream *s : camera->streams()) {\n+\t\tif (isRaw(s->configuration().pixelFormat)) {\n+\t\t\tnumRawBuffers = s->configuration().bufferCount;\n+\t\t\tbreak;\n+\t\t}\n+\t}\n \n+\t/* Decide how many internal buffers to allocate. */\n \tfor (auto const stream : data->streams_) {\n-\t\tret = stream->prepareBuffers(maxBuffers);\n+\t\tunsigned int numBuffers;\n+\n+\t\tif (stream == &data->unicam_[Unicam::Image] ||\n+\t\t    stream == &data->unicam_[Unicam::Embedded]) {\n+\t\t\t/*\n+\t\t\t * For Unicam, allocate a minimum of 4 buffers as we want\n+\t\t\t * to avoid any frame drops. If an application has configured\n+\t\t\t * a RAW stream, allocate additional buffers to make up the\n+\t\t\t * minimum, but ensure we have at least 2 sets of internal\n+\t\t\t * buffers to use to minimise frame drops.\n+\t\t\t */\n+\t\t\tconstexpr unsigned int minBuffers = 4;\n+\t\t\tnumBuffers = std::max<int>(2, minBuffers - numRawBuffers);\n+\t\t} else {\n+\t\t\t/*\n+\t\t\t * Since the ISP runs synchronous with the IPA and requests,\n+\t\t\t * we only ever need one set of internal buffers. Any buffers\n+\t\t\t * the application wants to hold onto will already be exported\n+\t\t\t * through PipelineHandlerRPi::exportFrameBuffers().\n+\t\t\t */\n+\t\t\tnumBuffers = 1;\n+\t\t}\n+\n+\t\tret = stream->prepareBuffers(numBuffers);\n \t\tif (ret < 0)\n \t\t\treturn ret;\n \t}\n","prefixes":["libcamera-devel","v2","2/3"]}