[{"id":34274,"web_url":"https://patchwork.libcamera.org/comment/34274/","msgid":"","date":"2025-05-19T10:26:31","subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush\n\nThanks for the patch.\n\nOn Mon, 19 May 2025 at 10:23, Naushir Patuck wrote:\n>\n> Split the pipeline handler drop frame tracking into startup frames and\n> invalid frames, as reported by the IPA.\n>\n> Remove the drop buffer handling logic in the pipeline handler. Now all\n> image buffers are returned out with the appropriate FrameStatus set\n> for startup or invalid frames.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 88 +++++++------------\n> .../pipeline/rpi/common/pipeline_base.h | 5 +-\n> 2 files changed, 37 insertions(+), 56 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 5956485953a2..21f2daf5bab5 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -659,9 +659,9 @@ int PipelineHandlerBase::start(Camera *camera, const ControlList *controls)\n> if (!result.controls.empty())\n> data->setSensorControls(result.controls);\n>\n> - /* Configure the number of dropped frames required on startup. */\n> - data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> - 0 : result.startupFrameCount + result.invalidFrameCount;\n> + /* Configure the number of startup and invalid frames reported by the IPA. */\n> + data->startupFrameCount_ = result.startupFrameCount;\n> + data->invalidFrameCount_ = result.invalidFrameCount;\n>\n> for (auto const stream : data->streams_)\n> stream->resetBuffers();\n> @@ -678,7 +678,6 @@ int PipelineHandlerBase::start(Camera *camera, const ControlList *controls)\n> data->buffersAllocated_ = true;\n> }\n>\n> - /* We need to set the dropFrameCount_ before queueing buffers. */\n> ret = queueAllBuffers(camera);\n> if (ret) {\n> LOG(RPI, Error) << \"Failed to queue buffers\";\n> @@ -898,23 +897,6 @@ int PipelineHandlerBase::queueAllBuffers(Camera *camera)\n> ret = stream->queueAllBuffers();\n> if (ret < 0)\n> return ret;\n> - } else {\n> - /*\n> - * For external streams, we must queue up a set of internal\n> - * buffers to handle the number of drop frames requested by\n> - * the IPA. This is done by passing nullptr in queueBuffer().\n> - *\n> - * The below queueBuffer() call will do nothing if there\n> - * are not enough internal buffers allocated, but this will\n> - * be handled by queuing the request for buffers in the\n> - * RPiStream object.\n> - */\n> - unsigned int i;\n> - for (i = 0; i < data->dropFrameCount_; i++) {\n> - ret = stream->queueBuffer(nullptr);\n> - if (ret)\n> - return ret;\n> - }\n> }\n> }\n>\n> @@ -1412,7 +1394,15 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> * buffer back to the stream.\n> */\n> Request *request = requestQueue_.empty() ? nullptr : requestQueue_.front();\n> - if (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {\n> + if (request && request->findBuffer(stream) == buffer) {\n> + FrameMetadata &md = buffer->_d()->metadata();\n> +\n> + /* Mark the non-converged and invalid frames in the metadata. */\n> + if (startupFrameCount_)\n> + md.status = FrameMetadata::Status::FrameStartup;\n> + if (invalidFrameCount_)\n> + md.status = FrameMetadata::Status::FrameError;\n\nMaybe \"if (validFrameCount_) ...\" followed by \"else if\n(startupFrameCount_) ...\" would be slightly easier to follow? I know,\nmakes no difference.\n\nReviewed-by: David Plowman \n\nThanks\nDavid\n\n> +\n> /*\n> * Tag the buffer as completed, returning it to the\n> * application.\n> @@ -1458,42 +1448,32 @@ void CameraData::handleState()\n>\n> void CameraData::checkRequestCompleted()\n> {\n> - bool requestCompleted = false;\n> - /*\n> - * If we are dropping this frame, do not touch the request, simply\n> - * change the state to IDLE when ready.\n> - */\n> - if (!dropFrameCount_) {\n> - Request *request = requestQueue_.front();\n> - if (request->hasPendingBuffers())\n> - return;\n> + Request *request = requestQueue_.front();\n> + if (request->hasPendingBuffers())\n> + return;\n>\n> - /* Must wait for metadata to be filled in before completing. */\n> - if (state_ != State::IpaComplete)\n> - return;\n> + /* Must wait for metadata to be filled in before completing. */\n> + if (state_ != State::IpaComplete)\n> + return;\n>\n> - LOG(RPI, Debug) << \"Completing request sequence: \"\n> - << request->sequence();\n> + LOG(RPI, Debug) << \"Completing request sequence: \"\n> + << request->sequence();\n>\n> - pipe()->completeRequest(request);\n> - requestQueue_.pop();\n> - requestCompleted = true;\n> - }\n> + pipe()->completeRequest(request);\n> + requestQueue_.pop();\n>\n> - /*\n> - * Make sure we have three outputs completed in the case of a dropped\n> - * frame.\n> - */\n> - if (state_ == State::IpaComplete &&\n> - ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||\n> - requestCompleted)) {\n> - LOG(RPI, Debug) << \"Going into Idle state\";\n> - state_ = State::Idle;\n> - if (dropFrameCount_) {\n> - dropFrameCount_--;\n> - LOG(RPI, Debug) << \"Dropping frame at the request of the IPA (\"\n> - << dropFrameCount_ << \" left)\";\n> - }\n> + LOG(RPI, Debug) << \"Going into Idle state\";\n> + state_ = State::Idle;\n> +\n> + if (startupFrameCount_) {\n> + startupFrameCount_--;\n> + LOG(RPI, Debug) << \"Decrementing startup frames to \"\n> + << startupFrameCount_;\n> + }\n> + if (invalidFrameCount_) {\n> + invalidFrameCount_--;\n> + LOG(RPI, Debug) << \"Decrementing invalid frames to \"\n> + << invalidFrameCount_;\n> }\n> }\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index aae0c2f35888..6023f9f9d6b3 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -48,7 +48,7 @@ class CameraData : public Camera::Private\n> public:\n> CameraData(PipelineHandler *pipe)\n> : Camera::Private(pipe), state_(State::Stopped),\n> - dropFrameCount_(0), buffersAllocated_(false),\n> + startupFrameCount_(0), invalidFrameCount_(0), buffersAllocated_(false),\n> ispOutputCount_(0), ispOutputTotal_(0)\n> {\n> }\n> @@ -151,7 +151,8 @@ public:\n> /* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> std::map cropParams_;\n>\n> - unsigned int dropFrameCount_;\n> + unsigned int startupFrameCount_;\n> + unsigned int invalidFrameCount_;\n>\n> /*\n> * If set, this stores the value that represets a gain of one for\n> --\n> 2.43.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 7DFBCBD78E\n\tfor ;\n\tMon, 19 May 2025 10:26:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 401E968D7A;\n\tMon, 19 May 2025 12:26:45 +0200 (CEST)","from mail-qt1-x833.google.com (mail-qt1-x833.google.com\n\t[IPv6:2607:f8b0:4864:20::833])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 27C0068B67\n\tfor ;\n\tMon, 19 May 2025 12:26:43 +0200 (CEST)","by mail-qt1-x833.google.com with SMTP id\n\td75a77b69052e-48d71b77cc0so59950121cf.1\n\tfor ;\n\tMon, 19 May 2025 03:26:43 -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=\"duVR2Y8w\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747650402; x=1748255202;\n\tdarn=lists.libcamera.org; \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=pgVIbCilwepVNtLyJuo+IDD0hkGUx+OCrCUAFOTji30=;\n\tb=duVR2Y8wIU3ePRha41vPVT7rc5+ydodZmeMH6CZu8fnLCeehj1kXgEHSdhqFP1TFBH\n\tOprcCvf96m3nriJEQoQMku+jG7+91I6DyPA/uwfE3hTzF1YLGLudCInRiEwOgzlwUJZH\n\tprRL4iN14vqT50DyuLOlDuQuqF/GD+6sS+F8jdC2rsewEbE/CU/M6DPLC3PQfmkCtLRT\n\tWVvAudablAVT834PS86Aig/JqXsx0+fVraibzmQ9e5xqB95oixG9gNxc06+BLXDHnpYn\n\tRw97N/XIBrlqwonEqTv1BgFWsz60tCCzEh4xx5a6kyFeB+W1m+LT0Os0B3g//lGbG5ou\n\tnYEA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747650402; x=1748255202;\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=pgVIbCilwepVNtLyJuo+IDD0hkGUx+OCrCUAFOTji30=;\n\tb=Vjy567mBuRKB/1/c0fbmIlcD11FMW9frhPze99g49A0JtYLbKkR4JYOmOJYKYSacYq\n\toitiH0MPBv4N9sKQpvA9f07AJrHuHQ1Ax+7RvIBJyQe45Cd0e2zfFvuh2rbWerPvWFyx\n\twEeSOnNhzLPxDb+EV87C3wirRg7ApKaiC81skFCpxMAWaioDylsmofJvWRUiJPwLtwvb\n\t4b03j22E0l14fGgiCVhI2hT+cRM2RirOAc7LuF41b+UFgvgTisgjKAGG/QjrEXOZdjyM\n\tpgMxHz2V2kDT7wrVPaqeyFzY8EGur0Sm36A15xucXBDws6uaRB+Wo6l4QrR/TpJRZ6t7\n\tyesA==","X-Gm-Message-State":"AOJu0Yx5geZBJjQrU+Jt9BDywZOo2pyRCAEwAQLGE0cWIeRQFVM9Tnsx\n\tZwOBuMYutntOriz3kpKGJEK3jxQ35d76dwKLaosuUZcLGyTX0pWaIuRp6ECeIRE8L5618pTVQ6D\n\tH3GO0uJQnojcH2eDBLvXEllscJ0ZC2wWknYFaDpXweIQLvRsRkPKSFmY=","X-Gm-Gg":"ASbGncsdL2cGU9bI23LmuNDp5zFK3FutWOElNkp43fbN+84UIm61adRIqk/sFRZtBjk\n\t0eV/i9zA/4rizEIpOkohgMGLwakpq7sTEmLgqHKgHko+txmzzxGrvzqfr183MAiN02BHjOdVqZI\n\tJnBnkv/FUhV2C3aN8qhDS1zDv5n99OoppyZB10tm68bfhn81TU3mEMB2EL9BIV8VY=","X-Google-Smtp-Source":"AGHT+IEaO15vyYeqc3f3AXT/FmopiwLrtgGB2VKsYhx7tsqAtSwfHUKr09D9XM8huSrbEkz1qxBAGxp55Pof+fHJOqw=","X-Received":"by 2002:a05:622a:2605:b0:494:9fc0:de3 with SMTP id\n\td75a77b69052e-494ae4662cemr258669031cf.32.1747650401916;\n\tMon, 19 May 2025 03:26:41 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-4-naush@raspberrypi.com>","In-Reply-To":"<20250519092245.269048-4-naush@raspberrypi.com>","From":"David Plowman ","Date":"Mon, 19 May 2025 11:26:31 +0100","X-Gm-Features":"AX0GCFspwfQE4xaWIEj7ICC_cfMo27rjH9E6qua0jINvh_gGb7-EqThF1U7FdhM","Message-ID":"","Subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34281,"web_url":"https://patchwork.libcamera.org/comment/34281/","msgid":"","date":"2025-05-20T07:01:58","subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Naush\n\nOn Mon, May 19, 2025 at 10:20:51AM +0100, Naushir Patuck wrote:\n> Split the pipeline handler drop frame tracking into startup frames and\n> invalid frames, as reported by the IPA.\n>\n> Remove the drop buffer handling logic in the pipeline handler. Now all\n> image buffers are returned out with the appropriate FrameStatus set\n> for startup or invalid frames.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/rpi/common/pipeline_base.cpp | 88 +++++++------------\n> .../pipeline/rpi/common/pipeline_base.h | 5 +-\n> 2 files changed, 37 insertions(+), 56 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> index 5956485953a2..21f2daf5bab5 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> @@ -659,9 +659,9 @@ int PipelineHandlerBase::start(Camera *camera, const ControlList *controls)\n> \tif (!result.controls.empty())\n> \t\tdata->setSensorControls(result.controls);\n>\n> -\t/* Configure the number of dropped frames required on startup. */\n> -\tdata->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> -\t\t\t0 : result.startupFrameCount + result.invalidFrameCount;\n> +\t/* Configure the number of startup and invalid frames reported by the IPA. */\n> +\tdata->startupFrameCount_ = result.startupFrameCount;\n> +\tdata->invalidFrameCount_ = result.invalidFrameCount;\n>\n> \tfor (auto const stream : data->streams_)\n> \t\tstream->resetBuffers();\n> @@ -678,7 +678,6 @@ int PipelineHandlerBase::start(Camera *camera, const ControlList *controls)\n> \t\tdata->buffersAllocated_ = true;\n> \t}\n>\n> -\t/* We need to set the dropFrameCount_ before queueing buffers. */\n> \tret = queueAllBuffers(camera);\n> \tif (ret) {\n> \t\tLOG(RPI, Error) << \"Failed to queue buffers\";\n> @@ -898,23 +897,6 @@ int PipelineHandlerBase::queueAllBuffers(Camera *camera)\n\nDoes this function now \"queue all buffers\" ? Or just prepares the\ninternal buffer pools\n\n> \t\t\tret = stream->queueAllBuffers();\n> \t\t\tif (ret < 0)\n> \t\t\t\treturn ret;\n> -\t\t} else {\n\nWithout the else the above branch could be made easier to read with\n\n\tfor (auto const stream : data->streams_) {\n\t\tif (stream->getFlags() & StreamFlag::External)\n continue;\n\n int ret = stream->queueAllBuffers();\n if (ret < 0)\n return ret;\n }\n\n\n> -\t\t\t/*\n> -\t\t\t * For external streams, we must queue up a set of internal\n> -\t\t\t * buffers to handle the number of drop frames requested by\n> -\t\t\t * the IPA. This is done by passing nullptr in queueBuffer().\n> -\t\t\t *\n> -\t\t\t * The below queueBuffer() call will do nothing if there\n> -\t\t\t * are not enough internal buffers allocated, but this will\n> -\t\t\t * be handled by queuing the request for buffers in the\n> -\t\t\t * RPiStream object.\n> -\t\t\t */\n> -\t\t\tunsigned int i;\n> -\t\t\tfor (i = 0; i < data->dropFrameCount_; i++) {\n> -\t\t\t\tret = stream->queueBuffer(nullptr);\n> -\t\t\t\tif (ret)\n> -\t\t\t\t\treturn ret;\n> -\t\t\t}\n> \t\t}\n> \t}\n>\n> @@ -1412,7 +1394,15 @@ void CameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::Stream *stream)\n> \t * buffer back to the stream.\n> \t */\n> \tRequest *request = requestQueue_.empty() ? nullptr : requestQueue_.front();\n> -\tif (!dropFrameCount_ && request && request->findBuffer(stream) == buffer) {\n> +\tif (request && request->findBuffer(stream) == buffer) {\n> +\t\tFrameMetadata &md = buffer->_d()->metadata();\n> +\n> +\t\t/* Mark the non-converged and invalid frames in the metadata. */\n> +\t\tif (startupFrameCount_)\n> +\t\t\tmd.status = FrameMetadata::Status::FrameStartup;\n> +\t\tif (invalidFrameCount_)\n> +\t\t\tmd.status = FrameMetadata::Status::FrameError;\n> +\n> \t\t/*\n> \t\t * Tag the buffer as completed, returning it to the\n> \t\t * application.\n> @@ -1458,42 +1448,32 @@ void CameraData::handleState()\n>\n> void CameraData::checkRequestCompleted()\n> {\n> -\tbool requestCompleted = false;\n> -\t/*\n> -\t * If we are dropping this frame, do not touch the request, simply\n> -\t * change the state to IDLE when ready.\n> -\t */\n> -\tif (!dropFrameCount_) {\n> -\t\tRequest *request = requestQueue_.front();\n> -\t\tif (request->hasPendingBuffers())\n> -\t\t\treturn;\n> +\tRequest *request = requestQueue_.front();\n> +\tif (request->hasPendingBuffers())\n> +\t\treturn;\n>\n> -\t\t/* Must wait for metadata to be filled in before completing. */\n> -\t\tif (state_ != State::IpaComplete)\n> -\t\t\treturn;\n> +\t/* Must wait for metadata to be filled in before completing. */\n> +\tif (state_ != State::IpaComplete)\n> +\t\treturn;\n>\n> -\t\tLOG(RPI, Debug) << \"Completing request sequence: \"\n> -\t\t\t\t<< request->sequence();\n> +\tLOG(RPI, Debug) << \"Completing request sequence: \"\n> +\t\t\t<< request->sequence();\n>\n> -\t\tpipe()->completeRequest(request);\n> -\t\trequestQueue_.pop();\n> -\t\trequestCompleted = true;\n> -\t}\n> +\tpipe()->completeRequest(request);\n> +\trequestQueue_.pop();\n>\n> -\t/*\n> -\t * Make sure we have three outputs completed in the case of a dropped\n> -\t * frame.\n> -\t */\n> -\tif (state_ == State::IpaComplete &&\n> -\t ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||\n> -\t requestCompleted)) {\n> -\t\tLOG(RPI, Debug) << \"Going into Idle state\";\n> -\t\tstate_ = State::Idle;\n> -\t\tif (dropFrameCount_) {\n> -\t\t\tdropFrameCount_--;\n> -\t\t\tLOG(RPI, Debug) << \"Dropping frame at the request of the IPA (\"\n> -\t\t\t\t\t<< dropFrameCount_ << \" left)\";\n> -\t\t}\n> +\tLOG(RPI, Debug) << \"Going into Idle state\";\n> +\tstate_ = State::Idle;\n> +\n> +\tif (startupFrameCount_) {\n> +\t\tstartupFrameCount_--;\n> +\t\tLOG(RPI, Debug) << \"Decrementing startup frames to \"\n> +\t\t\t\t<< startupFrameCount_;\n> +\t}\n> +\tif (invalidFrameCount_) {\n> +\t\tinvalidFrameCount_--;\n> +\t\tLOG(RPI, Debug) << \"Decrementing invalid frames to \"\n> +\t\t\t\t<< invalidFrameCount_;\n\nAssume you have\n invalidFrameCount_ = 2;\n startupFrameCount_ = 3;\n\nYou want to return\n\nFrame# 1 2 3 4 5 6 7\nStatus Invalid Invalid Startup Startup Startup Success Success\n\nright ?\n\nIf you decrement both invalidFrameCount_ and startupFrameCount_ at the\nsame time, won't you get:\n\n\nFrame# 1 2 3 4 5 6 7\nStatus Invalid Invalid Startup Success Success Success Success\n\ninstead ?\n\n> \t}\n> }\n>\n> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> index aae0c2f35888..6023f9f9d6b3 100644\n> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> @@ -48,7 +48,7 @@ class CameraData : public Camera::Private\n> public:\n> \tCameraData(PipelineHandler *pipe)\n> \t\t: Camera::Private(pipe), state_(State::Stopped),\n> -\t\t dropFrameCount_(0), buffersAllocated_(false),\n> +\t\t startupFrameCount_(0), invalidFrameCount_(0), buffersAllocated_(false),\n> \t\t ispOutputCount_(0), ispOutputTotal_(0)\n> \t{\n> \t}\n> @@ -151,7 +151,8 @@ public:\n> \t/* Mapping of CropParams keyed by the output stream order in CameraConfiguration */\n> \tstd::map cropParams_;\n>\n> -\tunsigned int dropFrameCount_;\n> +\tunsigned int startupFrameCount_;\n> +\tunsigned int invalidFrameCount_;\n>\n> \t/*\n> \t * If set, this stores the value that represets a gain of one for\n> --\n> 2.43.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 6A057BD78E\n\tfor ;\n\tTue, 20 May 2025 07:02:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D4AE668D7D;\n\tTue, 20 May 2025 09:02:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 19F7A614DD\n\tfor ;\n\tTue, 20 May 2025 09:02:02 +0200 (CEST)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 74F5E446;\n\tTue, 20 May 2025 09:01:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Tnq+VqXQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1747724501;\n\tbh=y09QBjOF87InB+Byk19B5c4zzACfkEJ+7kvIpSM+soY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Tnq+VqXQa/+V5ZHzihgaPaT7IakWE9a1dlpoL/PJwGAsBxBzDhWChI5u4jJjqIlz+\n\tVP4iA0u0OY+/iILCB2L7rA2famsa3B5iqhJdgyZ2ch2BilCdGBHp4TyOx2wOZngFqA\n\tKvPMLZWQsEGlhrSQFpdkH/HMBikNavZ2n3XtVfbE=","Date":"Tue, 20 May 2025 09:01:58 +0200","From":"Jacopo Mondi ","To":"Naushir Patuck ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","Message-ID":"","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-4-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20250519092245.269048-4-naush@raspberrypi.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34284,"web_url":"https://patchwork.libcamera.org/comment/34284/","msgid":"","date":"2025-05-20T08:09:19","subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nThank you for the review!\n\n\nOn Mon, 19 May 2025 at 11:26, David Plowman \nwrote:\n\n> Hi Naush\n>\n> Thanks for the patch.\n>\n> On Mon, 19 May 2025 at 10:23, Naushir Patuck \n> wrote:\n> >\n> > Split the pipeline handler drop frame tracking into startup frames and\n> > invalid frames, as reported by the IPA.\n> >\n> > Remove the drop buffer handling logic in the pipeline handler. Now all\n> > image buffers are returned out with the appropriate FrameStatus set\n> > for startup or invalid frames.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/rpi/common/pipeline_base.cpp | 88 +++++++------------\n> > .../pipeline/rpi/common/pipeline_base.h | 5 +-\n> > 2 files changed, 37 insertions(+), 56 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 5956485953a2..21f2daf5bab5 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -659,9 +659,9 @@ int PipelineHandlerBase::start(Camera *camera, const\n> ControlList *controls)\n> > if (!result.controls.empty())\n> > data->setSensorControls(result.controls);\n> >\n> > - /* Configure the number of dropped frames required on startup. */\n> > - data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> > - 0 : result.startupFrameCount +\n> result.invalidFrameCount;\n> > + /* Configure the number of startup and invalid frames reported\n> by the IPA. */\n> > + data->startupFrameCount_ = result.startupFrameCount;\n> > + data->invalidFrameCount_ = result.invalidFrameCount;\n> >\n> > for (auto const stream : data->streams_)\n> > stream->resetBuffers();\n> > @@ -678,7 +678,6 @@ int PipelineHandlerBase::start(Camera *camera, const\n> ControlList *controls)\n> > data->buffersAllocated_ = true;\n> > }\n> >\n> > - /* We need to set the dropFrameCount_ before queueing buffers. */\n> > ret = queueAllBuffers(camera);\n> > if (ret) {\n> > LOG(RPI, Error) << \"Failed to queue buffers\";\n> > @@ -898,23 +897,6 @@ int PipelineHandlerBase::queueAllBuffers(Camera\n> *camera)\n> > ret = stream->queueAllBuffers();\n> > if (ret < 0)\n> > return ret;\n> > - } else {\n> > - /*\n> > - * For external streams, we must queue up a set\n> of internal\n> > - * buffers to handle the number of drop frames\n> requested by\n> > - * the IPA. This is done by passing nullptr in\n> queueBuffer().\n> > - *\n> > - * The below queueBuffer() call will do nothing\n> if there\n> > - * are not enough internal buffers allocated,\n> but this will\n> > - * be handled by queuing the request for buffers\n> in the\n> > - * RPiStream object.\n> > - */\n> > - unsigned int i;\n> > - for (i = 0; i < data->dropFrameCount_; i++) {\n> > - ret = stream->queueBuffer(nullptr);\n> > - if (ret)\n> > - return ret;\n> > - }\n> > }\n> > }\n> >\n> > @@ -1412,7 +1394,15 @@ void CameraData::handleStreamBuffer(FrameBuffer\n> *buffer, RPi::Stream *stream)\n> > * buffer back to the stream.\n> > */\n> > Request *request = requestQueue_.empty() ? nullptr :\n> requestQueue_.front();\n> > - if (!dropFrameCount_ && request && request->findBuffer(stream)\n> == buffer) {\n> > + if (request && request->findBuffer(stream) == buffer) {\n> > + FrameMetadata &md = buffer->_d()->metadata();\n> > +\n> > + /* Mark the non-converged and invalid frames in the\n> metadata. */\n> > + if (startupFrameCount_)\n> > + md.status = FrameMetadata::Status::FrameStartup;\n> > + if (invalidFrameCount_)\n> > + md.status = FrameMetadata::Status::FrameError;\n>\n> Maybe \"if (validFrameCount_) ...\" followed by \"else if\n> (startupFrameCount_) ...\" would be slightly easier to follow? I know,\n> makes no difference.\n>\n>\nSure, I can make the change for the next revision.\n\nRegards,\nNaush\n\n\n> Reviewed-by: David Plowman \n>\n> Thanks\n> David\n>\n> > +\n> > /*\n> > * Tag the buffer as completed, returning it to the\n> > * application.\n> > @@ -1458,42 +1448,32 @@ void CameraData::handleState()\n> >\n> > void CameraData::checkRequestCompleted()\n> > {\n> > - bool requestCompleted = false;\n> > - /*\n> > - * If we are dropping this frame, do not touch the request,\n> simply\n> > - * change the state to IDLE when ready.\n> > - */\n> > - if (!dropFrameCount_) {\n> > - Request *request = requestQueue_.front();\n> > - if (request->hasPendingBuffers())\n> > - return;\n> > + Request *request = requestQueue_.front();\n> > + if (request->hasPendingBuffers())\n> > + return;\n> >\n> > - /* Must wait for metadata to be filled in before\n> completing. */\n> > - if (state_ != State::IpaComplete)\n> > - return;\n> > + /* Must wait for metadata to be filled in before completing. */\n> > + if (state_ != State::IpaComplete)\n> > + return;\n> >\n> > - LOG(RPI, Debug) << \"Completing request sequence: \"\n> > - << request->sequence();\n> > + LOG(RPI, Debug) << \"Completing request sequence: \"\n> > + << request->sequence();\n> >\n> > - pipe()->completeRequest(request);\n> > - requestQueue_.pop();\n> > - requestCompleted = true;\n> > - }\n> > + pipe()->completeRequest(request);\n> > + requestQueue_.pop();\n> >\n> > - /*\n> > - * Make sure we have three outputs completed in the case of a\n> dropped\n> > - * frame.\n> > - */\n> > - if (state_ == State::IpaComplete &&\n> > - ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||\n> > - requestCompleted)) {\n> > - LOG(RPI, Debug) << \"Going into Idle state\";\n> > - state_ = State::Idle;\n> > - if (dropFrameCount_) {\n> > - dropFrameCount_--;\n> > - LOG(RPI, Debug) << \"Dropping frame at the\n> request of the IPA (\"\n> > - << dropFrameCount_ << \" left)\";\n> > - }\n> > + LOG(RPI, Debug) << \"Going into Idle state\";\n> > + state_ = State::Idle;\n> > +\n> > + if (startupFrameCount_) {\n> > + startupFrameCount_--;\n> > + LOG(RPI, Debug) << \"Decrementing startup frames to \"\n> > + << startupFrameCount_;\n> > + }\n> > + if (invalidFrameCount_) {\n> > + invalidFrameCount_--;\n> > + LOG(RPI, Debug) << \"Decrementing invalid frames to \"\n> > + << invalidFrameCount_;\n> > }\n> > }\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index aae0c2f35888..6023f9f9d6b3 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -48,7 +48,7 @@ class CameraData : public Camera::Private\n> > public:\n> > CameraData(PipelineHandler *pipe)\n> > : Camera::Private(pipe), state_(State::Stopped),\n> > - dropFrameCount_(0), buffersAllocated_(false),\n> > + startupFrameCount_(0), invalidFrameCount_(0),\n> buffersAllocated_(false),\n> > ispOutputCount_(0), ispOutputTotal_(0)\n> > {\n> > }\n> > @@ -151,7 +151,8 @@ public:\n> > /* Mapping of CropParams keyed by the output stream order in\n> CameraConfiguration */\n> > std::map cropParams_;\n> >\n> > - unsigned int dropFrameCount_;\n> > + unsigned int startupFrameCount_;\n> > + unsigned int invalidFrameCount_;\n> >\n> > /*\n> > * If set, this stores the value that represets a gain of one for\n> > --\n> > 2.43.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 490C7BD78E\n\tfor ;\n\tTue, 20 May 2025 08:09:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E838968D7E;\n\tTue, 20 May 2025 10:09:57 +0200 (CEST)","from mail-vk1-xa2a.google.com (mail-vk1-xa2a.google.com\n\t[IPv6:2607:f8b0:4864:20::a2a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DEC0C614DD\n\tfor ;\n\tTue, 20 May 2025 10:09:55 +0200 (CEST)","by mail-vk1-xa2a.google.com with SMTP id\n\t71dfb90a1353d-52dc41851e2so179458e0c.1\n\tfor ;\n\tTue, 20 May 2025 01:09:55 -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=\"l6XtOp2L\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747728594; x=1748333394;\n\tdarn=lists.libcamera.org; \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=GZJAXP5KU3riMPLL7CYEE5omtqrCTkeZmIMuIux7F3A=;\n\tb=l6XtOp2LYJ5xpAm55A8SNDSc2Gv11l0+zelpbU7b/dhzmmRL/IUXQ3cAY4fsgY6QoY\n\tUllQ2FB2TDqpJVIcBiPtEtjxhGZUN4yQzfenNxACmiNTp6xGigw4vKVCBgbNG4kfXanG\n\t3RDqNNLOEtIANRpm1wUVPIY+Do7qs67GkagiMkYRn4mw7jy5E697JshJiyI2iTF1u80o\n\trXdcRw4/MXMk3iotnJ9rIoXBf2Rmvhw5BLB/9NSczvHaFTxp4HhAWAk/7Jlh9x7xjwzR\n\tK7Tk082agWiLIbgHYVoZC9UfaVLdV5ZUqmrrqKxQYiWOqno5tLqTB3vSZpjGI91ufBkg\n\tqoMg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747728594; x=1748333394;\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=GZJAXP5KU3riMPLL7CYEE5omtqrCTkeZmIMuIux7F3A=;\n\tb=ngiTKoR3H4vzsZSrHUc/xUw4eukHq2do0b/QjOOx40V+snFubc+fttkBOlfbnUKl5x\n\tDnTAps3gSQ2J+inyhmdZbDYYhzBn4/9ovecvMWJG7RbOuwp3aWHcVo4FEtVUKnZYL35s\n\t3Ij1/ujwltsSkaUz8TD5arIKblnlEo1sGLF1xGhyoozFrIR6vFApHO46ymTaXxOkMAyh\n\tq6nHP5mHdfWrwA4wTd5ez4USrTkfBpjNCQ6xNvLA1xMUN5fZ1IaXWsHmpcaHk0O/eN+G\n\ti73oWaYUbVJVALHxvQldO1aLc9Dn58ARMSsbUrnutOFH+z8MdgJmNqDt8lBqHZKf1D4P\n\txyag==","X-Gm-Message-State":"AOJu0YwOX+uQD/MGJx9Avptdj139gtCWC+rElwwMGYvybO/2qXiH+G9y\n\t8mtpqlk/7aA2srnntd0lx4i7B6fklAvalla0FkJsv1BX2UWEBuLx6HPOTqygZbNvhCOIb2fC3a5\n\tFdZNAqRs5x86tanQQmabnYWPOYm+cKFi2GRmSbj+3/EK7hsDWZH0M","X-Gm-Gg":"ASbGnctNqaS0k76fVWgcsZkHMqBfME39r+H9eX3WDa7eFYSfONGvSCJ7MyW9+EOqGz9\n\tN/HaHauHeJo/y1+B7XKX8OfugGBa/54G/lBkRZH0nLOwKFyTGpyi0xXyrcWNWi1wms5naP5iIUe\n\tjgwSx4uA8ItSRIV8QsV2T4uFwGdPfZzUaa","X-Google-Smtp-Source":"AGHT+IEzu0j7ky8HzRD5UtwzcJ8gR02P+q7702wRi3i60GGzd/9T9WCWHbDYsoAJ7r7eD+QelUHlZMCF0DHJZfKVj84=","X-Received":"by 2002:a05:6102:5e96:b0:4dd:afa2:614c with SMTP id\n\tada2fe7eead31-4dfa6bff620mr5125992137.5.1747728594476;\n\tTue, 20 May 2025 01:09:54 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 20 May 2025 09:09:19 +0100","X-Gm-Features":"AX0GCFvVWk7UPcW-dq1UR36o-w7v2LF3qC5r_3VkwbwqC7nh8SO3DmvJSCcxalo","Message-ID":"","Subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"00000000000009bd7606358cc6f6\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":34285,"web_url":"https://patchwork.libcamera.org/comment/34285/","msgid":"","date":"2025-05-20T08:14:33","subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Jacopo,\n\nThanks for the feedback!\n\nOn Tue, 20 May 2025 at 08:02, Jacopo Mondi \nwrote:\n\n> Hi Naush\n>\n> On Mon, May 19, 2025 at 10:20:51AM +0100, Naushir Patuck wrote:\n> > Split the pipeline handler drop frame tracking into startup frames and\n> > invalid frames, as reported by the IPA.\n> >\n> > Remove the drop buffer handling logic in the pipeline handler. Now all\n> > image buffers are returned out with the appropriate FrameStatus set\n> > for startup or invalid frames.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > .../pipeline/rpi/common/pipeline_base.cpp | 88 +++++++------------\n> > .../pipeline/rpi/common/pipeline_base.h | 5 +-\n> > 2 files changed, 37 insertions(+), 56 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > index 5956485953a2..21f2daf5bab5 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp\n> > @@ -659,9 +659,9 @@ int PipelineHandlerBase::start(Camera *camera, const\n> ControlList *controls)\n> > if (!result.controls.empty())\n> > data->setSensorControls(result.controls);\n> >\n> > - /* Configure the number of dropped frames required on startup. */\n> > - data->dropFrameCount_ = data->config_.disableStartupFrameDrops ?\n> > - 0 : result.startupFrameCount +\n> result.invalidFrameCount;\n> > + /* Configure the number of startup and invalid frames reported by\n> the IPA. */\n> > + data->startupFrameCount_ = result.startupFrameCount;\n> > + data->invalidFrameCount_ = result.invalidFrameCount;\n> >\n> > for (auto const stream : data->streams_)\n> > stream->resetBuffers();\n> > @@ -678,7 +678,6 @@ int PipelineHandlerBase::start(Camera *camera, const\n> ControlList *controls)\n> > data->buffersAllocated_ = true;\n> > }\n> >\n> > - /* We need to set the dropFrameCount_ before queueing buffers. */\n> > ret = queueAllBuffers(camera);\n> > if (ret) {\n> > LOG(RPI, Error) << \"Failed to queue buffers\";\n> > @@ -898,23 +897,6 @@ int PipelineHandlerBase::queueAllBuffers(Camera\n> *camera)\n>\n> Does this function now \"queue all buffers\" ? Or just prepares the\n> internal buffer pools\n>\n\nIt does indeed queue all buffers from the internal pools. prepareBuffers()\nis a separate call.\n\n\n>\n> > ret = stream->queueAllBuffers();\n> > if (ret < 0)\n> > return ret;\n> > - } else {\n>\n> Without the else the above branch could be made easier to read with\n>\n> for (auto const stream : data->streams_) {\n> if (stream->getFlags() & StreamFlag::External)\n> continue;\n>\n> int ret = stream->queueAllBuffers();\n> if (ret < 0)\n> return ret;\n> }\n>\n>\nAck. I'll update this for the next version.\n\n\n>\n> > - /*\n> > - * For external streams, we must queue up a set of\n> internal\n> > - * buffers to handle the number of drop frames\n> requested by\n> > - * the IPA. This is done by passing nullptr in\n> queueBuffer().\n> > - *\n> > - * The below queueBuffer() call will do nothing if\n> there\n> > - * are not enough internal buffers allocated, but\n> this will\n> > - * be handled by queuing the request for buffers\n> in the\n> > - * RPiStream object.\n> > - */\n> > - unsigned int i;\n> > - for (i = 0; i < data->dropFrameCount_; i++) {\n> > - ret = stream->queueBuffer(nullptr);\n> > - if (ret)\n> > - return ret;\n> > - }\n> > }\n> > }\n> >\n> > @@ -1412,7 +1394,15 @@ void CameraData::handleStreamBuffer(FrameBuffer\n> *buffer, RPi::Stream *stream)\n> > * buffer back to the stream.\n> > */\n> > Request *request = requestQueue_.empty() ? nullptr :\n> requestQueue_.front();\n> > - if (!dropFrameCount_ && request && request->findBuffer(stream) ==\n> buffer) {\n> > + if (request && request->findBuffer(stream) == buffer) {\n> > + FrameMetadata &md = buffer->_d()->metadata();\n> > +\n> > + /* Mark the non-converged and invalid frames in the\n> metadata. */\n> > + if (startupFrameCount_)\n> > + md.status = FrameMetadata::Status::FrameStartup;\n> > + if (invalidFrameCount_)\n> > + md.status = FrameMetadata::Status::FrameError;\n> > +\n> > /*\n> > * Tag the buffer as completed, returning it to the\n> > * application.\n> > @@ -1458,42 +1448,32 @@ void CameraData::handleState()\n> >\n> > void CameraData::checkRequestCompleted()\n> > {\n> > - bool requestCompleted = false;\n> > - /*\n> > - * If we are dropping this frame, do not touch the request, simply\n> > - * change the state to IDLE when ready.\n> > - */\n> > - if (!dropFrameCount_) {\n> > - Request *request = requestQueue_.front();\n> > - if (request->hasPendingBuffers())\n> > - return;\n> > + Request *request = requestQueue_.front();\n> > + if (request->hasPendingBuffers())\n> > + return;\n> >\n> > - /* Must wait for metadata to be filled in before\n> completing. */\n> > - if (state_ != State::IpaComplete)\n> > - return;\n> > + /* Must wait for metadata to be filled in before completing. */\n> > + if (state_ != State::IpaComplete)\n> > + return;\n> >\n> > - LOG(RPI, Debug) << \"Completing request sequence: \"\n> > - << request->sequence();\n> > + LOG(RPI, Debug) << \"Completing request sequence: \"\n> > + << request->sequence();\n> >\n> > - pipe()->completeRequest(request);\n> > - requestQueue_.pop();\n> > - requestCompleted = true;\n> > - }\n> > + pipe()->completeRequest(request);\n> > + requestQueue_.pop();\n> >\n> > - /*\n> > - * Make sure we have three outputs completed in the case of a\n> dropped\n> > - * frame.\n> > - */\n> > - if (state_ == State::IpaComplete &&\n> > - ((ispOutputCount_ == ispOutputTotal_ && dropFrameCount_) ||\n> > - requestCompleted)) {\n> > - LOG(RPI, Debug) << \"Going into Idle state\";\n> > - state_ = State::Idle;\n> > - if (dropFrameCount_) {\n> > - dropFrameCount_--;\n> > - LOG(RPI, Debug) << \"Dropping frame at the request\n> of the IPA (\"\n> > - << dropFrameCount_ << \" left)\";\n> > - }\n> > + LOG(RPI, Debug) << \"Going into Idle state\";\n> > + state_ = State::Idle;\n> > +\n> > + if (startupFrameCount_) {\n> > + startupFrameCount_--;\n> > + LOG(RPI, Debug) << \"Decrementing startup frames to \"\n> > + << startupFrameCount_;\n> > + }\n> > + if (invalidFrameCount_) {\n> > + invalidFrameCount_--;\n> > + LOG(RPI, Debug) << \"Decrementing invalid frames to \"\n> > + << invalidFrameCount_;\n>\n> Assume you have\n> invalidFrameCount_ = 2;\n> startupFrameCount_ = 3;\n>\n> You want to return\n>\n> Frame# 1 2 3 4 5 6 7\n> Status Invalid Invalid Startup Startup Startup Success Success\n>\n> right ?\n>\n> If you decrement both invalidFrameCount_ and startupFrameCount_ at the\n> same time, won't you get:\n>\n>\n> Frame# 1 2 3 4 5 6 7\n> Status Invalid Invalid Startup Success Success Success Success\n>\n> instead ?\n>\n\nYou are right, this is a bug. I should decrement invalidFrameCount to 0,\nthen decrement startupFrameCount. Will fix this in the next version.\n\nRegards,\nNaush\n\n\n\n>\n> > }\n> > }\n> >\n> > diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > index aae0c2f35888..6023f9f9d6b3 100644\n> > --- a/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.h\n> > @@ -48,7 +48,7 @@ class CameraData : public Camera::Private\n> > public:\n> > CameraData(PipelineHandler *pipe)\n> > : Camera::Private(pipe), state_(State::Stopped),\n> > - dropFrameCount_(0), buffersAllocated_(false),\n> > + startupFrameCount_(0), invalidFrameCount_(0),\n> buffersAllocated_(false),\n> > ispOutputCount_(0), ispOutputTotal_(0)\n> > {\n> > }\n> > @@ -151,7 +151,8 @@ public:\n> > /* Mapping of CropParams keyed by the output stream order in\n> CameraConfiguration */\n> > std::map cropParams_;\n> >\n> > - unsigned int dropFrameCount_;\n> > + unsigned int startupFrameCount_;\n> > + unsigned int invalidFrameCount_;\n> >\n> > /*\n> > * If set, this stores the value that represets a gain of one for\n> > --\n> > 2.43.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 8ACC5C31E9\n\tfor ;\n\tTue, 20 May 2025 08:15:13 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3917868D85;\n\tTue, 20 May 2025 10:15:12 +0200 (CEST)","from mail-vk1-xa2c.google.com (mail-vk1-xa2c.google.com\n\t[IPv6:2607:f8b0:4864:20::a2c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C754A614DD\n\tfor ;\n\tTue, 20 May 2025 10:15:09 +0200 (CEST)","by mail-vk1-xa2c.google.com with SMTP id\n\t71dfb90a1353d-52db7f0779eso193641e0c.2\n\tfor ;\n\tTue, 20 May 2025 01:15:09 -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=\"S0DitIwa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google; t=1747728908; x=1748333708;\n\tdarn=lists.libcamera.org; \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=YbAM7IbaVb/AFFq/ZNBW1fJmDGBEVMCoM2uDKmWF8rc=;\n\tb=S0DitIwajjUcM3Yl8mCINdcypvA/AZGRabnozgS7A0jh//hpO3GDIZf8LswZyG7ZYC\n\t7kQ1SU4byu8zFLTL1VeH7Ufc7GxWuiQUkIuQ10ib8VwNUgwVP/5qnz7gpUfkKeoZ01ms\n\teDTIu1dUV+egB4Wn68SaB0jqVwfoU+5V9iAPVgekUJcbml0virUFAAPnOPeAvn0xNdth\n\thB8HIHnlqFnj4M+zOAlcWbeC8ssVMqvp4sq77jp2iPgN8025J6BXPP+BK+/uLmcTTffz\n\tIz7XoRGAVd0rArZIKJlk2KLIcFvGZeMWLIlIk76funzj01sp2tb7AXqrHh/O79gMd6uh\n\tlp2w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1747728908; x=1748333708;\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=YbAM7IbaVb/AFFq/ZNBW1fJmDGBEVMCoM2uDKmWF8rc=;\n\tb=SMysN9gCozr3Rnk/iRTugqXtBGU7FSh3z6JtrcH20wHaBNkurF2xnNfSUKxiLlNnMV\n\tCW7Z+laUix0Kql091uQiEFB9If4moNjT8X2/d+CkiOoJ5lc/oRazgZDwTHnHgvLFOAEh\n\tCi0hjndRqDSQvxvrHqVv7m7olC3BvVJxjRwUq/kX+3ipOQevx4lj79LwKB+KB4EXbeKD\n\t0iYbQuAYcCy1vTJ1JV9Jz7m/D6FU2JUGMlHRsid6OsftgQrqtBtWsp5BxGPO+zO+xS6m\n\tcdBUeYL9y+m7OawElhOLnXEG1I3KNcyRA9vMPF71lJVxFb9Q5stMfHbMxkSpaz87JjMk\n\tf/nw==","X-Gm-Message-State":"AOJu0YwASRu6W0IrPJ8sbQL2rCCMbzLmceWiGSofDEPAwtX/B0NxP/gy\n\tldw1ynN6poFNQS8awY1GsrrH0TXJowGzQYkGzt7eDvy5ohF5u8/G6VPXNG8tm6PhvmrePvJi9RU\n\tJBeJP6naMiK207M8fIXg4FozftxVpryZzjm3g9Nyo7j1oFg4iFc+NIyY=","X-Gm-Gg":"ASbGncvik81pwZSh/Ur1F91azi2HDpw4e9sjSFfbfUMo9dvrSOI5yfS4gxIHKtAyH3I\n\tLuPNqIV3eswow2Lz2mTtFvGPfvNyNNocO4Ey7kqtWtqm6D2Z2Pq+FFZHC6VS03L7GVhuXLahQ7j\n\tkyYmvCBNVG9B1PBLkRvYOyQ+a2r6kADNB3","X-Google-Smtp-Source":"AGHT+IEEkab1KhxIIuO9icuWZT3La9zt4D1dO26g2uH6H/Yfi1j1+vo8kuo6D9HckVwVi5SLq2DE4zzFLmR8jTn2dbA=","X-Received":"by 2002:a05:6102:1628:b0:4cd:6339:36a1 with SMTP id\n\tada2fe7eead31-4dfa692e85emr4921862137.0.1747728908522;\n\tTue, 20 May 2025 01:15:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20250519092245.269048-1-naush@raspberrypi.com>\n\t<20250519092245.269048-4-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Tue, 20 May 2025 09:14:33 +0100","X-Gm-Features":"AX0GCFv_k5BKkxunXITprGLC0fbwQCNI-eDJEP0ruHPMAa7arunQDHqKckPJNbs","Message-ID":"","Subject":"Re: [PATCH v1 3/6] pipeline: ipa: rpi: Split\n\tRPiCameraData::dropFrameCount_","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000c1b30406358cd81d\"","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]