[{"id":26526,"web_url":"https://patchwork.libcamera.org/comment/26526/","msgid":"<20230302095856.25grfyid4ijscwst@uno.localdomain>","date":"2023-03-02T09:58:56","subject":"Re: [libcamera-devel] [PATCH v1 3/8] pipeline: raspberrypi: Handle\n\tMandatoryStream hints for ISP Output0","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"On Fri, Feb 03, 2023 at 09:44:19AM +0000, Naushir Patuck via libcamera-devel wrote:\n> Look for MandatoryStream flag in the hints field of the ISP Output0\n> StreamConfiguration structure. If this flag is set, it guarantees that\n> the application will provide buffers for the ISP, do not allocate any\n> internal buffers for the device.\n>\n> Signed-off-by: Naushir Patuck \n> Reviewed-by: Kieran Bingham \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 29 ++++++++++++++-----\n> 1 file changed, 22 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 7d786fe839f7..b6ed66a1cf46 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1522,7 +1522,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \tRPiCameraData *data = cameraData(camera);\n> \tunsigned int minUnicamBuffers = data->config_.minUnicamBuffers;\n> \tunsigned int minTotalUnicamBuffers = data->config_.minTotalUnicamBuffers;\n> -\tunsigned int numRawBuffers = 0;\n> +\tunsigned int numRawBuffers = 0, minIspBuffers = 1;\n\nnit: I guess we prefer one variable per line (not sure how much we can\nenforce it though)\n\n> \tint ret;\n>\n> \tfor (Stream *s : camera->streams()) {\n> @@ -1546,8 +1546,21 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t\t\t\tminTotalUnicamBuffers = 0;\n> \t\t\t\t}\n> \t\t\t}\n> -\n> -\t\t\tbreak;\n> +\t\t} else if (s == &data->isp_[Isp::Output0]) {\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 a maximum of one internal buffer. Any\n> +\t\t\t * buffers the application wants to hold onto will already\n> +\t\t\t * be exported through PipelineHandlerRPi::exportFrameBuffers().\n> +\t\t\t *\n> +\t\t\t * However, as above, if the application provides a guarantee\n> +\t\t\t * that the buffer will always be provided for the ISP Output0\n> +\t\t\t * stream in a Request, we don't need any internal buffers\n> +\t\t\t * allocated.\n> +\t\t\t */\n> +\t\t\tif (!data->dropFrameCount_ &&\n> +\t\t\t s->configuration().hints & StreamConfiguration::Hint::MandatoryStream)\n> +\t\t\t\tminIspBuffers = 0;\n> \t\t}\n> \t}\n>\n> @@ -1583,12 +1596,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t\t * so allocate the minimum required to avoid frame drops.\n> \t\t\t */\n> \t\t\tnumBuffers = data->config_.minTotalUnicamBuffers;\n> +\t\t} else if (stream == &data->isp_[Isp::Output0]) {\n> +\t\t\t/* Buffer count for this is handled in the earlier loop above. */\n> +\t\t\tnumBuffers = minIspBuffers;\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 * Same reasoning as for ISP Output 0, we only ever need\n\nIsn't it different ? I got that Ouput1 is not a concern and we can\nunconditionally allocate a buffer due to its smaller resolution ?\n\n> +\t\t\t * a maximum of one internal buffer for Output1 (required\n> +\t\t\t * for colour denoise) and ISP statistics.\n> \t\t\t */\n> \t\t\tnumBuffers = 1;\n> \t\t}\n> --\n> 2.25.1\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 C5934BE080\n\tfor ;\n\tThu, 2 Mar 2023 09:59:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 14DB2626C1;\n\tThu, 2 Mar 2023 10:59:01 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B3B0D626C0\n\tfor ;\n\tThu, 2 Mar 2023 10:58:59 +0100 (CET)","from ideasonboard.com (host-87-18-61-24.retail.telecomitalia.it\n\t[87.18.61.24])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2BF2456A;\n\tThu, 2 Mar 2023 10:58:59 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677751141;\n\tbh=2tSu1Riykk63ya0sIW7iZVpx1rqRIlTARzR3CiHcS+M=;\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=eTut17jHJePtahCaosbQf26eNT+1TzlS47xRmF7cp0V7Bp4fwhuhtSFLHn1KAAuqk\n\tpjLt9aC+r070PFDSN2ScIv4qKfqNCnOfikVGiZBYo1l08hAgZe145iQz17Oz8Su2De\n\tEc/HAEaTSplM9G79nZqz3LDpvidm0vCtdOTpEEZbtD9SCVeBImmFqEABSmuMFnN0yJ\n\tzwnPZza0UZGAJCPRGmSCeMTq4HwL/FzyrvirsIFj4hsomij28mnGXCSPS3qekUKHGe\n\t87u2RZAPYYKzD1YOzY5KjDP7u8vTCax+sZPBsxFZtP6g2kuWV0aLA157QKa9QQC4Dz\n\tPAKv0HtQ3gyeQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677751139;\n\tbh=2tSu1Riykk63ya0sIW7iZVpx1rqRIlTARzR3CiHcS+M=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jEAgRWc5AFvgxkJ3cxVZZ0kqLOtZrueOVGgg6hTecTDvRm8QH4uZiUYjcfAeXRBMf\n\tvv9yaeZx189OB9F5EE9+1cIduW8vA2PajhJQRcD+ciZOkSVrXQ5Vq3Dr0NGO64JlKi\n\thpxvgW14fZwdqRKTAgntuxPnxi+WxHr30uOVbSFw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jEAgRWc5\"; dkim-atps=neutral","Date":"Thu, 2 Mar 2023 10:58:56 +0100","To":"Naushir Patuck ","Message-ID":"<20230302095856.25grfyid4ijscwst@uno.localdomain>","References":"<20230203094424.25243-1-naush@raspberrypi.com>\n\t<20230203094424.25243-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230203094424.25243-4-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 3/8] pipeline: raspberrypi: Handle\n\tMandatoryStream hints for ISP Output0","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]