[{"id":11425,"web_url":"https://patchwork.libcamera.org/comment/11425/","msgid":"<20200718150351.GB555842@oden.dyn.berto.se>","date":"2020-07-18T15:03:51","subject":"Re: [libcamera-devel] [PATCH v3 02/10] libcamera: pipeline: ipa:\n\traspberrypi: Rework drop frame signalling","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Naushir,\n\nThanks for your work.\n\nOn 2020-07-17 09:54:02 +0100, Naushir Patuck wrote:\n> The IPA now signals up front how many frames it wants the pipeline\n> handler to drop. This makes it easier to handle up-coming changes to the\n> buffer handling for import/export buffers.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  include/libcamera/ipa/raspberrypi.h           |  2 +-\n>  src/ipa/raspberrypi/raspberrypi.cpp           | 20 +++++------\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 36 +++++++++++--------\n>  3 files changed, 33 insertions(+), 25 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\n> index a4937769..0f31b6ea 100644\n> --- a/include/libcamera/ipa/raspberrypi.h\n> +++ b/include/libcamera/ipa/raspberrypi.h\n> @@ -14,6 +14,7 @@ enum RPiConfigParameters {\n>  \tRPI_IPA_CONFIG_LS_TABLE = (1 << 0),\n>  \tRPI_IPA_CONFIG_STAGGERED_WRITE = (1 << 1),\n>  \tRPI_IPA_CONFIG_SENSOR = (1 << 2),\n> +\tRPI_IPA_CONFIG_DROP_FRAMES = (1 << 3),\n>  };\n>  \n>  enum RPiOperations {\n> @@ -21,7 +22,6 @@ enum RPiOperations {\n>  \tRPI_IPA_ACTION_V4L2_SET_ISP,\n>  \tRPI_IPA_ACTION_STATS_METADATA_COMPLETE,\n>  \tRPI_IPA_ACTION_RUN_ISP,\n> -\tRPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME,\n>  \tRPI_IPA_ACTION_EMBEDDED_COMPLETE,\n>  \tRPI_IPA_EVENT_SIGNAL_STAT_READY,\n>  \tRPI_IPA_EVENT_SIGNAL_ISP_PREPARE,\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 2fcbc782..a574fba9 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -64,8 +64,8 @@ class IPARPi : public IPAInterface\n>  public:\n>  \tIPARPi()\n>  \t\t: lastMode_({}), controller_(), controllerInit_(false),\n> -\t\t  frame_count_(0), check_count_(0), hide_count_(0),\n> -\t\t  mistrust_count_(0), lsTableHandle_(0), lsTable_(nullptr)\n> +\t\t  frame_count_(0), check_count_(0), mistrust_count_(0),\n> +\t\t  lsTableHandle_(0), lsTable_(nullptr)\n>  \t{\n>  \t}\n>  \n> @@ -134,8 +134,6 @@ private:\n>  \tuint64_t frame_count_;\n>  \t/* For checking the sequencing of Prepare/Process calls. */\n>  \tuint64_t check_count_;\n> -\t/* How many frames the pipeline handler should hide, or \"drop\". */\n> -\tunsigned int hide_count_;\n>  \t/* How many frames we should avoid running control algos on. */\n>  \tunsigned int mistrust_count_;\n>  \t/* LS table allocation passed in from the pipeline handler. */\n> @@ -239,14 +237,18 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n>  \t */\n>  \tframe_count_ = 0;\n>  \tcheck_count_ = 0;\n> +\tint drop_frame = 0;\n>  \tif (controllerInit_) {\n> -\t\thide_count_ = helper_->HideFramesModeSwitch();\n> +\t\tdrop_frame = helper_->HideFramesModeSwitch();\n>  \t\tmistrust_count_ = helper_->MistrustFramesModeSwitch();\n>  \t} else {\n> -\t\thide_count_ = helper_->HideFramesStartup();\n> +\t\tdrop_frame = helper_->HideFramesStartup();\n>  \t\tmistrust_count_ = helper_->MistrustFramesStartup();\n>  \t}\n>  \n> +\tresult->data.push_back(drop_frame);\n> +\tresult->operation |= RPI_IPA_CONFIG_DROP_FRAMES;\n> +\n>  \tstruct AgcStatus agcStatus;\n>  \t/* These zero values mean not program anything (unless overwritten). */\n>  \tagcStatus.shutter_time = 0.0;\n> @@ -348,13 +350,11 @@ void IPARPi::processEvent(const IPAOperationData &event)\n>  \t\t * they are \"unreliable\".\n>  \t\t */\n>  \t\tprepareISP(embeddedbufferId);\n> +\t\tframe_count_++;\n>  \n>  \t\t/* Ready to push the input buffer into the ISP. */\n>  \t\tIPAOperationData op;\n> -\t\tif (++frame_count_ > hide_count_)\n> -\t\t\top.operation = RPI_IPA_ACTION_RUN_ISP;\n> -\t\telse\n> -\t\t\top.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;\n> +\t\top.operation = RPI_IPA_ACTION_RUN_ISP;\n>  \t\top.data = { bayerbufferId & RPiIpaMask::ID };\n>  \t\tqueueFrameAction.emit(0, op);\n>  \t\tbreak;\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 09514697..3884d93d 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -132,7 +132,7 @@ class RPiCameraData : public CameraData\n>  public:\n>  \tRPiCameraData(PipelineHandler *pipe)\n>  \t\t: CameraData(pipe), sensor_(nullptr), lsTable_(nullptr),\n> -\t\t  state_(State::Stopped), dropFrame_(false), ispOutputCount_(0)\n> +\t\t  state_(State::Stopped), dropFrameCount_(0), ispOutputCount_(0)\n>  \t{\n>  \t}\n>  \n> @@ -193,14 +193,15 @@ public:\n>  \tstd::queue<FrameBuffer *> embeddedQueue_;\n>  \tstd::deque<Request *> requestQueue_;\n>  \n> +\tunsigned int dropFrameCount_;\n> +\n>  private:\n>  \tvoid checkRequestCompleted();\n>  \tvoid tryRunPipeline();\n>  \tvoid tryFlushQueues();\n>  \tFrameBuffer *updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp, V4L2VideoDevice *dev);\n>  \n> -\tbool dropFrame_;\n> -\tint ispOutputCount_;\n> +\tunsigned int ispOutputCount_;\n>  };\n>  \n>  class RPiCameraConfiguration : public CameraConfiguration\n> @@ -1022,6 +1023,7 @@ int RPiCameraData::configureIPA()\n>  \tipa_->configure(sensorInfo, streamConfig, entityControls, ipaConfig,\n>  \t\t\t&result);\n>  \n> +\tunsigned int resultIdx = 0;\n>  \tif (result.operation & RPI_IPA_CONFIG_STAGGERED_WRITE) {\n>  \t\t/*\n>  \t\t * Setup our staggered control writer with the sensor default\n> @@ -1029,9 +1031,9 @@ int RPiCameraData::configureIPA()\n>  \t\t */\n>  \t\tif (!staggeredCtrl_) {\n>  \t\t\tstaggeredCtrl_.init(unicam_[Unicam::Image].dev(),\n> -\t\t\t\t\t    { { V4L2_CID_ANALOGUE_GAIN, result.data[0] },\n> -\t\t\t\t\t      { V4L2_CID_EXPOSURE, result.data[1] } });\n> -\t\t\tsensorMetadata_ = result.data[2];\n> +\t\t\t\t\t    { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },\n> +\t\t\t\t\t      { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });\n> +\t\t\tsensorMetadata_ = result.data[resultIdx++];\n>  \t\t}\n>  \n>  \t\t/* Configure the H/V flip controls based on the sensor rotation. */\n> @@ -1048,6 +1050,11 @@ int RPiCameraData::configureIPA()\n>  \t\t\tLOG(RPI, Error) << \"V4L2 staggered set failed\";\n>  \t}\n>  \n> +\tif (result.operation & RPI_IPA_CONFIG_DROP_FRAMES) {\n> +\t\t/* Configure the number of dropped frames required on startup. */\n> +\t\tdropFrameCount_ = result.data[resultIdx++];\n> +\t}\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -1098,7 +1105,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n>  \t\tbreak;\n>  \t}\n>  \n> -\tcase RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME:\n>  \tcase RPI_IPA_ACTION_RUN_ISP: {\n>  \t\tunsigned int bufferId = action.data[0];\n>  \t\tFrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get();\n> @@ -1107,7 +1113,6 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n>  \t\t\t\t<< \", timestamp: \" << buffer->metadata().timestamp;\n>  \n>  \t\tisp_[Isp::Input].dev()->queueBuffer(buffer);\n> -\t\tdropFrame_ = (action.operation == RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME) ? true : false;\n>  \t\tispOutputCount_ = 0;\n>  \t\tbreak;\n>  \t}\n> @@ -1273,7 +1278,7 @@ void RPiCameraData::clearIncompleteRequests()\n>  void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream *stream)\n>  {\n>  \tif (stream->isExternal()) {\n> -\t\tif (!dropFrame_) {\n> +\t\tif (!dropFrameCount_) {\n>  \t\t\tRequest *request = buffer->request();\n>  \t\t\tpipe_->completeBuffer(camera_, request, buffer);\n>  \t\t}\n> @@ -1285,7 +1290,7 @@ void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, const RPi::RPiStream\n>  \t\t * simply memcpy to the Request buffer and requeue back to the\n>  \t\t * device.\n>  \t\t */\n> -\t\tif (stream == &unicam_[Unicam::Image] && !dropFrame_) {\n> +\t\tif (stream == &unicam_[Unicam::Image] && !dropFrameCount_) {\n>  \t\t\tconst Stream *rawStream = static_cast<const Stream *>(&isp_[Isp::Input]);\n>  \t\t\tRequest *request = requestQueue_.front();\n>  \t\t\tFrameBuffer *raw = request->findBuffer(const_cast<Stream *>(rawStream));\n> @@ -1330,7 +1335,7 @@ void RPiCameraData::checkRequestCompleted()\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 (!dropFrame_) {\n> +\tif (!dropFrameCount_) {\n>  \t\tRequest *request = requestQueue_.front();\n>  \t\tif (request->hasPendingBuffers())\n>  \t\t\treturn;\n> @@ -1349,10 +1354,13 @@ void RPiCameraData::checkRequestCompleted()\n>  \t * frame.\n>  \t */\n>  \tif (state_ == State::IpaComplete &&\n> -\t    ((ispOutputCount_ == 3 && dropFrame_) || requestCompleted)) {\n> +\t    ((ispOutputCount_ == 3 && dropFrameCount_) || requestCompleted)) {\n>  \t\tstate_ = State::Idle;\n> -\t\tif (dropFrame_)\n> -\t\t\tLOG(RPI, Info) << \"Dropping frame at the request of the IPA\";\n> +\t\tif (dropFrameCount_) {\n> +\t\t\tdropFrameCount_--;\n> +\t\t\tLOG(RPI, Info) << \"Dropping frame at the request of the IPA (\"\n> +\t\t\t\t       << dropFrameCount_ << \" left)\";\n> +\t\t}\n>  \t}\n>  }\n>  \n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 4DA14BD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 18 Jul 2020 15:03:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C482960589;\n\tSat, 18 Jul 2020 17:03:54 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6E6FC60493\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jul 2020 17:03:53 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id h22so15849717lji.9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jul 2020 08:03:53 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tv25sm2228005ljg.95.2020.07.18.08.03.51\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 18 Jul 2020 08:03:51 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"q7oOem/X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=yPFUdn/HS9TGHkBY5rSZDFNSzjv7amCIpMYiW1ZBjek=;\n\tb=q7oOem/XP55Cb0nMGsNRC0pmEEbgxz+IfpDr6WTv2uDOVY4h9REf4hbsD/mM7po+6f\n\tGTbxaStY3Bmq44oxemjt/0/CUw4OvGiRxPDm99qTcJxpmSk5SABTQkQ55skSas2JBFAJ\n\tuIFA/NkSVtWriMk3bf3CkJsXZZAKsfC4enAy4+8Y91at4f0pHFuajAMVD10i17wjyVZL\n\tcssEXjk8L8UqpaI3eu7I2XN5ixFhfSsDKADyzkbbBj/NZ710FtFRpDQY+VAzLPCAVCxP\n\twy3ZNu2flxENDDh6yrL6B0OgwbopvobE9Jzx57QOOh0pr8mMZy+BjJTMHDGQV47pfYuF\n\tkaUw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=yPFUdn/HS9TGHkBY5rSZDFNSzjv7amCIpMYiW1ZBjek=;\n\tb=sZIc+OlJR2zs7nhedn3CTmsq33IqK6rjR6IFh6/VxvzZ46QLVgpOEz4MDIDZhYZCW7\n\t5NurFXJECBXhb7JCUJ4aN1S6EJxJp18+8qHzJfqGReDHHy5krGnZbKqJ2Y1IuoW+lTzO\n\tMgekQsjvSBkQnqK8iTLuNtAKwaQ3pKQkpHoH+FLoZg0TThsqBCtFtvJ3gtgOx8SLdTt4\n\tphPL0bF73r5PFkgiCnajwOMy9A9HMZpjDkHcg9MSi+P7EpLAqJ/p9SUIWqa1B1RvjOyS\n\tDwlaOlbYjbreMHlileIYBiitM/WfpPIuwINAdDQvWsplX5NPEqttYey9jzOmUyApNmza\n\ttf1A==","X-Gm-Message-State":"AOAM530d/jSCWMmVV7qCYwVySRX7e0VoGErfNfLA7Y8uHfoPc9CdTmiQ\n\twK5cOyY6SKGOccbd9TtgblEjtuKf+y7EVg==","X-Google-Smtp-Source":"ABdhPJwUOpi8jUSfbcfpjyOeS6StCprHdurjYhYG5SsmQmPcFIIHvSxWnfAFYHMgJMxMVEMdJnjkxQ==","X-Received":"by 2002:a05:651c:50d:: with SMTP id\n\to13mr7113954ljp.181.1595084632679; \n\tSat, 18 Jul 2020 08:03:52 -0700 (PDT)","Date":"Sat, 18 Jul 2020 17:03:51 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20200718150351.GB555842@oden.dyn.berto.se>","References":"<20200717085410.732308-1-naush@raspberrypi.com>\n\t<20200717085410.732308-3-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200717085410.732308-3-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 02/10] libcamera: pipeline: ipa:\n\traspberrypi: Rework drop frame signalling","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]