[{"id":26284,"web_url":"https://patchwork.libcamera.org/comment/26284/","msgid":"<167421343001.42371.15378884961239215955@Monstersaurus>","date":"2023-01-20T11:17:10","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> Add a new pipeline config parameter \"disable_startup_frame_drops\" to disable\n> any startup drop frames, overriding the IPA request.\n> \n> When this parameter is set, it allows the pipeline handler to run with no\n> internally allocated Unicam buffers (\"min_unicam_buffers\").\n> \n> Signed-off-by: Naushir Patuck \n> Reviewed-by: David Plowman \n> ---\n> src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> 2 files changed, 14 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> index 001a906af528..421f30e62aa3 100644\n> --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> @@ -15,6 +15,10 @@\n> #\n> # internal buffer count = max(min_unicam_buffers,\n> # min_total_unicam_buffers - external buffer count)\n> - \"min_total_unicam_buffers\": 4\n> + \"min_total_unicam_buffers\": 4,\n> +\n> + # Override any request from the IPA to drop a number of startup\n> + # frames.\n> + \"disable_startup_frame_drops\": false\n\nMany configuration files I've seen and used specify options as ...\noptional. While the documentation describes the default value...\n\n> }\n> }\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 39f48e0a57fb..3529d331deb6 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -309,6 +309,11 @@ public:\n> * the Unicam Image stream.\n> */\n> unsigned int minTotalUnicamBuffers;\n> + /*\n> + * Override any request from the IPA to drop a number of startup\n> + * frames.\n> + */\n> + bool disableStartupFrameDrops;\n> };\n> \n> Config config_;\n> @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> data->setSensorControls(startConfig.controls);\n> \n> /* Configure the number of dropped frames required on startup. */\n> - data->dropFrameCount_ = startConfig.dropFrameCount;\n> + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount;\n> \n> for (auto const stream : data->streams_)\n> stream->resetBuffers();\n> @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> config_ = {\n> .minUnicamBuffers = 2,\n> .minTotalUnicamBuffers = 4,\n> + .disableStartupFrameDrops = false,\n> };\n> \n> char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> phConfig[\"min_unicam_buffers\"].get(config_.minUnicamBuffers);\n> config_.minTotalUnicamBuffers =\n> phConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> + config_.disableStartupFrameDrops =\n> + phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n\nThat leads me to believe the parser should be optional here with\nvalue_or() ? or such? (applies to the other options too?)\n\nI guess it could be value_or(config_.disableStartupFrameDrops); to keep\nit the same... Those lines might get long though, but it could be\ncleaned up otherwise with a helper/macro maybe:\n\n\tCONFIG_OPTION(bool, disableStartupFrameDrops, disable_startup_frame_drops);\n\nas I expect this to grow with more options so keeping it readable may be\nhelpful.\n\n\n> if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\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 E2A2AC3240\n\tfor ;\n\tFri, 20 Jan 2023 11:17:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 686FC625E4;\n\tFri, 20 Jan 2023 12:17:14 +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 1A10861EFD\n\tfor ;\n\tFri, 20 Jan 2023 12:17:13 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 97701514;\n\tFri, 20 Jan 2023 12:17:12 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674213434;\n\tbh=tuw/ccugFNcVLRwIc3dgAu7RCWX7mROJW8PwGYVy5NE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=AF2x7Sm14HvvCIoQ9/BAJGudtWugsq/NxwLexuKsWYIj8MKHmhXw0BGwsxkrAFJWa\n\tmy56UAQiifAaeTTGOQ5Fx1hPw1UJktomlpvP31ukkIN+mbzu6R8z8G2a1h/oLqv1CB\n\t/90MgYl+qiHI2UTvycBHclsqtB9wng8IMxzQKxEFrE0I88YGv4EQrXHCy4LrM9mL/r\n\tfig0IGCsU4OQP1q+rh1VjoXvdZR4L+maZ4QpnRIHsrun+bA1Q0pBuRCdcKVVps2B9Y\n\tVJkx8E8ntmGaSxT1C0dBK1wWeZexCH777qCPWmo/BseSvnAMLro+68Fpepc606679j\n\tmB+phC/ZAKT7g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674213432;\n\tbh=tuw/ccugFNcVLRwIc3dgAu7RCWX7mROJW8PwGYVy5NE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=uBRmLmQwbq6mXg6mu6jJ1OVnfASrZGUZ8x7wy8GkXbclZq7xcwlTPyRxmyFl+D9r7\n\temRyQsbWc4dBEmlDnZPShsyflicz4zM6ZISOdGe2xb5p17mSV+eQw+5KR99wweYIMl\n\tpzOYAQTnzYqMk24ejOXUC4NBNcLWgsKURF84axGI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"uBRmLmQw\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20230118085953.7027-11-naush@raspberrypi.com>","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 20 Jan 2023 11:17:10 +0000","Message-ID":"<167421343001.42371.15378884961239215955@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26292,"web_url":"https://patchwork.libcamera.org/comment/26292/","msgid":"","date":"2023-01-20T11:55:56","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for your feedback!\n\nOn Fri, 20 Jan 2023 at 11:17, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> > Add a new pipeline config parameter \"disable_startup_frame_drops\" to\n> disable\n> > any startup drop frames, overriding the IPA request.\n> >\n> > When this parameter is set, it allows the pipeline handler to run with no\n> > internally allocated Unicam buffers (\"min_unicam_buffers\").\n> >\n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> > 2 files changed, 14 insertions(+), 2 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > index 001a906af528..421f30e62aa3 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > @@ -15,6 +15,10 @@\n> > #\n> > # internal buffer count = max(min_unicam_buffers,\n> > # min_total_unicam_buffers - external buffer\n> count)\n> > - \"min_total_unicam_buffers\": 4\n> > + \"min_total_unicam_buffers\": 4,\n> > +\n> > + # Override any request from the IPA to drop a number of\n> startup\n> > + # frames.\n> > + \"disable_startup_frame_drops\": false\n>\n> Many configuration files I've seen and used specify options as ...\n> optional. While the documentation describes the default value...\n>\n> > }\n> > }\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 39f48e0a57fb..3529d331deb6 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -309,6 +309,11 @@ public:\n> > * the Unicam Image stream.\n> > */\n> > unsigned int minTotalUnicamBuffers;\n> > + /*\n> > + * Override any request from the IPA to drop a number of\n> startup\n> > + * frames.\n> > + */\n> > + bool disableStartupFrameDrops;\n> > };\n> >\n> > Config config_;\n> > @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera,\n> const ControlList *controls)\n> > data->setSensorControls(startConfig.controls);\n> >\n> > /* Configure the number of dropped frames required on startup. */\n> > - data->dropFrameCount_ = startConfig.dropFrameCount;\n> > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> 0 : startConfig.dropFrameCount;\n> >\n> > for (auto const stream : data->streams_)\n> > stream->resetBuffers();\n> > @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> > config_ = {\n> > .minUnicamBuffers = 2,\n> > .minTotalUnicamBuffers = 4,\n> > + .disableStartupFrameDrops = false,\n> > };\n> >\n> > char const *configFromEnv =\n> utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> > phConfig[\"min_unicam_buffers\"].get int>(config_.minUnicamBuffers);\n> > config_.minTotalUnicamBuffers =\n> > phConfig[\"min_total_unicam_buffers\"].get int>(config_.minTotalUnicamBuffers);\n> > + config_.disableStartupFrameDrops =\n> > +\n> phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n>\n> That leads me to believe the parser should be optional here with\n> value_or() ? or such? (applies to the other options too?)\n>\n\nI think the above code does exactly that right?\nWe are using the T YamlObject::get(const T &defaultValue) signature here\nthat returns defaultValue if the key is not present.\n\nRegards,\nNaush\n\n\n> I guess it could be value_or(config_.disableStartupFrameDrops); to keep\n> it the same... Those lines might get long though, but it could be\n> cleaned up otherwise with a helper/macro maybe:\n>\n> CONFIG_OPTION(bool, disableStartupFrameDrops,\n> disable_startup_frame_drops);\n>\n> as I expect this to grow with more options so keeping it readable may be\n> helpful.\n>\n>\n> > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > LOG(RPI, Error) << \"Invalid configuration:\n> min_total_unicam_buffers must be >= min_unicam_buffers\";\n> > --\n> > 2.25.1\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 A8C79BE175\n\tfor ;\n\tFri, 20 Jan 2023 11:56:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 20741625D8;\n\tFri, 20 Jan 2023 12:56:15 +0100 (CET)","from mail-yb1-xb2e.google.com (mail-yb1-xb2e.google.com\n\t[IPv6:2607:f8b0:4864:20::b2e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8E6EC61EFD\n\tfor ;\n\tFri, 20 Jan 2023 12:56:13 +0100 (CET)","by mail-yb1-xb2e.google.com with SMTP id 66so6372835yba.4\n\tfor ;\n\tFri, 20 Jan 2023 03:56:13 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674215775;\n\tbh=NMEPmgxVu9jwXk5bpuHyd/1bGdhy0U1QOwhSEP96b/0=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=dkqbGc6fOR4r0gpoxyGq1cZXamBxIw4xa1eUJooiPWHOVuvZFRgABNnb8pCkXYxL3\n\twIpMmeuYQnCYm68Zxi6zuk9pFqpKpCvG1GhohGPS97qV4MAMxUCQV7Ohnkaw/wL+Gh\n\tVioG3qr3CnGQlnWO1PG2OOlIxjRmaQaaUKmPPpE08qo1qnApNZyTyb9DxeT+uKZiCY\n\tkcWhePiOD7MPEUvNgzmuLDiDyJbBpZ8GtT0UfZcM6HwWyjrXsdiVaXUMZL5w3dmBHZ\n\tJUimjGNcqoUxHX1WRjEcdloAliX1nUmnnmVhzZtD3EYf5wiUtdFGTkVZuzONzEeqch\n\tmATHAy0OK7u5g==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=qDaG+X+5CeM3dAJm0whAeII/6YHqR6QZW2UX9yd+QwE=;\n\tb=E9hcWPd4uQrilOc/mFU3TUAdCqC9GZ4EYHevo4EC0v/YwXpV7SLP1Ss9rprIXq04MM\n\t+nFdwebcz3ZcIYRacHMutu58BHPn877Uh21sAERrINetXFoYoyZv1ZNSFiGEr8CGEUci\n\t7tGzXt9sPy95UdTOLSXcxhFqDKZlhTwU3rOTT5DnmalhV4MlVM9GptY8wqgq91vY7/N/\n\t0T2pi/WHzr9yHLGLggjtXiSTARqsLiO2hrObjHsIVU2L7ThxBgLSP7k7cPJJx2WxIVeY\n\twJnTXEgr4JJdmi89FrLrTn7KU/PCtYJjYoAehtxG4+iY4HQ+kkoEk7UYR1klW8TN7zZp\n\tEQjg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"E9hcWPd4\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=qDaG+X+5CeM3dAJm0whAeII/6YHqR6QZW2UX9yd+QwE=;\n\tb=X5FSHJGnn//pkOfKiS5bJl/qFzFIcqVsew/IDgYkeQbRJaF7evHsboPpnhdaqGzAvU\n\tB3Bl6QPCel1EIEtluQ04CAXIo93uD3cifZlx4jWk+5UMSmAdDYOWlN90dLlVs9xOWWbO\n\tIf49lH6Ep/I3Z5svq3X2CDaOxZqxSQCjTptyfDAamOHAMRDErH99+SjX7ZQYRrUV8zJw\n\t4OBFxxxh/BaDkEBrjc+Op6YmDwLDOicEfjvGJpeoH9zEOzZFC/tMAMY8Ip9do61bnyO2\n\t+MO2ZCPkNaw5K1WN3QNN8mA3IO78jR6t9Os8+qFNukLip5NhC5yJ1Jse7YHkHmVkmYWC\n\tXwfw==","X-Gm-Message-State":"AFqh2koCtjqThvlWQoPrAcUB+A+X/+E57CzN9C+V6O/zuHrihZp7Zr6Q\n\t60L6IPXEkpoW203tvVhFN+tiMMJ8zYs48Ez8olVflw==","X-Google-Smtp-Source":"AMrXdXu3Jtqr/OEea78zkNNob5FLfB+GxZb8b/527w7VI4S2oN29eg0NtR0yJTZeQm927uBw4adFKFg6fu+xFgRqhBo=","X-Received":"by 2002:a25:e808:0:b0:7f1:de22:7e7c with SMTP id\n\tk8-20020a25e808000000b007f1de227e7cmr1329110ybd.330.1674215772469;\n\tFri, 20 Jan 2023 03:56:12 -0800 (PST)","MIME-Version":"1.0","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>\n\t<167421343001.42371.15378884961239215955@Monstersaurus>","In-Reply-To":"<167421343001.42371.15378884961239215955@Monstersaurus>","Date":"Fri, 20 Jan 2023 11:55:56 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"00000000000065520805f2b0bc79\"","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26294,"web_url":"https://patchwork.libcamera.org/comment/26294/","msgid":"<167422355720.1709372.16355026027316169144@Monstersaurus>","date":"2023-01-20T14:05:57","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2023-01-20 11:55:56)\n> Hi Kieran,\n> \n> Thank you for your feedback!\n> \n> On Fri, 20 Jan 2023 at 11:17, Kieran Bingham <\n> kieran.bingham@ideasonboard.com> wrote:\n> \n> > Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> > > Add a new pipeline config parameter \"disable_startup_frame_drops\" to\n> > disable\n> > > any startup drop frames, overriding the IPA request.\n> > >\n> > > When this parameter is set, it allows the pipeline handler to run with no\n> > > internally allocated Unicam buffers (\"min_unicam_buffers\").\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > Reviewed-by: David Plowman \n> > > ---\n> > > src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> > > 2 files changed, 14 insertions(+), 2 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > index 001a906af528..421f30e62aa3 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > @@ -15,6 +15,10 @@\n> > > #\n> > > # internal buffer count = max(min_unicam_buffers,\n> > > # min_total_unicam_buffers - external buffer\n> > count)\n> > > - \"min_total_unicam_buffers\": 4\n> > > + \"min_total_unicam_buffers\": 4,\n> > > +\n> > > + # Override any request from the IPA to drop a number of\n> > startup\n> > > + # frames.\n> > > + \"disable_startup_frame_drops\": false\n> >\n> > Many configuration files I've seen and used specify options as ...\n> > optional. While the documentation describes the default value...\n> >\n> > > }\n> > > }\n> > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > index 39f48e0a57fb..3529d331deb6 100644\n> > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > @@ -309,6 +309,11 @@ public:\n> > > * the Unicam Image stream.\n> > > */\n> > > unsigned int minTotalUnicamBuffers;\n> > > + /*\n> > > + * Override any request from the IPA to drop a number of\n> > startup\n> > > + * frames.\n> > > + */\n> > > + bool disableStartupFrameDrops;\n> > > };\n> > >\n> > > Config config_;\n> > > @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera,\n> > const ControlList *controls)\n> > > data->setSensorControls(startConfig.controls);\n> > >\n> > > /* Configure the number of dropped frames required on startup. */\n> > > - data->dropFrameCount_ = startConfig.dropFrameCount;\n> > > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> > 0 : startConfig.dropFrameCount;\n> > >\n> > > for (auto const stream : data->streams_)\n> > > stream->resetBuffers();\n> > > @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> > > config_ = {\n> > > .minUnicamBuffers = 2,\n> > > .minTotalUnicamBuffers = 4,\n> > > + .disableStartupFrameDrops = false,\n> > > };\n> > >\n> > > char const *configFromEnv =\n> > utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > > @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> > > phConfig[\"min_unicam_buffers\"].get > int>(config_.minUnicamBuffers);\n> > > config_.minTotalUnicamBuffers =\n> > > phConfig[\"min_total_unicam_buffers\"].get > int>(config_.minTotalUnicamBuffers);\n> > > + config_.disableStartupFrameDrops =\n> > > +\n> > phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> >\n> > That leads me to believe the parser should be optional here with\n> > value_or() ? or such? (applies to the other options too?)\n> >\n> \n> I think the above code does exactly that right?\n> We are using the T YamlObject::get(const T &defaultValue) signature here\n> that returns defaultValue if the key is not present.\n\nAhh - yes - I'm completely blind! Of course it is!\n\nSorry for the distraction.\n\nAha, which also means I should add this:\n\nReviewed-by: Kieran Bingham \n\n> \n> Regards,\n> Naush\n> \n> \n> > I guess it could be value_or(config_.disableStartupFrameDrops); to keep\n> > it the same... Those lines might get long though, but it could be\n> > cleaned up otherwise with a helper/macro maybe:\n> >\n> > CONFIG_OPTION(bool, disableStartupFrameDrops,\n> > disable_startup_frame_drops);\n> >\n> > as I expect this to grow with more options so keeping it readable may be\n> > helpful.\n> >\n> >\n> > > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > > LOG(RPI, Error) << \"Invalid configuration:\n> > min_total_unicam_buffers must be >= min_unicam_buffers\";\n> > > --\n> > > 2.25.1\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 6C191BE175\n\tfor ;\n\tFri, 20 Jan 2023 14:06:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C6418625E4;\n\tFri, 20 Jan 2023 15:06:02 +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 8B09F61EFD\n\tfor ;\n\tFri, 20 Jan 2023 15:06:00 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0F1B6514;\n\tFri, 20 Jan 2023 15:06:00 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674223562;\n\tbh=yFH89VMbRwHDmU7xgDYeBINsrAHXzpxk5Zz+d3PkXBw=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=m+9lc7tvJzviqmxHDDn9nD2odo5g+NR6hVqZC9xhB3tdltgY8sFfTmEP1BQHD7qrx\n\th+NdkA9znL3ZeNCoMxrmS475EyjTMtWFMf1CN4SWMqsRXnATsb0RQn7rtVRVJCw5w8\n\tAIsRAOuMBJ7PD1S38jzEQ4xbHIUXy65ui0mmrggcp/Ia4YsknIaqXEr75G1NB3vNF/\n\tifJYkRsyjYlA58n9EIZp6WrHkriOK1qbplPbDOOjAgTd1YslgBfGucSBzcWYVCmt9b\n\tFRzBgRK5mYJd2nhBzIbpDrDivSuLIokspzmm4o5FoxxIQyQjMz8YZ4W8e2ukyuUM4x\n\tqW1AqLi6XhcwA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674223560;\n\tbh=yFH89VMbRwHDmU7xgDYeBINsrAHXzpxk5Zz+d3PkXBw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=j6rvwCvfKRQxvJlCNLzFyr87JKjuY1LNKoF4axhTtw+q/i1aJOUYAlyGvXP6wMWpa\n\tt6v6o92ymUW9AMnWnxwcmSWUlnz8KIDvqi6lu36pHY+TAukfA8J49XnWzVzD7JTM5/\n\tU1bkcpzgCJqhiw2a3sqR122XIQm46733mXc4TjVY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"j6rvwCvf\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>\n\t<167421343001.42371.15378884961239215955@Monstersaurus>\n\t","To":"Naushir Patuck ","Date":"Fri, 20 Jan 2023 14:05:57 +0000","Message-ID":"<167422355720.1709372.16355026027316169144@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26316,"web_url":"https://patchwork.libcamera.org/comment/26316/","msgid":"","date":"2023-01-22T22:56:37","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Fri, Jan 20, 2023 at 02:05:57PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck (2023-01-20 11:55:56)\n> > On Fri, 20 Jan 2023 at 11:17, Kieran Bingham wrote:\n> > > Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> > > > Add a new pipeline config parameter \"disable_startup_frame_drops\" to disable\n> > > > any startup drop frames, overriding the IPA request.\n> > > >\n> > > > When this parameter is set, it allows the pipeline handler to run with no\n> > > > internally allocated Unicam buffers (\"min_unicam_buffers\").\n\nAt some point I'm sure I'll be tempted to propose dropping the startup\nframe drops mechanism altogether. For now, this sounds good.\n\n> > > > Signed-off-by: Naushir Patuck \n> > > > Reviewed-by: David Plowman \n> > > > ---\n> > > > src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> > > > 2 files changed, 14 insertions(+), 2 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > index 001a906af528..421f30e62aa3 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > @@ -15,6 +15,10 @@\n> > > > #\n> > > > # internal buffer count = max(min_unicam_buffers,\n> > > > # min_total_unicam_buffers - external buffer count)\n> > > > - \"min_total_unicam_buffers\": 4\n> > > > + \"min_total_unicam_buffers\": 4,\n> > > > +\n> > > > + # Override any request from the IPA to drop a number of startup\n> > > > + # frames.\n> > > > + \"disable_startup_frame_drops\": false\n> > >\n> > > Many configuration files I've seen and used specify options as ...\n> > > optional. While the documentation describes the default value...\n> > >\n> > > > }\n> > > > }\n> > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > index 39f48e0a57fb..3529d331deb6 100644\n> > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > @@ -309,6 +309,11 @@ public:\n> > > > * the Unicam Image stream.\n> > > > */\n> > > > unsigned int minTotalUnicamBuffers;\n> > > > + /*\n> > > > + * Override any request from the IPA to drop a number of startup\n> > > > + * frames.\n> > > > + */\n> > > > + bool disableStartupFrameDrops;\n> > > > };\n> > > >\n> > > > Config config_;\n> > > > @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> > > > data->setSensorControls(startConfig.controls);\n> > > >\n> > > > /* Configure the number of dropped frames required on startup. */\n> > > > - data->dropFrameCount_ = startConfig.dropFrameCount;\n> > > > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount;\n\n\tdata->dropFrameCount_ = data->config_.disableStartupFrameDrops\n\t\t\t ? 0 : startConfig.dropFrameCount;\n\nReviewed-by: Laurent Pinchart \n\n> > > >\n> > > > for (auto const stream : data->streams_)\n> > > > stream->resetBuffers();\n> > > > @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> > > > config_ = {\n> > > > .minUnicamBuffers = 2,\n> > > > .minTotalUnicamBuffers = 4,\n> > > > + .disableStartupFrameDrops = false,\n> > > > };\n> > > >\n> > > > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > > > @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> > > > phConfig[\"min_unicam_buffers\"].get(config_.minUnicamBuffers);\n> > > > config_.minTotalUnicamBuffers =\n> > > > phConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> > > > + config_.disableStartupFrameDrops =\n> > > > + phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> > >\n> > > That leads me to believe the parser should be optional here with\n> > > value_or() ? or such? (applies to the other options too?)\n> > \n> > I think the above code does exactly that right?\n> > We are using the T YamlObject::get(const T &defaultValue) signature here\n> > that returns defaultValue if the key is not present.\n> \n> Ahh - yes - I'm completely blind! Of course it is!\n> \n> Sorry for the distraction.\n> \n> Aha, which also means I should add this:\n> \n> Reviewed-by: Kieran Bingham \n> \n> > > I guess it could be value_or(config_.disableStartupFrameDrops); to keep\n> > > it the same... Those lines might get long though, but it could be\n> > > cleaned up otherwise with a helper/macro maybe:\n> > >\n> > > CONFIG_OPTION(bool, disableStartupFrameDrops,\n> > > disable_startup_frame_drops);\n> > >\n> > > as I expect this to grow with more options so keeping it readable may be\n> > > helpful.\n> > >\n> > > > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > > > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";","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 09BFFBDB1D\n\tfor ;\n\tSun, 22 Jan 2023 22:56:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7AF7E625DF;\n\tSun, 22 Jan 2023 23:56:41 +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 EB6A3625DD\n\tfor ;\n\tSun, 22 Jan 2023 23:56:39 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 56B7C471;\n\tSun, 22 Jan 2023 23:56:39 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674428201;\n\tbh=B4G+yEFczX8mSYh1Ly8wdX49AKdB3omoJVd9suDqjiA=;\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=HuRn6uk+ogzwh3L7uQF1V5xTZldP4+eofkMeG6KoNSGjR6zN6/y6W5NIaImrPnxAe\n\tFZa5laj8i6lLHoBpB5vGp2uRov5Uy0bMdSVIgtCdAZpTUFvIfutS0M8WxtRPbc8dKd\n\tRg2WxbBYycZL5qvNRBEQux/J3EoqLwDE5s97XYho7FLIHPPLV9w8hS41cIhhYHFsFW\n\tORweTzQ9JagcImuGC10VBiannjdHp9naGYeWihtPuc5zHkJBaKUwbSRs4m8DU4CB9d\n\tRCnvHXg02GorT/wRklb7xWE1IZTsHCIBJqaGKvNwlv2XdxZd1U8VuyE0EZEX9cpxXx\n\tc/dRsolrHUuKw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674428199;\n\tbh=B4G+yEFczX8mSYh1Ly8wdX49AKdB3omoJVd9suDqjiA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=K2nW4CaZWDgsGiVo310K85vecfGhxL2nTqgLij/08iEzyibqRHJV0+sH/vcB4UhXa\n\tWNqT25FlpmxFGQAKruW4iF4BjQDh4hSyHk+7qNOBEHwzc0XJHNt7CN88PRJewSy8/h\n\tcPvjPO+QpmpHwpfbzL0WYMD98vzeovnBNl3V+v1s="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"K2nW4CaZ\"; dkim-atps=neutral","Date":"Mon, 23 Jan 2023 00:56:37 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>\n\t<167421343001.42371.15378884961239215955@Monstersaurus>\n\t\n\t<167422355720.1709372.16355026027316169144@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<167422355720.1709372.16355026027316169144@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26318,"web_url":"https://patchwork.libcamera.org/comment/26318/","msgid":"","date":"2023-01-22T23:09:19","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Fri, Jan 20, 2023 at 11:17:10AM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> > Add a new pipeline config parameter \"disable_startup_frame_drops\" to disable\n> > any startup drop frames, overriding the IPA request.\n> > \n> > When this parameter is set, it allows the pipeline handler to run with no\n> > internally allocated Unicam buffers (\"min_unicam_buffers\").\n> > \n> > Signed-off-by: Naushir Patuck \n> > Reviewed-by: David Plowman \n> > ---\n> > src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> > 2 files changed, 14 insertions(+), 2 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > index 001a906af528..421f30e62aa3 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > @@ -15,6 +15,10 @@\n> > #\n> > # internal buffer count = max(min_unicam_buffers,\n> > # min_total_unicam_buffers - external buffer count)\n> > - \"min_total_unicam_buffers\": 4\n> > + \"min_total_unicam_buffers\": 4,\n> > +\n> > + # Override any request from the IPA to drop a number of startup\n> > + # frames.\n> > + \"disable_startup_frame_drops\": false\n> \n> Many configuration files I've seen and used specify options as ...\n> optional. While the documentation describes the default value...\n\nOn this topic, we could also comment out the default parameters in this\nfile, that's a common practice too.\n\n> > }\n> > }\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 39f48e0a57fb..3529d331deb6 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -309,6 +309,11 @@ public:\n> > * the Unicam Image stream.\n> > */\n> > unsigned int minTotalUnicamBuffers;\n> > + /*\n> > + * Override any request from the IPA to drop a number of startup\n> > + * frames.\n> > + */\n> > + bool disableStartupFrameDrops;\n> > };\n> > \n> > Config config_;\n> > @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> > data->setSensorControls(startConfig.controls);\n> > \n> > /* Configure the number of dropped frames required on startup. */\n> > - data->dropFrameCount_ = startConfig.dropFrameCount;\n> > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount;\n> > \n> > for (auto const stream : data->streams_)\n> > stream->resetBuffers();\n> > @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> > config_ = {\n> > .minUnicamBuffers = 2,\n> > .minTotalUnicamBuffers = 4,\n> > + .disableStartupFrameDrops = false,\n> > };\n> > \n> > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> > phConfig[\"min_unicam_buffers\"].get(config_.minUnicamBuffers);\n> > config_.minTotalUnicamBuffers =\n> > phConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> > + config_.disableStartupFrameDrops =\n> > + phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> \n> That leads me to believe the parser should be optional here with\n> value_or() ? or such? (applies to the other options too?)\n> \n> I guess it could be value_or(config_.disableStartupFrameDrops); to keep\n> it the same... Those lines might get long though, but it could be\n> cleaned up otherwise with a helper/macro maybe:\n> \n> \tCONFIG_OPTION(bool, disableStartupFrameDrops, disable_startup_frame_drops);\n> \n> as I expect this to grow with more options so keeping it readable may be\n> helpful.\n> \n> > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";","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 A6093BDB1D\n\tfor ;\n\tSun, 22 Jan 2023 23:09:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 060DA625E4;\n\tMon, 23 Jan 2023 00:09:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9908A61EFC\n\tfor ;\n\tMon, 23 Jan 2023 00:09:22 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 06BFF74C;\n\tMon, 23 Jan 2023 00:09:21 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674428964;\n\tbh=03aRrIIqrVQSRvE7Zf456DRP4oF/o6vh1e0nCQOO5x0=;\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=rIxUpiAA3XbzoNCM68OZjGJpB2gfKR7LBX9rubpVN8HfU0nMCJI3UJ2eXZ+hTmM5d\n\tn3K4g72vSHeDYzfp/f+Zbm9WG0rT1lU7wxw4OluyMoJGRLNuQL4uh34woX/Bf6enCB\n\tMJ2KWAnY97FbXdsKcn7zw9ndOZVyCK9Wi2xcdZIJPck5pXEb2RA12aNJtxrWeVOHE8\n\tJmeRKd+ZNTZi+WdizvVxz2IzoXV7g+A5gVpS0y8rYtYYw3Cam8b5OiGxe5NJRmMsjN\n\txMYZBgo74f0AJeSJegqikvxh8RnsfhaDlFCBFFGuMPfJp2anrSUG+HG60GvIsTcrXO\n\tp7erSYouFfHXg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1674428962;\n\tbh=03aRrIIqrVQSRvE7Zf456DRP4oF/o6vh1e0nCQOO5x0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OP/NFTSN7eVfP7HF3PzVOQeURpsquJffOQAq/8AOMDPF+PxxcB6sQrvB7SUzZ1InE\n\tbqcJkvCxA7yS8R3gYFqSIyxx90xBmWMjHkK3PU349db/0WPdBBqUph62sn1XIPkEr/\n\tMovywBfJw6+D2t9fsYS5M5m3ob27hWXFw19qfPp8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OP/NFTSN\"; dkim-atps=neutral","Date":"Mon, 23 Jan 2023 01:09:19 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>\n\t<167421343001.42371.15378884961239215955@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<167421343001.42371.15378884961239215955@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26353,"web_url":"https://patchwork.libcamera.org/comment/26353/","msgid":"","date":"2023-01-25T14:13:38","subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Sun, 22 Jan 2023 at 22:56, Laurent Pinchart\n wrote:\n>\n> Hello,\n>\n> On Fri, Jan 20, 2023 at 02:05:57PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Quoting Naushir Patuck (2023-01-20 11:55:56)\n> > > On Fri, 20 Jan 2023 at 11:17, Kieran Bingham wrote:\n> > > > Quoting Naushir Patuck via libcamera-devel (2023-01-18 08:59:51)\n> > > > > Add a new pipeline config parameter \"disable_startup_frame_drops\" to disable\n> > > > > any startup drop frames, overriding the IPA request.\n> > > > >\n> > > > > When this parameter is set, it allows the pipeline handler to run with no\n> > > > > internally allocated Unicam buffers (\"min_unicam_buffers\").\n>\n> At some point I'm sure I'll be tempted to propose dropping the startup\n> frame drops mechanism altogether. For now, this sounds good.\n\nIf this is to be removed, it would be nice to have a mechanism to flag frames\nthat are not suitable for display or encoding so the application knows to drop\nit.\n\n\n>\n> > > > > Signed-off-by: Naushir Patuck \n> > > > > Reviewed-by: David Plowman \n> > > > > ---\n> > > > > src/libcamera/pipeline/raspberrypi/data/example.yaml | 6 +++++-\n> > > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 10 +++++++++-\n> > > > > 2 files changed, 14 insertions(+), 2 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/raspberrypi/data/example.yaml b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > > index 001a906af528..421f30e62aa3 100644\n> > > > > --- a/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > > +++ b/src/libcamera/pipeline/raspberrypi/data/example.yaml\n> > > > > @@ -15,6 +15,10 @@\n> > > > > #\n> > > > > # internal buffer count = max(min_unicam_buffers,\n> > > > > # min_total_unicam_buffers - external buffer count)\n> > > > > - \"min_total_unicam_buffers\": 4\n> > > > > + \"min_total_unicam_buffers\": 4,\n> > > > > +\n> > > > > + # Override any request from the IPA to drop a number of startup\n> > > > > + # frames.\n> > > > > + \"disable_startup_frame_drops\": false\n> > > >\n> > > > Many configuration files I've seen and used specify options as ...\n> > > > optional. While the documentation describes the default value...\n> > > >\n> > > > > }\n> > > > > }\n> > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > index 39f48e0a57fb..3529d331deb6 100644\n> > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > > > @@ -309,6 +309,11 @@ public:\n> > > > > * the Unicam Image stream.\n> > > > > */\n> > > > > unsigned int minTotalUnicamBuffers;\n> > > > > + /*\n> > > > > + * Override any request from the IPA to drop a number of startup\n> > > > > + * frames.\n> > > > > + */\n> > > > > + bool disableStartupFrameDrops;\n> > > > > };\n> > > > >\n> > > > > Config config_;\n> > > > > @@ -1053,7 +1058,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)\n> > > > > data->setSensorControls(startConfig.controls);\n> > > > >\n> > > > > /* Configure the number of dropped frames required on startup. */\n> > > > > - data->dropFrameCount_ = startConfig.dropFrameCount;\n> > > > > + data->dropFrameCount_ = data->config_.disableStartupFrameDrops ? 0 : startConfig.dropFrameCount;\n>\n> data->dropFrameCount_ = data->config_.disableStartupFrameDrops\n> ? 0 : startConfig.dropFrameCount;\n>\n> Reviewed-by: Laurent Pinchart \n>\n> > > > >\n> > > > > for (auto const stream : data->streams_)\n> > > > > stream->resetBuffers();\n> > > > > @@ -1706,6 +1711,7 @@ int RPiCameraData::configurePipeline()\n> > > > > config_ = {\n> > > > > .minUnicamBuffers = 2,\n> > > > > .minTotalUnicamBuffers = 4,\n> > > > > + .disableStartupFrameDrops = false,\n> > > > > };\n> > > > >\n> > > > > char const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RPI_CONFIG_FILE\");\n> > > > > @@ -1739,6 +1745,8 @@ int RPiCameraData::configurePipeline()\n> > > > > phConfig[\"min_unicam_buffers\"].get(config_.minUnicamBuffers);\n> > > > > config_.minTotalUnicamBuffers =\n> > > > > phConfig[\"min_total_unicam_buffers\"].get(config_.minTotalUnicamBuffers);\n> > > > > + config_.disableStartupFrameDrops =\n> > > > > + phConfig[\"disable_startup_frame_drops\"].get(config_.disableStartupFrameDrops);\n> > > >\n> > > > That leads me to believe the parser should be optional here with\n> > > > value_or() ? or such? (applies to the other options too?)\n> > >\n> > > I think the above code does exactly that right?\n> > > We are using the T YamlObject::get(const T &defaultValue) signature here\n> > > that returns defaultValue if the key is not present.\n> >\n> > Ahh - yes - I'm completely blind! Of course it is!\n> >\n> > Sorry for the distraction.\n> >\n> > Aha, which also means I should add this:\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> > > > I guess it could be value_or(config_.disableStartupFrameDrops); to keep\n> > > > it the same... Those lines might get long though, but it could be\n> > > > cleaned up otherwise with a helper/macro maybe:\n> > > >\n> > > > CONFIG_OPTION(bool, disableStartupFrameDrops,\n> > > > disable_startup_frame_drops);\n> > > >\n> > > > as I expect this to grow with more options so keeping it readable may be\n> > > > helpful.\n> > > >\n> > > > > if (config_.minTotalUnicamBuffers < config_.minUnicamBuffers) {\n> > > > > LOG(RPI, Error) << \"Invalid configuration: min_total_unicam_buffers must be >= min_unicam_buffers\";\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 3CC40BDB1D\n\tfor ;\n\tWed, 25 Jan 2023 14:13:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A54AE625E4;\n\tWed, 25 Jan 2023 15:13:57 +0100 (CET)","from mail-yw1-x1133.google.com (mail-yw1-x1133.google.com\n\t[IPv6:2607:f8b0:4864:20::1133])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A0EC6603C0\n\tfor ;\n\tWed, 25 Jan 2023 15:13:55 +0100 (CET)","by mail-yw1-x1133.google.com with SMTP id\n\t00721157ae682-4ff1fa82bbbso218360127b3.10\n\tfor ;\n\tWed, 25 Jan 2023 06:13:55 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1674656037;\n\tbh=aqxZ2qFN8DeWuyzVygr6nK/5IrT4u+VpfrKQR8lDDuE=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=WqWqDwXumen8JuXn6asLlP6rfl/oeQoD0tKsGecRFM1+G7vC9tjC+AQjcnpaPLqpU\n\t1oM+bZtPDEnQXUqvKBU/xDZ//EQVkZKEC7rQfqat9BmqYmXXda20WE+WaCVA2WPPkS\n\tvJHRWjlTyHsl1eUuuGi9ZVa9+H38c1nZNGCTwRC9Zj4qhMoiCUYVezshge1ykhge1I\n\tQPVsi1IkWUBUCvQmTSUVZ2941ZXdAlcjbXZxeRkDl33U8QZdVjuGa7lNJ33UuFnlTz\n\tPk34fJ+9nbDiJIbnr1eZb1Xj4AhG4VG+07bI0DsLvFvnFjZ3J6zDPtQHRJkWKepHho\n\ttQipg2bLZ69Pw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=/5CneJiQ5FszxYuZtwoDOejK+VF9130SwzLV+1IWARM=;\n\tb=NzwO4e/QP+ecZUrpJlITvzWQoNj24g7dFTiuG4dpXn3FhE2TSQCurO0F3VvNPDF7bL\n\tbfWeQJ1mPdOleRMxHoYH6/UH+ArR+fS9d0d54pGbvPMy4mZiRbS747o17CPnRXcivNFB\n\t8R0G/Vsht1WDmcZM1vhEoUfZQnCAZRkR5PNoclekZxQo6lM8alEsbaDY2PVAsqp43aj2\n\tGYffLA3qKyE6H7/CeoTDq5OVdeVeV9xmz4YUzWx1oSO/EHsAgIFCZD7Y35if47qd8JJ+\n\toHLLRcNSc9DFpE5twrw/gmnyjbOSobgoMSWZVSNqpt+xdlPdUGld+XC+ntuVYPUHA2s3\n\t2Iqg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"NzwO4e/Q\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=/5CneJiQ5FszxYuZtwoDOejK+VF9130SwzLV+1IWARM=;\n\tb=EjkQ91goWr+2KU5nfXL1PVFdTjAYAHleZ7aHdhhuy0cR6nPt31p631V/Msb4V7zGkQ\n\tUgqWW0Db+uqsDflOUTGyotrEKqR3BbVGAu0UR6gVJYAic73gaQvyPnIK63meaAxPLqlk\n\tAruYLL5XUZmpvEeFdXK+NWHMUicsMrOAkQ0JyfrLrDv/yplAzkteNFyV5VYvDhvsH+99\n\ts977PPnsZbsEDFNMtjSttNojJNKniQ+ztnHn7Yez7JZ0QpI9b7lf+/409ZeUFZVx2klF\n\tl3CqRIWVw8IFdDSdEocdK7sQCoAe1xyoNFwpBQIFZIGGOnimj+K9FVIhKSh5cIUCDOrU\n\tFnLA==","X-Gm-Message-State":"AFqh2koc5i8grQmZahq9WxZmkrY7bbeBqVSMgtS3q17XBTfpnkXCClDg\n\txDO+a54T7btBn822mqcmy0PYcUBPwEDXXzMq1vBaXkQVMARAlG8yLBM=","X-Google-Smtp-Source":"AMrXdXsyHc67LSLOmA4Cn2kGbu+V+RIrKRtsi6VR+4+HxbFrfAOZ71EfcBC8X+Y2SmYw94q0RafGO7o5F2Q44kTRKro=","X-Received":"by 2002:a81:7186:0:b0:501:31a3:fa0b with SMTP id\n\tm128-20020a817186000000b0050131a3fa0bmr2352224ywc.493.1674656034369;\n\tWed, 25 Jan 2023 06:13:54 -0800 (PST)","MIME-Version":"1.0","References":"<20230118085953.7027-1-naush@raspberrypi.com>\n\t<20230118085953.7027-11-naush@raspberrypi.com>\n\t<167421343001.42371.15378884961239215955@Monstersaurus>\n\t\n\t<167422355720.1709372.16355026027316169144@Monstersaurus>\n\t","In-Reply-To":"","Date":"Wed, 25 Jan 2023 14:13:38 +0000","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH v5 10/12] pipeline: raspberrypi: Add a\n\tparameter to disable startup drop frames","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":"Naushir Patuck via libcamera-devel\n\t","Reply-To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]