[{"id":24321,"web_url":"https://patchwork.libcamera.org/comment/24321/","msgid":"","date":"2022-08-03T08:44:17","subject":"Re: [libcamera-devel] [PATCH v4 9/9] ipu3: Fixes frame delay","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Sorry I forgot to include the comment that explains how |stillCount| is\ncalculated and what it does.\nTo avoid the spam, I'll put the comment here and upload it with PATCH v5:\n\n\n/*\n\n* If the StillCapture request is the first request or isn't\n\n* consequential, we need to requeue the same buffers to ImgU1 to\n\n* mitigate the frame delay (i.e. |stillCount| being 2). Otherwise,\n\n* |stillCount| would be 1, which means the buffers are queued once as\n\n* usual.\n\n*/\n\n\n\nOn Tue, Aug 2, 2022 at 6:30 PM Harvey Yang \nwrote:\n\n> Like the shipped ipu3 HAL [1], handle the frame delay with one extra input,\n> needed for the first frame and StillCapture's non-sequential frame\n> requests.\n>\n> [1]:\n>\n> https://source.corp.google.com/chromeos_public/src/platform/camera/hal/intel/ipu3/psl/ipu3/ImguUnit.cpp;l=773\n>\n> Signed-off-by: Harvey Yang \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 73 +++++++++++++++++++++++-----\n> 1 file changed, 62 insertions(+), 11 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index f8101c6c..36ca47c2 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -90,6 +90,9 @@ public:\n>\n> ControlInfoMap ipaControls_;\n>\n> + bool firstRequest_ = true;\n> + int lastCaptureRequestSeq_ = -1;\n> +\n> private:\n> void metadataReady(unsigned int id, const ControlList &metadata);\n> void paramsBufferReady(unsigned int id);\n> @@ -884,6 +887,9 @@ int PipelineHandlerIPU3::start(Camera *camera,\n> [[maybe_unused]] const ControlLis\n> CIO2Device *cio2 = &data->cio2_;\n> int ret;\n>\n> + data->firstRequest_ = true;\n> + data->lastCaptureRequestSeq_ = -1;\n> +\n> imgu0_.input_->bufferReady.connect(data,\n>\n> &IPU3CameraData::tryReturnBuffer);\n> imgu0_.output_->bufferReady.connect(data,\n> @@ -1422,6 +1428,11 @@ void IPU3CameraData::paramsBufferReady(unsigned int\n> id)\n> if (!info)\n> return;\n>\n> + int yuvCount = firstRequest_ ? 2 : 1;\n> + int stillCount = firstRequest_ || (lastCaptureRequestSeq_ + 1) !=\n> static_cast(info->request->sequence()) ? 2 : 1;\n> +\n> + firstRequest_ = false;\n> +\n> bool hasYuv = false;\n> bool hasCapture = false;\n> /* Queue all buffers from the request aimed for the ImgU. */\n> @@ -1431,33 +1442,53 @@ void IPU3CameraData::paramsBufferReady(unsigned\n> int id)\n>\n> if (stream == &outStream_) {\n> hasYuv = true;\n> - imgu0_->output_->queueBuffer(outbuffer);\n> +\n> + for (int i = 0; i < yuvCount; ++i) {\n> + bufferReturnCounters[outbuffer] += 1;\n> + imgu0_->output_->queueBuffer(outbuffer);\n> + }\n> } else if (stream == &vfStream_) {\n> hasYuv = true;\n> - imgu0_->viewfinder_->queueBuffer(outbuffer);\n> +\n> + for (int i = 0; i < yuvCount; ++i) {\n> + bufferReturnCounters[outbuffer] += 1;\n> +\n> imgu0_->viewfinder_->queueBuffer(outbuffer);\n> + }\n> } else if (stream == &outCaptureStream_) {\n> hasCapture = true;\n>\n> - imgu1_->output_->queueBuffer(outbuffer);\n> + for (int i = 0; i < stillCount; ++i) {\n> + bufferReturnCounters[outbuffer] += 1;\n> + imgu1_->output_->queueBuffer(outbuffer);\n> + }\n> }\n> }\n>\n> if (hasYuv) {\n> - bufferReturnCounters[info->rawBuffer] += 1;\n> -\n> - imgu0_->param_->queueBuffer(info->paramBuffer);\n> - imgu0_->stat_->queueBuffer(info->statBuffer);\n> - imgu0_->input_->queueBuffer(info->rawBuffer);\n> + for (int i = 0; i < yuvCount; ++i) {\n> + bufferReturnCounters[info->paramBuffer] += 1;\n> + bufferReturnCounters[info->statBuffer] += 1;\n> + bufferReturnCounters[info->rawBuffer] += 1;\n> +\n> + imgu0_->param_->queueBuffer(info->paramBuffer);\n> + imgu0_->stat_->queueBuffer(info->statBuffer);\n> + imgu0_->input_->queueBuffer(info->rawBuffer);\n> + }\n> } else {\n> info->paramDequeued = true;\n> info->metadataProcessed = true;\n> }\n>\n> if (hasCapture) {\n> - bufferReturnCounters[info->rawBuffer] += 1;\n> + lastCaptureRequestSeq_ = info->request->sequence();\n>\n> - imgu1_->param_->queueBuffer(info->captureParamBuffer);\n> - imgu1_->input_->queueBuffer(info->rawBuffer);\n> + for (int i = 0; i < stillCount; ++i) {\n> + bufferReturnCounters[info->captureParamBuffer] +=\n> 1;\n> + bufferReturnCounters[info->rawBuffer] += 1;\n> +\n> +\n> imgu1_->param_->queueBuffer(info->captureParamBuffer);\n> + imgu1_->input_->queueBuffer(info->rawBuffer);\n> + }\n> } else {\n> info->captureParamDequeued = true;\n> }\n> @@ -1498,6 +1529,11 @@ void IPU3CameraData::tryReturnBuffer(FrameBuffer\n> *buffer)\n> */\n> void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> {\n> + if (--bufferReturnCounters[buffer] > 0)\n> + return;\n> +\n> + bufferReturnCounters.erase(buffer);\n> +\n> IPU3Frames::Info *info = frameInfos_.find(buffer);\n> if (!info)\n> return;\n> @@ -1564,6 +1600,11 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer\n> *buffer)\n>\n> void IPU3CameraData::paramBufferReady(FrameBuffer *buffer)\n> {\n> + if (--bufferReturnCounters[buffer] > 0)\n> + return;\n> +\n> + bufferReturnCounters.erase(buffer);\n> +\n> IPU3Frames::Info *info = frameInfos_.find(buffer);\n> if (!info)\n> return;\n> @@ -1584,6 +1625,11 @@ void IPU3CameraData::paramBufferReady(FrameBuffer\n> *buffer)\n>\n> void IPU3CameraData::captureParamBufferReady(FrameBuffer *buffer)\n> {\n> + if (--bufferReturnCounters[buffer] > 0)\n> + return;\n> +\n> + bufferReturnCounters.erase(buffer);\n> +\n> IPU3Frames::Info *info = frameInfos_.find(buffer);\n> if (!info)\n> return;\n> @@ -1604,6 +1650,11 @@ void\n> IPU3CameraData::captureParamBufferReady(FrameBuffer *buffer)\n>\n> void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> {\n> + if (--bufferReturnCounters[buffer] > 0)\n> + return;\n> +\n> + bufferReturnCounters.erase(buffer);\n> +\n> IPU3Frames::Info *info = frameInfos_.find(buffer);\n> if (!info)\n> return;\n> --\n> 2.37.1.455.g008518b4e5-goog\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 44A7DBE173\n\tfor ;\n\tWed, 3 Aug 2022 08:44:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA4C163315;\n\tWed, 3 Aug 2022 10:44:31 +0200 (CEST)","from mail-lj1-x231.google.com (mail-lj1-x231.google.com\n\t[IPv6:2a00:1450:4864:20::231])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D438C603EF\n\tfor ;\n\tWed, 3 Aug 2022 10:44:29 +0200 (CEST)","by mail-lj1-x231.google.com with SMTP id e11so18202246ljl.4\n\tfor ;\n\tWed, 03 Aug 2022 01:44:29 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1659516271;\n\tbh=jwTRFX3R90+96yfvLIE3X41Dknq3UoivN2PsbUFds80=;\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=kqf0bDczw8B8IXb+tui93vUK8vbPVucX/kYpZKsxiEXVjBPS/iGrhC7mny7WeHlNO\n\tpAM7XjHYdTM9sAJTHmYaSZ8nuLetQ1GwHLDPuknZR3/fnzViCFc7sQvIlxC7cFEPgL\n\tWiLNT98ly0SMNDpbxLZYzdIRgkk2kCcbrs6iiKudQEtGpydr+BcuqyW6g/7u3HQzbq\n\tULnSeHRvCY5qYKwpwxjwviqM0wDFsI+kD0tz6TshsCQYqTUta8DuFLDmoQAdK5awV4\n\tFiNy6th8uR+Bj5lH3eTub/O3mMERO4P09ERwY2P+H+CEuRvl7ukkvzS7hpkPEeYX8B\n\teW4Wa6upBAdJQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=gAAUsmNsiA1CMPTE/hriz3SVQI923yroelYTQBSXJtI=;\n\tb=enXU4M1Jclxig0upph8nCto/XToYXdsEIUCvWuyXvgPXz9s2vv9rDg+PRBvMFSJxeZ\n\tedeMrzJOR84weJzjCOGlkIYWwsgFAfYHSc3Lsu1eLxkeHpnrkUyJqMTgqiEWsgGOVrYO\n\tkUzGaZerLtQwfWYU5e7hN3cOhB+s4aJjWvcFQ="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"enXU4M1J\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=gAAUsmNsiA1CMPTE/hriz3SVQI923yroelYTQBSXJtI=;\n\tb=v1wCyERXQoWQmSAVzpbbWa8+gp3YCCrL/432TLSHuhVMYMtN2bzSMwwJW+F+tN5NPQ\n\tI0topxlFMnFmXSGetiUaqgPuiYVSKR4dB6k6/QJ3Pp1Z7Wv3J0/umtdHp+kv0xUP/0jR\n\trGTvJNWzPo/NmQy17a4g4FSB8bo0f8PDO9tq0bAhYVh3hXsiUcNGlSksS8gjPo6OeZM9\n\tcNG5kQIEYHPWP4KbNOglHieEiwWYoutnN0mbW5yDEmwHoOx5FIaaygIWhjG95lN68WLM\n\to4URn5XV3bqI6/NgtjR/vBVX066XJyXmLibnAJA9aUkD6qV3NHl2MkJwZjmlU3VAVY4s\n\t84DA==","X-Gm-Message-State":"AJIora9m9o+RyYD/7k9nbYem8lTwDX0IuiGbCBQdotRoiyjmNy5bApaj\n\ti5alNPxAloncp4Dt6K8UvdiG6zKbHd2qtC4hDBSEvQGpRydJJA==","X-Google-Smtp-Source":"AGRyM1uU+XlTAb2EupOSjyDWbMNNHtOri4k+zoF/T19dUeBaPW/mKokrUL8+O3mh+1DpzaxJjZD1CvAc/j2ZwDWWfWk=","X-Received":"by 2002:a2e:92c8:0:b0:25d:6ddf:e71d with SMTP id\n\tk8-20020a2e92c8000000b0025d6ddfe71dmr7544851ljh.170.1659516268716;\n\tWed, 03 Aug 2022 01:44:28 -0700 (PDT)","MIME-Version":"1.0","References":"<20220802102943.3221109-1-chenghaoyang@google.com>\n\t<20220802102943.3221109-10-chenghaoyang@google.com>","In-Reply-To":"<20220802102943.3221109-10-chenghaoyang@google.com>","Date":"Wed, 3 Aug 2022 16:44:17 +0800","Message-ID":"","To":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/alternative; boundary=\"000000000000b23c0505e5523def\"","Subject":"Re: [libcamera-devel] [PATCH v4 9/9] ipu3: Fixes frame delay","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":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"Harvey Yang ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]