[{"id":11455,"web_url":"https://patchwork.libcamera.org/comment/11455/","msgid":"<ba0b54a0-9d58-0fae-96f8-77633e44ad54@ideasonboard.com>","date":"2020-07-21T11:27:51","subject":"Re: [libcamera-devel] [PATCH v4 9/9] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Naush,\n\nOn 20/07/2020 10:13, 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> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nI wonder if this means we should add an internal cookie or ability to\nidentify the IPA buffers in the buffer class itself.\n\nBut still, this looks like it works and fixes the issue.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\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 c28fe997..e4fc5ac7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -876,7 +876,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> @@ -896,30 +897,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> @@ -1097,7 +1092,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> @@ -1117,12 +1112,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> @@ -1132,7 +1129,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> @@ -1172,7 +1169,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> @@ -1183,12 +1180,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> @@ -1198,7 +1197,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> @@ -1208,7 +1207,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> @@ -1427,13 +1426,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 6635a751..73eb04a1 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -178,15 +178,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> @@ -199,7 +201,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 6222c867..45cf5237 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -48,7 +48,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>","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 75847BD792\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 21 Jul 2020 11:28:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4161A6088C;\n\tTue, 21 Jul 2020 13:28:02 +0200 (CEST)","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 C47B060496\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 Jul 2020 13:28:00 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 07C46585;\n\tTue, 21 Jul 2020 13:27:53 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"KbIH5xUB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595330874;\n\tbh=rfQz+oujE0JSzeU0z+GeA/1rksraf9T75Y8pG9sKbUA=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=KbIH5xUBbTr68ojBuD66/DFeS2pHKWMYQ8sW50BR8TGaCuDykYh3HqnRyGC/c7vIc\n\tE4xw0v6bRMOtmysh25joGkpf1w+KLPY6K7Z82QQ7WmgZ+7T3vkjnmTzFJZUUcc1JQY\n\tY9R9wVBdAIb+v08DIZSkdHTRuDDKIxA/uEmVLH0s=","To":"Naushir Patuck <naush@raspberrypi.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-10-naush@raspberrypi.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<ba0b54a0-9d58-0fae-96f8-77633e44ad54@ideasonboard.com>","Date":"Tue, 21 Jul 2020 12:27:51 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20200720091311.805092-10-naush@raspberrypi.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v4 9/9] 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>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11463,"web_url":"https://patchwork.libcamera.org/comment/11463/","msgid":"<CAEmqJPp0CLM5HA-zYKaGqMXC7Riyw1uw-VBBrrGRAayt13yO2Q@mail.gmail.com>","date":"2020-07-21T12:38:00","subject":"Re: [libcamera-devel] [PATCH v4 9/9] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"HI Kieran,\n\nOn Tue, 21 Jul 2020 at 12:28, Kieran Bingham\n<kieran.bingham@ideasonboard.com> wrote:\n>\n> Hi Naush,\n>\n> On 20/07/2020 10:13, 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> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n>\n> I wonder if this means we should add an internal cookie or ability to\n> identify the IPA buffers in the buffer class itself.\n>\n> But still, this looks like it works and fixes the issue.\n\nI did think cookies were for internal use only :-)  However, given the\nsimplicity of using a buffer index, an internal-only cookie probably\nnot needed.\n\nRegards,\nNaush\n\n\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\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 c28fe997..e4fc5ac7 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -876,7 +876,8 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> >  int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >  {\n> >       RPiCameraData *data = cameraData(camera);\n> > -     int count, ret;\n> > +     unsigned int index;\n> > +     int ret;\n> >\n> >       /*\n> >        * Decide how many internal buffers to allocate. For now, simply look\n> > @@ -896,30 +897,24 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >       }\n> >\n> >       /*\n> > -      * Add cookies to the ISP Input buffers so that we can link them with\n> > -      * the IPA and RPI_IPA_EVENT_SIGNAL_ISP_PREPARE event.\n> > -      */\n> > -     count = 0;\n> > -     for (auto const &b : data->unicam_[Unicam::Image].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -     }\n> > -\n> > -     /*\n> > -      * Add cookies to the stats and embedded data buffers and link them with\n> > -      * the IPA.\n> > +      * Link the FrameBuffers with the index of their position in the vector\n> > +      * stored in the RPi stream object.\n> > +      *\n> > +      * This will allow us to identify buffers passed between the pipeline\n> > +      * handler and the IPA.\n> >        */\n> > -     count = 0;\n> > +     index = 0;\n> >       for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -             data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b->cookie(),\n> > +             data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,\n> >                                             .planes = b->planes() });\n> > +             index++;\n> >       }\n> >\n> > -     count = 0;\n> > +     index = 0;\n> >       for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -             data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b->cookie(),\n> > +             data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,\n> >                                             .planes = b->planes() });\n> > +             index++;\n> >       }\n> >\n> >       data->ipa_->mapBuffers(data->ipaBuffers_);\n> > @@ -1097,7 +1092,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> >               unsigned int bufferId = action.data[0];\n> >               FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);\n> >\n> > -             LOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << buffer->cookie()\n> > +             LOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << bufferId\n> >                               << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >               isp_[Isp::Input].queueBuffer(buffer);\n> > @@ -1117,12 +1112,14 @@ done:\n> >  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >  {\n> >       RPi::RPiStream *stream = nullptr;\n> > +     int index;\n> >\n> >       if (state_ == State::Stopped)\n> >               return;\n> >\n> >       for (RPi::RPiStream &s : unicam_) {\n> > -             if (s.findFrameBuffer(buffer)) {\n> > +             index = s.getBufferIndex(buffer);\n> > +             if (index != -1) {\n> >                       stream = &s;\n> >                       break;\n> >               }\n> > @@ -1132,7 +1129,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >       ASSERT(stream);\n> >\n> >       LOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer dequeue\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << index\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       if (stream == &unicam_[Unicam::Image]) {\n> > @@ -1172,7 +1169,7 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >               return;\n> >\n> >       LOG(RPI, Debug) << \"Stream ISP Input buffer complete\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << unicam_[Unicam::Image].getBufferIndex(buffer)\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       /* The ISP input buffer gets re-queued into Unicam. */\n> > @@ -1183,12 +1180,14 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >  void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >  {\n> >       RPi::RPiStream *stream = nullptr;\n> > +     int index;\n> >\n> >       if (state_ == State::Stopped)\n> >               return;\n> >\n> >       for (RPi::RPiStream &s : isp_) {\n> > -             if (s.findFrameBuffer(buffer)) {\n> > +             index = s.getBufferIndex(buffer);\n> > +             if (index != -1) {\n> >                       stream = &s;\n> >                       break;\n> >               }\n> > @@ -1198,7 +1197,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >       ASSERT(stream);\n> >\n> >       LOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer complete\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << index\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       /*\n> > @@ -1208,7 +1207,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >       if (stream == &isp_[Isp::Stats]) {\n> >               IPAOperationData op;\n> >               op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;\n> > -             op.data = { RPiIpaMask::STATS | buffer->cookie() };\n> > +             op.data = { RPiIpaMask::STATS | static_cast<unsigned int>(index) };\n> >               ipa_->processEvent(op);\n> >       } else {\n> >               /* Any other ISP output can be handed back to the application now. */\n> > @@ -1427,13 +1426,16 @@ void RPiCameraData::tryRunPipeline()\n> >       /* Set our state to say the pipeline is active. */\n> >       state_ = State::Busy;\n> >\n> > +     unsigned int bayerIndex = unicam_[Unicam::Image].getBufferIndex(bayerBuffer);\n> > +     unsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferIndex(embeddedBuffer);\n> > +\n> >       LOG(RPI, Debug) << \"Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:\"\n> > -                     << \" Bayer buffer id: \" << bayerBuffer->cookie()\n> > -                     << \" Embedded buffer id: \" << embeddedBuffer->cookie();\n> > +                     << \" Bayer buffer id: \" << bayerIndex\n> > +                     << \" Embedded buffer id: \" << embeddedIndex;\n> >\n> >       op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;\n> > -     op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(),\n> > -                 RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() };\n> > +     op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedIndex,\n> > +                 RPiIpaMask::BAYER_DATA | bayerIndex };\n> >       ipa_->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 6635a751..73eb04a1 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> > @@ -178,15 +178,17 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)\n> >       }\n> >  }\n> >\n> > -bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> > +int RPiStream::getBufferIndex(FrameBuffer *buffer) const\n> >  {\n> >       if (importOnly_)\n> > -             return false;\n> > +             return -1;\n> >\n> > -     if (std::find(bufferList_.begin(), bufferList_.end(), buffer) != bufferList_.end())\n> > -             return true;\n> > +     /* Find the buffer in the list, and return the index position. */\n> > +     auto it = std::find(bufferList_.begin(), bufferList_.end(), buffer);\n> > +     if (it != bufferList_.end())\n> > +             return std::distance(bufferList_.begin(), it);\n> >\n> > -     return false;\n> > +     return -1;\n> >  }\n> >\n> >  void RPiStream::clearBuffers()\n> > @@ -199,7 +201,7 @@ void RPiStream::clearBuffers()\n> >\n> >  int RPiStream::queueToDevice(FrameBuffer *buffer)\n> >  {\n> > -     LOG(RPISTREAM, Debug) << \"Queuing buffer \" << buffer->cookie()\n> > +     LOG(RPISTREAM, Debug) << \"Queuing buffer \" << getBufferIndex(buffer)\n> >                             << \" for \" << name_;\n> >\n> >       int 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 6222c867..45cf5237 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> > @@ -48,7 +48,7 @@ public:\n> >       int queueAllBuffers();\n> >       int queueBuffer(FrameBuffer *buffer);\n> >       void returnBuffer(FrameBuffer *buffer);\n> > -     bool findFrameBuffer(FrameBuffer *buffer) const;\n> > +     int getBufferIndex(FrameBuffer *buffer) const;\n> >\n> >  private:\n> >       void clearBuffers();\n> >\n>\n> --\n> Regards\n> --\n> Kieran","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 90474C0109\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 21 Jul 2020 12:38:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 26189608A0;\n\tTue, 21 Jul 2020 14:38:18 +0200 (CEST)","from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 531426053C\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 Jul 2020 14:38:17 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id b25so23873439ljp.6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 21 Jul 2020 05:38:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"nhkcNMXc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=YgW8C+h63Qy2gYsCk/xEQ8x+RrDTU72R3T7y5XRnEYo=;\n\tb=nhkcNMXcb9BPwaZlPKqkEPuHJNjcDyD8SuI1s3HYXkr123sKeAWzc7F4pd/Zrj9c+x\n\tILw0W95yuqMb8YQTpBJSgddmJ9HRZCrMKSZYFZzl/RyRW5wgQJ5gXRvn+ol/d+Tt6dPl\n\tIUOBVcR90Urmsi61gbKOxLBQe5+pKm6Wxd+Zi1cFAD0YIGITWS6kpK25ZpznGMwjb0U2\n\t5IRjpEQvmdgS7OF3BtcNj78kRunIbDeiorrnje6Yd4x6HbbXVl9scktN60rOpi041ZXu\n\tBknL00SLhawsYUYM+SqPfEhMXVz1OSsPxVRcqG+aCVUagEMTvrycGQcuSn3h2bh9ygej\n\tEFeg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=YgW8C+h63Qy2gYsCk/xEQ8x+RrDTU72R3T7y5XRnEYo=;\n\tb=hFZxEu6I/cduypAaXsk+JY1Eowqk2HqmIi7/X0MmW83FZ9AiKRDocadOvqRWpplQfC\n\t3RtNBgXtNuIC+xOa6Hrz7gj/vslq722NSAiYXhCkoN6Lbx9eYJ1RhGL1IIKD26QqeO6A\n\tETMRJmA0hDD7DmcDkY2CkGZjh3n9qoJK+Y7RnbseFR308hWO2M3CTaCDrEEWnxTQ/gD9\n\tZBKzO8l3cDx54W18KIP2u/av9gnMaqmMJ+f5hRxVBrCyIT6COcLM3N0ckvI7WHoet4G8\n\tnY+RO0oj6+HXH+Qd731hUEx0/DT4ntvx4XcVYAnxJdzJdpVvefoT4y099uUjpS4VAlzP\n\t8rdA==","X-Gm-Message-State":"AOAM5309WUMZ2AH75+zKk5yTWbkAdOWx7VXECCu0JSJdZk8NSmvLexQw\n\tWthMvNjKwjUrlmQXRuG6FxNB5eQErunFx4+WvqGOuA==","X-Google-Smtp-Source":"ABdhPJzjvDdj0AzR4BsxwOSm2Sfx8xTrnjzgKCsAjlnhrwxwLdvKxzN7Hp9g/o9Z3hilhuU+J6/bDsTxaXa/RB8MAfM=","X-Received":"by 2002:a2e:97d7:: with SMTP id\n\tm23mr13247227ljj.103.1595335096594; \n\tTue, 21 Jul 2020 05:38:16 -0700 (PDT)","MIME-Version":"1.0","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-10-naush@raspberrypi.com>\n\t<ba0b54a0-9d58-0fae-96f8-77633e44ad54@ideasonboard.com>","In-Reply-To":"<ba0b54a0-9d58-0fae-96f8-77633e44ad54@ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Tue, 21 Jul 2020 13:38:00 +0100","Message-ID":"<CAEmqJPp0CLM5HA-zYKaGqMXC7Riyw1uw-VBBrrGRAayt13yO2Q@mail.gmail.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 9/9] 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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11511,"web_url":"https://patchwork.libcamera.org/comment/11511/","msgid":"<20200722160334.GL29813@pendragon.ideasonboard.com>","date":"2020-07-22T16:03:34","subject":"Re: [libcamera-devel] [PATCH v4 9/9] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Mon, Jul 20, 2020 at 10:13:11AM +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> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\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 c28fe997..e4fc5ac7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -876,7 +876,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> @@ -896,30 +897,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> @@ -1097,7 +1092,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> @@ -1117,12 +1112,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> @@ -1132,7 +1129,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> @@ -1172,7 +1169,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> @@ -1183,12 +1180,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> @@ -1198,7 +1197,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> @@ -1208,7 +1207,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> @@ -1427,13 +1426,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 6635a751..73eb04a1 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -178,15 +178,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\nI also have trouble understanding this patch (likely due to the trouble\nunderstanding the previous ones though). bufferList_ is documented as\n\"All framebuffers associated with this device stream\" and is filled when\nexporting buffers or at start() time. I don't understand how this can\nwork, as applications can provide new frame buffers at any time. What am\nI missing ?\n\n>  \n> -\treturn false;\n> +\treturn -1;\n>  }\n>  \n>  void RPiStream::clearBuffers()\n> @@ -199,7 +201,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 6222c867..45cf5237 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -48,7 +48,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();","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 0A6DCC2E68\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Jul 2020 16:03:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 88ECB6099C;\n\tWed, 22 Jul 2020 18:03:42 +0200 (CEST)","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 4028160540\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jul 2020 18:03:41 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C149C329;\n\tWed, 22 Jul 2020 18:03:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"S4fUGs16\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1595433819;\n\tbh=AhFzIll/giTse7nMkPWyeyZd9u56wWyxMqsb4WYmIWY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=S4fUGs160vV9ttYnb2nlyAJ1ISZjzDpKykAL04IF4PaTYkrRUfCtrW+xje64i1dJB\n\tDzHHQ8j/6z+Zy0XBVnwjv9OJJzizFzp18kFWenNk5ipo2UjEz/HeriEDx6ItXLLpi3\n\t6QeQWnhPLsTqjKt7kTtaVRf7zw50SwOBnRRhDOaE=","Date":"Wed, 22 Jul 2020 19:03:34 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<20200722160334.GL29813@pendragon.ideasonboard.com>","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-10-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200720091311.805092-10-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 9/9] 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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":11513,"web_url":"https://patchwork.libcamera.org/comment/11513/","msgid":"<CAEmqJPpXucWjDAcnn+e-_goUdqZCe3h2WEb8ESjPNw63PHsmPA@mail.gmail.com>","date":"2020-07-22T16:09:52","subject":"Re: [libcamera-devel] [PATCH v4 9/9] libcamera: pipeline: ipa:\n\traspberrypi: Remove use of FrameBuffer cookie","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Wed, 22 Jul 2020 at 17:03, Laurent Pinchart\n<laurent.pinchart@ideasonboard.com> wrote:\n>\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Mon, Jul 20, 2020 at 10:13:11AM +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> > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\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 c28fe997..e4fc5ac7 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -876,7 +876,8 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> >  int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >  {\n> >       RPiCameraData *data = cameraData(camera);\n> > -     int count, ret;\n> > +     unsigned int index;\n> > +     int ret;\n> >\n> >       /*\n> >        * Decide how many internal buffers to allocate. For now, simply look\n> > @@ -896,30 +897,24 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> >       }\n> >\n> >       /*\n> > -      * Add cookies to the ISP Input buffers so that we can link them with\n> > -      * the IPA and RPI_IPA_EVENT_SIGNAL_ISP_PREPARE event.\n> > -      */\n> > -     count = 0;\n> > -     for (auto const &b : data->unicam_[Unicam::Image].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -     }\n> > -\n> > -     /*\n> > -      * Add cookies to the stats and embedded data buffers and link them with\n> > -      * the IPA.\n> > +      * Link the FrameBuffers with the index of their position in the vector\n> > +      * stored in the RPi stream object.\n> > +      *\n> > +      * This will allow us to identify buffers passed between the pipeline\n> > +      * handler and the IPA.\n> >        */\n> > -     count = 0;\n> > +     index = 0;\n> >       for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -             data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b->cookie(),\n> > +             data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,\n> >                                             .planes = b->planes() });\n> > +             index++;\n> >       }\n> >\n> > -     count = 0;\n> > +     index = 0;\n> >       for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {\n> > -             b->setCookie(count++);\n> > -             data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b->cookie(),\n> > +             data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,\n> >                                             .planes = b->planes() });\n> > +             index++;\n> >       }\n> >\n> >       data->ipa_->mapBuffers(data->ipaBuffers_);\n> > @@ -1097,7 +1092,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData\n> >               unsigned int bufferId = action.data[0];\n> >               FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);\n> >\n> > -             LOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << buffer->cookie()\n> > +             LOG(RPI, Debug) << \"Input re-queue to ISP, buffer id \" << bufferId\n> >                               << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >               isp_[Isp::Input].queueBuffer(buffer);\n> > @@ -1117,12 +1112,14 @@ done:\n> >  void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >  {\n> >       RPi::RPiStream *stream = nullptr;\n> > +     int index;\n> >\n> >       if (state_ == State::Stopped)\n> >               return;\n> >\n> >       for (RPi::RPiStream &s : unicam_) {\n> > -             if (s.findFrameBuffer(buffer)) {\n> > +             index = s.getBufferIndex(buffer);\n> > +             if (index != -1) {\n> >                       stream = &s;\n> >                       break;\n> >               }\n> > @@ -1132,7 +1129,7 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer)\n> >       ASSERT(stream);\n> >\n> >       LOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer dequeue\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << index\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       if (stream == &unicam_[Unicam::Image]) {\n> > @@ -1172,7 +1169,7 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >               return;\n> >\n> >       LOG(RPI, Debug) << \"Stream ISP Input buffer complete\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << unicam_[Unicam::Image].getBufferIndex(buffer)\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       /* The ISP input buffer gets re-queued into Unicam. */\n> > @@ -1183,12 +1180,14 @@ void RPiCameraData::ispInputDequeue(FrameBuffer *buffer)\n> >  void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >  {\n> >       RPi::RPiStream *stream = nullptr;\n> > +     int index;\n> >\n> >       if (state_ == State::Stopped)\n> >               return;\n> >\n> >       for (RPi::RPiStream &s : isp_) {\n> > -             if (s.findFrameBuffer(buffer)) {\n> > +             index = s.getBufferIndex(buffer);\n> > +             if (index != -1) {\n> >                       stream = &s;\n> >                       break;\n> >               }\n> > @@ -1198,7 +1197,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >       ASSERT(stream);\n> >\n> >       LOG(RPI, Debug) << \"Stream \" << stream->name() << \" buffer complete\"\n> > -                     << \", buffer id \" << buffer->cookie()\n> > +                     << \", buffer id \" << index\n> >                       << \", timestamp: \" << buffer->metadata().timestamp;\n> >\n> >       /*\n> > @@ -1208,7 +1207,7 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)\n> >       if (stream == &isp_[Isp::Stats]) {\n> >               IPAOperationData op;\n> >               op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;\n> > -             op.data = { RPiIpaMask::STATS | buffer->cookie() };\n> > +             op.data = { RPiIpaMask::STATS | static_cast<unsigned int>(index) };\n> >               ipa_->processEvent(op);\n> >       } else {\n> >               /* Any other ISP output can be handed back to the application now. */\n> > @@ -1427,13 +1426,16 @@ void RPiCameraData::tryRunPipeline()\n> >       /* Set our state to say the pipeline is active. */\n> >       state_ = State::Busy;\n> >\n> > +     unsigned int bayerIndex = unicam_[Unicam::Image].getBufferIndex(bayerBuffer);\n> > +     unsigned int embeddedIndex = unicam_[Unicam::Embedded].getBufferIndex(embeddedBuffer);\n> > +\n> >       LOG(RPI, Debug) << \"Signalling RPI_IPA_EVENT_SIGNAL_ISP_PREPARE:\"\n> > -                     << \" Bayer buffer id: \" << bayerBuffer->cookie()\n> > -                     << \" Embedded buffer id: \" << embeddedBuffer->cookie();\n> > +                     << \" Bayer buffer id: \" << bayerIndex\n> > +                     << \" Embedded buffer id: \" << embeddedIndex;\n> >\n> >       op.operation = RPI_IPA_EVENT_SIGNAL_ISP_PREPARE;\n> > -     op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedBuffer->cookie(),\n> > -                 RPiIpaMask::BAYER_DATA | bayerBuffer->cookie() };\n> > +     op.data = { RPiIpaMask::EMBEDDED_DATA | embeddedIndex,\n> > +                 RPiIpaMask::BAYER_DATA | bayerIndex };\n> >       ipa_->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 6635a751..73eb04a1 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> > @@ -178,15 +178,17 @@ void RPiStream::returnBuffer(FrameBuffer *buffer)\n> >       }\n> >  }\n> >\n> > -bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const\n> > +int RPiStream::getBufferIndex(FrameBuffer *buffer) const\n> >  {\n> >       if (importOnly_)\n> > -             return false;\n> > +             return -1;\n> >\n> > -     if (std::find(bufferList_.begin(), bufferList_.end(), buffer) != bufferList_.end())\n> > -             return true;\n> > +     /* Find the buffer in the list, and return the index position. */\n> > +     auto it = std::find(bufferList_.begin(), bufferList_.end(), buffer);\n> > +     if (it != bufferList_.end())\n> > +             return std::distance(bufferList_.begin(), it);\n>\n> I also have trouble understanding this patch (likely due to the trouble\n> understanding the previous ones though). bufferList_ is documented as\n> \"All framebuffers associated with this device stream\" and is filled when\n> exporting buffers or at start() time. I don't understand how this can\n> work, as applications can provide new frame buffers at any time. What am\n> I missing ?\n\nCurrently, an application can only provide our pipeline with buffers\nit allocates using exportFrameBuffers().  I was not aware that it\ncould allocate them outside of that until Niklas mentioned it.  This\ncode does *not* work for those cases.  Luckily the apps don't do this\neither so all is good for now :-)\n\nI do have a change that is in progress for the cases where new\nframebuffers (from any external allocation) can be used and tracked\ncorrectly.  I wanted this to go through before introducing that set of\nchanges.\n\nRegards,\nNaush\n>\n> >\n> > -     return false;\n> > +     return -1;\n> >  }\n> >\n> >  void RPiStream::clearBuffers()\n> > @@ -199,7 +201,7 @@ void RPiStream::clearBuffers()\n> >\n> >  int RPiStream::queueToDevice(FrameBuffer *buffer)\n> >  {\n> > -     LOG(RPISTREAM, Debug) << \"Queuing buffer \" << buffer->cookie()\n> > +     LOG(RPISTREAM, Debug) << \"Queuing buffer \" << getBufferIndex(buffer)\n> >                             << \" for \" << name_;\n> >\n> >       int 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 6222c867..45cf5237 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> > @@ -48,7 +48,7 @@ public:\n> >       int queueAllBuffers();\n> >       int queueBuffer(FrameBuffer *buffer);\n> >       void returnBuffer(FrameBuffer *buffer);\n> > -     bool findFrameBuffer(FrameBuffer *buffer) const;\n> > +     int getBufferIndex(FrameBuffer *buffer) const;\n> >\n> >  private:\n> >       void clearBuffers();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 75E82BDB1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 22 Jul 2020 16:10:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D8C7260C34;\n\tWed, 22 Jul 2020 18:10:10 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 83F4C60540\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jul 2020 18:10:09 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id m15so960775lfp.7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 22 Jul 2020 09:10:09 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"gaj3r6hN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=L1RDl5fDAEYaAspFyLs0u5K/Lk0G/8ezRCJez61lnkU=;\n\tb=gaj3r6hNXThjfsGmwoFN6jl6/B0loxNc1LYZyyBHYd+Hf9qKW7vouEGgIePmHTPFPx\n\tUGHcKGgKoWIt/cM4jg37IHECowoBRaJHvDqbGA13Afi4qqMXpOEonauYArsiGneDIlAy\n\tVJV+FIAn9TL9UyRKU1cROi3w2kE6W3qMzVy8icLeplHcmR47uaOK8QNqb9ygFm43hyX2\n\tk1Shq7QV3EsTdyhOlD31urw1JJX+zaBh0FBXERV0/0LoP+jnNBngO4EtGGXpbA26pjJb\n\tLBGoU6upKo+gvqxQBLNNn5ZYxcDtcTNXvchmUhtjGyY26amAXo1fAGjEQoZtuRe3dDs9\n\tSjAA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=L1RDl5fDAEYaAspFyLs0u5K/Lk0G/8ezRCJez61lnkU=;\n\tb=NveGvXYrpagBQ70x0azjhM9ZyFzzBhtmLFzOJjVoys2R4j5K/auZKitjF1IoZnnXxG\n\tQTSUXBLdt7zG2OyZ8BGuUgibxt3RufdgyrimgW2qMJHN3Rf63FAhJE9oS6EKwU/wWQTI\n\tzvP6LcGRyUZVsVmb3tfJ+GZ25YJEKJL9KVWtN9n7HyrcaCavsd95XWVS1iTN9f2FFCTJ\n\tBygF//QX4l+is1sj4vCaVDT2YmcGyuPDJ3yIdPCpknhteG9rb8n7eSa6VwbG3isjVdKV\n\tMWDaPWQnH7VqRfgYN5nzE+R2TdnSdQWwuo1rk/3zrn0ZOZ6Ndu9m81L5KkUiCMsbOFSb\n\thrng==","X-Gm-Message-State":"AOAM533QshSHhg952NAMNpP3f2l1+26gpSb+mHcHZwz8xQSlnizMLN88\n\tkN8KthB/IowvM8OeiIe9yKlT0w6V6/EimUZ9kX5tSg==","X-Google-Smtp-Source":"ABdhPJwKD8MwczbGmv+YRuaY2HziZJB1oFJEPjOFhV2DgyyitVWlQS+WrV5aHQ18/QKKaEnY1qEU+2x7C60Hueq/fqU=","X-Received":"by 2002:a19:c894:: with SMTP id y142mr58687lff.74.1595434208715; \n\tWed, 22 Jul 2020 09:10:08 -0700 (PDT)","MIME-Version":"1.0","References":"<20200720091311.805092-1-naush@raspberrypi.com>\n\t<20200720091311.805092-10-naush@raspberrypi.com>\n\t<20200722160334.GL29813@pendragon.ideasonboard.com>","In-Reply-To":"<20200722160334.GL29813@pendragon.ideasonboard.com>","From":"Naushir Patuck <naush@raspberrypi.com>","Date":"Wed, 22 Jul 2020 17:09:52 +0100","Message-ID":"<CAEmqJPpXucWjDAcnn+e-_goUdqZCe3h2WEb8ESjPNw63PHsmPA@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 9/9] 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=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]