[{"id":26525,"web_url":"https://patchwork.libcamera.org/comment/26525/","msgid":"<20230302095403.bpeud5frzeqrorav@uno.localdomain>","date":"2023-03-02T09:54:03","subject":"Re: [libcamera-devel] [PATCH v1 2/8] pipeline: raspberrypi: Handle\n\tMandatoryStream hints for Unicam Image","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Fri, Feb 03, 2023 at 09:44:18AM +0000, Naushir Patuck via libcamera-devel wrote:\n> Look for MandatoryStream flag in the hints field of the Unicam Image\n> StreamConfiguration structure. If this flag is set, it guarantees that the\n> application will provide buffers for Unicam Image, so override the\n> minUnicamBuffers and minTotalUnicamBuffers config parameters in the following\n> way:\n>\n> - If startup drop frames are required, allocate at least 1 internal buffer.\n> - If no startup drop frames are required, do not allocate any internal buffers.\n>\n> All other buffer allocations remain unchanged.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 34 +++++++++++++++----\n> 1 file changed, 27 insertions(+), 7 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index d752911ddfff..7d786fe839f7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1520,12 +1520,33 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> {\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> \tint ret;\n>\n> \tfor (Stream *s : camera->streams()) {\n> -\t\tif (isRaw(s->configuration().pixelFormat)) {\n> +\t\tif (s == &data->unicam_[Unicam::Image]) {\n\nWhat if the sensor produces YUV ? I guess it doesn't really make a\ndifference, we'll have to drop frames or not regardless of the image\nformat ?\n\n> \t\t\tnumRawBuffers = s->configuration().bufferCount;\n> +\t\t\t/*\n> +\t\t\t * If the application provides a guarantees that Unicam\n> +\t\t\t * image buffers will always be provided for the RAW stream\n> +\t\t\t * in a Request, we need:\n> +\t\t\t * - at least 1 internal Unicam buffer to handle startup frame drops,\n> +\t\t\t * - no internal Unicam buffers if there are no startup frame drops.\n\n\n> +\t\t\t */\n> +\t\t\tif (s->configuration().hints &\n> +\t\t\t StreamConfiguration::Hint::MandatoryStream) {\n> +\t\t\t\tif (data->dropFrameCount_) {\n> +\t\t\t\t\tminUnicamBuffers = std::max(1u, minUnicamBuffers);\n> +\t\t\t\t\tminTotalUnicamBuffers =\n> +\t\t\t\t\t\tstd::max(minUnicamBuffers, minTotalUnicamBuffers);\n> +\t\t\t\t} else {\n> +\t\t\t\t\tminUnicamBuffers = 0;\n> +\t\t\t\t\tminTotalUnicamBuffers = 0;\n> +\t\t\t\t}\n> +\t\t\t}\n> +\n> \t\t\tbreak;\n> \t\t}\n> \t}\n> @@ -1537,7 +1558,6 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t * For Unicam, allocate a minimum number of buffers for internal\n> \t\t * use as we want to avoid any frame drops.\n> \t\t */\n> -\t\tconst unsigned int minBuffers = data->config_.minTotalUnicamBuffers;\n> \t\tif (stream == &data->unicam_[Unicam::Image]) {\n> \t\t\t/*\n> \t\t\t * If an application has configured a RAW stream, allocate\n> @@ -1545,8 +1565,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t\t * we have at least minUnicamBuffers of internal buffers\n> \t\t\t * to use to minimise frame drops.\n> \t\t\t */\n> -\t\t\tnumBuffers = std::max(data->config_.minUnicamBuffers,\n> -\t\t\t\t\t\t minBuffers - numRawBuffers);\n> +\t\t\tnumBuffers = std::max(minUnicamBuffers,\n> +\t\t\t\t\t\t minTotalUnicamBuffers - numRawBuffers);\n\nif minTotalUnicamBuffers == 0, is there a risk that\n(minTotalUnicamBuffers - numRawBuffers) becomes negative ? as\nstd::max do we risk any wrong type conversion ?\n\n> \t\t} else if (stream == &data->isp_[Isp::Input]) {\n> \t\t\t/*\n> \t\t\t * ISP input buffers are imported from Unicam, so follow\n> @@ -1554,15 +1574,15 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> \t\t\t * available.\n> \t\t\t */\n> \t\t\tnumBuffers = numRawBuffers +\n> -\t\t\t\t std::max(data->config_.minUnicamBuffers,\n> -\t\t\t\t\t\t minBuffers - numRawBuffers);\n> +\t\t\t\t std::max(minUnicamBuffers,\n> +\t\t\t\t\t\t minTotalUnicamBuffers - numRawBuffers);\n\nditto\n\n>\n> \t\t} else if (stream == &data->unicam_[Unicam::Embedded]) {\n> \t\t\t/*\n> \t\t\t * Embedded data buffers are (currently) for internal use,\n> \t\t\t * so allocate the minimum required to avoid frame drops.\n> \t\t\t */\n> -\t\t\tnumBuffers = minBuffers;\n> +\t\t\tnumBuffers = data->config_.minTotalUnicamBuffers;\n> \t\t} else {\n> \t\t\t/*\n> \t\t\t * Since the ISP runs synchronous with the IPA and requests,\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 CED0BBF415\n\tfor ;\n\tThu, 2 Mar 2023 09:54:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4665B626C6;\n\tThu, 2 Mar 2023 10:54:08 +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 5A26962668\n\tfor ;\n\tThu, 2 Mar 2023 10:54:07 +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 B416256A;\n\tThu, 2 Mar 2023 10:54:06 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677750848;\n\tbh=nLSXQVYndDZ3f7TatbAOwUWokANrikslMxoLpjZje6A=;\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=BX0aorPNXq0/fYbJ6d99RDuCzjJmL6nkN1TGHAoBOxfmG300tiBeS0VoQAD1vv984\n\tfiCz6VNg9+meg2UJzTOreBu2WcowHXNZ2uTSGryIus4JMmWzvZodHyS0y3a9Ny/rcd\n\ttOdtENn7RoLbJD/4VJLNZw5/uj3b9HmuhpW26E+PjRf4/z9OUP0sHFwwrYSjE/eVzC\n\tA3UDmj1Nl03r2RAYfYBZPLVG9L2mMS+Fj7757KMsxRcu6WnJd8mEJFvzfiP/F8bNNj\n\t/SzCVgwhEB4ZPFs+p+lJZHeqYZ9fwfJk07yZv0VZ/zJBEhDhL/xGTbv8IDWJEUtWNS\n\tfp1u7y4uRm7eA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677750847;\n\tbh=nLSXQVYndDZ3f7TatbAOwUWokANrikslMxoLpjZje6A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v9wjkQG6lNJ8ND1z1pmwUwRA76nrA3WBzNGo4nOWwHi5mYRGfFMLmH5VvYHlTvQOM\n\t1NzzgqWyuXHiljuDrfin6mJ4PJwQKWwW9syIsiCSwN71SeYGkt5A6ewULeKQRxXv9s\n\tuUN8icfBUS60k976qtKCtbzxTJH+DIJ/seHy3lNU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"v9wjkQG6\"; dkim-atps=neutral","Date":"Thu, 2 Mar 2023 10:54:03 +0100","To":"Naushir Patuck ","Message-ID":"<20230302095403.bpeud5frzeqrorav@uno.localdomain>","References":"<20230203094424.25243-1-naush@raspberrypi.com>\n\t<20230203094424.25243-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230203094424.25243-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v1 2/8] pipeline: raspberrypi: Handle\n\tMandatoryStream hints for Unicam Image","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\" "}}]