[{"id":11429,"web_url":"https://patchwork.libcamera.org/comment/11429/","msgid":"<20200718155648.GA576734@oden.dyn.berto.se>","date":"2020-07-18T15:56:48","subject":"Re: [libcamera-devel] [PATCH v3 09/10] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","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:09 +0100, Naushir Patuck wrote:\n> The FrameBuffer cookie may be set by the application, so this cannot\n> be set by the pipeline handler as well. Revert to using a simple index\n> into the buffer list to identify buffers passing to and from the IPA.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n\nWith the understanding that this do not yet support external buffers for \nthe bayer stream.\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 60 ++++++++++---------\n>  .../pipeline/raspberrypi/rpi_stream.cpp       | 14 +++--\n>  .../pipeline/raspberrypi/rpi_stream.h         |  2 +-\n>  3 files changed, 40 insertions(+), 36 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index ce56ad1a..e400c10c 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -888,7 +888,8 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n>  int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n>  {\n>  \tRPiCameraData *data = cameraData(camera);\n> -\tint count, ret;\n> +\tunsigned int index;\n> +\tint ret;\n>  \n>  \t/*\n>  \t * Decide how many internal buffers to allocate. For now, simply look\n> @@ -908,30 +909,24 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n>  \t}\n>  \n>  \t/*\n> -\t * Add cookies to the ISP Input buffers so that we can link them with\n> -\t * the IPA and RPI_IPA_EVENT_SIGNAL_ISP_PREPARE event.\n> -\t */\n> -\tcount = 0;\n> -\tfor (auto const &b : data->unicam_[Unicam::Image].getBuffers()) {\n> -\t\tb->setCookie(count++);\n> -\t}\n> -\n> -\t/*\n> -\t * Add cookies to the stats and embedded data buffers and link them with\n> -\t * the IPA.\n> +\t * Link the FrameBuffers with the index of their position in the vector\n> +\t * stored in the RPi stream object.\n> +\t *\n> +\t * This will allow us to identify buffers passed between the pipeline\n> +\t * handler and the IPA.\n>  \t */\n> -\tcount = 0;\n> +\tindex = 0;\n>  \tfor (auto const &b : data->isp_[Isp::Stats].getBuffers()) {\n> -\t\tb->setCookie(count++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b->cookie(),\n> +\t\tdata->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,\n>  \t\t\t\t\t      .planes = b->planes() });\n> +\t\tindex++;\n>  \t}\n>  \n> -\tcount = 0;\n> +\tindex = 0;\n>  \tfor (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {\n> -\t\tb->setCookie(count++);\n> -\t\tdata->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b->cookie(),\n> +\t\tdata->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,\n>  \t\t\t\t\t      .planes = b->planes() });\n> +\t\tindex++;\n>  \t}\n>  \n>  \tdata->ipa_->mapBuffers(data->ipaBuffers_);\n> @@ -1120,7 +1115,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n>  \t\tunsigned int bufferId = action.data[0];\n>  \t\tFrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);\n>  \n> -\t\tLOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << buffer->cookie()\n> +\t\tLOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << bufferId\n>  \t\t\t\t<< \", timestamp: \" << buffer->metadata().timestamp;\n>  \n>  \t\tisp_[Isp::Input].queueBuffer(buffer);\n> @@ -1140,12 +1135,14 @@ done:\n>  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n>  {\n>  \tRPi::RPiStream *stream = nullptr;\n> +\tint index;\n>  \n>  \tif (state_ == State::Stopped)\n>  \t\treturn;\n>  \n>  \tfor (RPi::RPiStream &s : unicam_) {\n> -\t\tif (s.findFrameBuffer(buffer)) {\n> +\t\tindex = s.getBufferIndex(buffer);\n> +\t\tif (index != -1) {\n>  \t\t\tstream = &s;\n>  \t\t\tbreak;\n>  \t\t}\n> @@ -1155,7 +1152,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n>  \tASSERT(stream);\n>  \n>  \tLOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer dequeue\"\n> -\t\t\t<< \", buffer id \" << buffer->cookie()\n> +\t\t\t<< \", buffer id \" << index\n>  \t\t\t<< \", timestamp: \" << buffer->metadata().timestamp;\n>  \n>  \tif (stream == &unicam_[Unicam::Image]) {\n> @@ -1195,7 +1192,7 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n>  \t\treturn;\n>  \n>  \tLOG(RPI, Debug) << \"Stream ISP Input buffer complete\"\n> -\t\t\t<< \", buffer id \" << buffer->cookie()\n> +\t\t\t<< \", buffer id \" << unicam_[Unicam::Image].getBufferIndex(buffer)\n>  \t\t\t<< \", timestamp: \" << buffer->metadata().timestamp;\n>  \n>  \t/* The ISP input buffer gets re-queued into Unicam. */\n> @@ -1206,12 +1203,14 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n>  void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n>  {\n>  \tRPi::RPiStream *stream = nullptr;\n> +\tint index;\n>  \n>  \tif (state_ == State::Stopped)\n>  \t\treturn;\n>  \n>  \tfor (RPi::RPiStream &s : isp_) {\n> -\t\tif (s.findFrameBuffer(buffer)) {\n> +\t\tindex = s.getBufferIndex(buffer);\n> +\t\tif (index != -1) {\n>  \t\t\tstream = &s;\n>  \t\t\tbreak;\n>  \t\t}\n> @@ -1221,7 +1220,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n>  \tASSERT(stream);\n>  \n>  \tLOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer complete\"\n> -\t\t\t<< \", buffer id \" << buffer->cookie()\n> +\t\t\t<< \", buffer id \" << index\n>  \t\t\t<< \", timestamp: \" << buffer->metadata().timestamp;\n>  \n>  \t/*\n> @@ -1231,7 +1230,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n>  \tif (stream == &isp_[Isp::Stats]) {\n>  \t\tIPAOperationData op;\n>  \t\top.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;\n> -\t\top.data = { RPiIpaMask::STATS | buffer->cookie() };\n> +\t\top.data = { RPiIpaMask::STATS | static_cast<unsigned int>(index) };\n>  \t\tipa_->processEvent(op);\n>  \t} else {\n>  \t\t/* Any other ISP output can be handed back to the application now. */\n> @@ -1450,13 +1449,16 @@ void RPiCameraData::tryRunPipeline()\n>  \t/* Set our state to say the pipeline is active. */\n>  \tstate_ = State::Busy;\n>  \n> +\tunsigned int bayerIndex = unicam_[Unicam::Image].getBufferIndex(bayerBuffer);\n> +\tunsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferIndex(embeddedBuffer);\n> +\n>  \tLOG(RPI, Debug) << \"Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:\"\n> -\t\t\t<< \" Bayer buffer id: \" << bayerBuffer->cookie()\n> -\t\t\t<< \" Embedded buffer id: \" << embeddedBuffer->cookie();\n> +\t\t\t<< \" Bayer buffer id: \" << bayerIndex\n> +\t\t\t<< \" Embedded buffer id: \" << embeddedIndex;\n>  \n>  \top.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;\n> -\top.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(),\n> -\t\t    RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() };\n> +\top.data = { RPiIpaMask::EMBEDDED_DATA | embeddedIndex,\n> +\t\t    RPiIpaMask::BAYER_DATA | bayerIndex };\n>  \tipa_->processEvent(op);\n>  }\n>  \n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> index 4818117e..61226e94 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -179,15 +179,17 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)\n>  \t}\n>  }\n>  \n> -bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> +int RPiStream::getBufferIndex(FrameBuffer *buffer) const\n>  {\n>  \tif (importOnly_)\n> -\t\treturn false;\n> +\t\treturn -1;\n>  \n> -\tif (std::find(bufferList_.begin(), bufferList_.end(), buffer) != bufferList_.end())\n> -\t\treturn true;\n> +\t/* Find the buffer in the list, and return the index position. */\n> +\tauto it = std::find(bufferList_.begin(), bufferList_.end(), buffer);\n> +\tif (it != bufferList_.end())\n> +\t\treturn std::distance(bufferList_.begin(), it);\n>  \n> -\treturn false;\n> +\treturn -1;\n>  }\n>  \n>  void RPiStream::clearBuffers()\n> @@ -200,7 +202,7 @@ void RPiStream::clearBuffers()\n>  \n>  int RPiStream::queueToDevice(FrameBuffer *buffer)\n>  {\n> -\tLOG(RPISTREAM, Debug) << \"Queuing buffer \" << buffer->cookie()\n> +\tLOG(RPISTREAM, Debug) << \"Queuing buffer \" << getBufferIndex(buffer)\n>  \t\t\t      << \" for \" << name_;\n>  \n>  \tint ret = dev_->queueBuffer(buffer);\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> index 6c8dfa83..a6fd5c8e 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -47,7 +47,7 @@ public:\n>  \tint queueAllBuffers();\n>  \tint queueBuffer(FrameBuffer *buffer);\n>  \tvoid returnBuffer(FrameBuffer *buffer);\n> -\tbool findFrameBuffer(FrameBuffer *buffer) const;\n> +\tint getBufferIndex(FrameBuffer *buffer) const;\n>  \n>  private:\n>  \tvoid clearBuffers();\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 24FE1BD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSat, 18 Jul 2020 15:56:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A80F60589;\n\tSat, 18 Jul 2020 17:56:53 +0200 (CEST)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6FD3060493\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jul 2020 17:56:51 +0200 (CEST)","by mail-lj1-x241.google.com with SMTP id q7so15974652ljm.1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 18 Jul 2020 08:56:51 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tg24sm2246393ljl.139.2020.07.18.08.56.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 18 Jul 2020 08:56:48 -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=\"jmEwvUHV\"; 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=5r0u1yyGdVdmQuAiZyKMaR0vVz8TcB+f1adcVPnW77w=;\n\tb=jmEwvUHVxR+Y4ka2MLjfB4TsGpZ8HTrHJaUjG7mIcsFSf950O4o/6UC0wdnMD3RKPT\n\t7SXrfoJlNCIEo9/pGs6birfHTgDrIR9/qZZk+wV+Ikkop8nyPzg8iyVwxhqPxRrhHWrs\n\tUZxamPwRAOOSHBEXpF7E5arc6kECEEriw/lO6jW6x0I/Md6ahjw1BLlR21p/ktZOXBco\n\tQBV010R6zkoxk3NTWhaC/ZsHTIb/oyQvrBFHsXEtlZvSjx2Lz4vjJv0mA5CzZIX7x9Zr\n\t1OsEsiLbcHXpwsSoF5Etzk6D7g1086uZpxeMf6jjhuxloz5FysA+6GJq+T7bB36FD92F\n\tHD1g==","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=5r0u1yyGdVdmQuAiZyKMaR0vVz8TcB+f1adcVPnW77w=;\n\tb=GbkwApSbk9QozLz2AxwNpvH0ORqKz4ENcNCpE8iFfVi97O2mHVXu1Cyik8lq9HJq8d\n\tGoAQ5fP+4EZu10bCH0CjBqRehRm0gwOMXoPSXFU8QRg7HzCjpNsroKIJaT8y1i0TiyN1\n\tFsahYpQKY8vZ0d7Mw8vMeeHqvmVoYPLpp5gDu84Kpy6AM/VpKbcn5QAclodKMHo4tqGP\n\tiDQo5b9ZX4h2LhZ03f4lLYMzaA4sVmfe2ncBJrd3l0ekMN7j+HineIG9yhc4onG7i1NK\n\tK712gv0ndU3qa70CtExnx2KvUHK1KP79nV+iPS0q2QnhH4QBSoNfm6GA+x4/klQsv4ca\n\tqEbw==","X-Gm-Message-State":"AOAM5304WUFbS4ykbCXLlG6+Jf+iUedrW9kzrxV52YWk+QbSmrOFazWz\n\tuP4B+LtI/pvO/TYfxE5nc86LPg==","X-Google-Smtp-Source":"ABdhPJyZYgxpYeyB1Dm2o1T6Sh2NRW/0yMhAGOxU2VUcOVE5iOXSsiJzGOfRKz4NKQ9KAYDV+Wmr9A==","X-Received":"by 2002:a05:651c:2046:: with SMTP id\n\tt6mr6122161ljo.217.1595087810536; \n\tSat, 18 Jul 2020 08:56:50 -0700 (PDT)","Date":"Sat, 18 Jul 2020 17:56:48 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20200718155648.GA576734@oden.dyn.berto.se>","References":"<20200717085410.732308-1-naush@raspberrypi.com>\n\t<20200717085410.732308-10-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200717085410.732308-10-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 09/10] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","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>"}}]