[{"id":11418,"web_url":"https://patchwork.libcamera.org/comment/11418/","msgid":"","date":"2020-07-16T14:03:33","subject":"Re: [libcamera-devel] [PATCH v2 10/10] libcamera: pipeline:\n\traspberrypi: Handle any externally allocated FrameBuffer","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 15 Jul 2020 at 15:07, Naushir Patuck wrote:\n>\n> Handle the case where a FrameBuffer that has been externally allocated\n> (i.e. not through the v4l2 video device) is passed into a Request.\n>\n> We must store the buffer pointer in our internal buffer list to identify\n> when used, as well as mmap the buffer in the IPA if needed.\n>\n> Signed-off-by: Naushir Patuck \n> ---\n> .../pipeline/raspberrypi/raspberrypi.cpp | 90 +++++++++++--------\n> .../pipeline/raspberrypi/rpi_stream.cpp | 5 ++\n> .../pipeline/raspberrypi/rpi_stream.h | 1 +\n> 3 files changed, 61 insertions(+), 35 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index fc6fbfc3..a32ec687 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -172,8 +172,8 @@ public:\n> RPi::RPiDevice isp_;\n> /* The vector below is just for convenience when iterating over all streams. */\n> std::vector streams_;\n> - /* Buffers passed to the IPA. */\n> - std::vector ipaBuffers_;\n> + /* Stores the ids of the buffers mapped in the IPA. */\n> + std::vector ipaBufferIds_;\n>\n> /* VCSM allocation helper. */\n> ::RPi::Vcsm vcsm_;\n> @@ -193,7 +193,6 @@ public:\n> std::queue bayerQueue_;\n> std::queue embeddedQueue_;\n> std::deque requestQueue_;\n> -\n> unsigned int dropFrameCount_;\n>\n> private:\n> @@ -246,6 +245,8 @@ private:\n> int queueAllBuffers(Camera *camera);\n> int prepareBuffers(Camera *camera);\n> void freeBuffers(Camera *camera);\n> + void mapBuffers(Camera *camera, const std::vector &buffers,\n> + unsigned int startId);\n>\n> MediaDevice *unicam_;\n> MediaDevice *isp_;\n> @@ -730,18 +731,36 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)\n>\n> /* Push all buffers supplied in the Request to the respective streams. */\n> for (auto stream : data->streams_) {\n> - if (stream->isExternal()) {\n> - FrameBuffer *buffer = request->findBuffer(stream);\n> + if (!stream->isExternal())\n> + continue;\n> +\n> + FrameBuffer *buffer = request->findBuffer(stream);\n> + if (stream->getBufferIndex(buffer) == -1) {\n\nThis should check for buffer being valid, else we could dereference a nullptr!\n\n> /*\n> - * If we have been asked to drop frames by the IPA, passing\n> - * in a nullptr here will queue an internally allocated buffer.\n> - * The Request buffer will be stored in another queue to be\n> - * queued to the device - in sync with the Request.\n> + * This buffer is not recognised, so it must have been allocated\n> + * outside the v4l2 device. Store it in the stream buffer list\n> + * so we can track it.\n> */\n> - int ret = stream->queueBuffer(data->dropFrameCount_ ? nullptr : buffer);\n> - if (ret)\n> - return ret;\n> + stream->setExternalBuffer(buffer);\n> +\n> + /* Also get the IPA to mmap it if needed. */\n> + if (stream == &data->unicam_[Unicam::Embedded]) {\n> + mapBuffers(camera, { buffer },\n> + RPiIpaMask::EMBEDDED_DATA | (stream->getBuffers().size() - 1));\n> + } else if (stream == &data->isp_[Isp::Stats]) {\n> + mapBuffers(camera, { buffer },\n> + RPiIpaMask::STATS | (stream->getBuffers().size() - 1));\n> + }\n> }\n> + /*\n> + * If we have been asked to drop frames by the IPA, passing\n> + * in a nullptr here will queue an internally allocated buffer.\n> + * The Request buffer will be stored in another queue to be\n> + * queued to the device - in sync with the Request.\n> + */\n> + int ret = stream->queueBuffer(data->dropFrameCount_ ? nullptr : buffer);\n> + if (ret)\n> + return ret;\n> }\n>\n> /* Push the request to the back of the queue. */\n> @@ -919,7 +938,6 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)\n> int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> {\n> RPiCameraData *data = cameraData(camera);\n> - unsigned int index;\n> int ret;\n>\n> /*\n> @@ -939,42 +957,44 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)\n> return ret;\n> }\n>\n> + /*\n> + * Pass the stats and embedded data buffers to the IPA. No other\n> + * buffers need to be passed.\n> + */\n> + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), RPiIpaMask::STATS);\n> + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), RPiIpaMask::EMBEDDED_DATA);\n> +\n> + return 0;\n> +}\n> +\n> +void PipelineHandlerRPi::mapBuffers(Camera *camera, const std::vector &buffers,\n> + unsigned int startId)\n> +{\n> + RPiCameraData *data = cameraData(camera);\n> + std::vector ipaBuffers;\n> + unsigned int id = startId;\n> /*\n> * Link the FrameBuffers with the index of their position in the vector\n> - * stored in the RPi stream object.\n> + * stored in the RPi stream object - along with an identifer mask.\n> *\n> * This will allow us to identify buffers passed between the pipeline\n> * handler and the IPA.\n> */\n> - index = 0;\n> - for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {\n> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,\n> - .planes = b->planes() });\n> - index++;\n> + for (auto const &b : buffers) {\n> + ipaBuffers.push_back({ .id = id, .planes = b->planes() });\n> + data->ipaBufferIds_.push_back(id);\n> + id++;\n> }\n>\n> - index = 0;\n> - for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {\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> -\n> - return 0;\n> + data->ipa_->mapBuffers(ipaBuffers);\n> }\n>\n> void PipelineHandlerRPi::freeBuffers(Camera *camera)\n> {\n> RPiCameraData *data = cameraData(camera);\n>\n> - std::vector ids;\n> - for (IPABuffer &ipabuf : data->ipaBuffers_)\n> - ids.push_back(ipabuf.id);\n> -\n> - data->ipa_->unmapBuffers(ids);\n> - data->ipaBuffers_.clear();\n> + data->ipa_->unmapBuffers(data->ipaBufferIds_);\n> + data->ipaBufferIds_.clear();\n>\n> for (auto const stream : data->streams_)\n> stream->releaseBuffers();\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> index 97f87ad7..862a094a 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp\n> @@ -48,6 +48,11 @@ void RPiStream::setExternalBuffers(std::vector> *bu\n> [](std::unique_ptr &b) { return b.get(); });\n> }\n>\n> +void RPiStream::setExternalBuffer(FrameBuffer *buffer)\n> +{\n> + bufferList_.push_back(buffer);\n> +}\n> +\n> const std::vector &RPiStream::getBuffers() const\n> {\n> return bufferList_;\n> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> index 16b90fac..266441df 100644\n> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h\n> @@ -41,6 +41,7 @@ public:\n> void reset();\n> std::string name() const;\n> void setExternalBuffers(std::vector> *buffers);\n> + void setExternalBuffer(FrameBuffer *buffer);\n> const std::vector &getBuffers() const;\n> void releaseBuffers();\n> int prepareBuffers(unsigned int count);\n> --\n> 2.25.1\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 E27CBBD792\n\tfor ;\n\tThu, 16 Jul 2020 14:03:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6C9C760556;\n\tThu, 16 Jul 2020 16:03:53 +0200 (CEST)","from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 26A486048A\n\tfor ;\n\tThu, 16 Jul 2020 16:03:51 +0200 (CEST)","by mail-lf1-x141.google.com with SMTP id k17so3336237lfg.3\n\tfor ;\n\tThu, 16 Jul 2020 07:03:51 -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=\"hFR7ahoG\"; 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\tbh=fyJ+EIIFdok9BYiLVrW7bOJitV14Pb4xA8cAzx1+27U=;\n\tb=hFR7ahoGXp/pm1mjgcwSuDSzCatqCF7xDiUXbikt7S3Jy+gtGFx3dbClHOqsxyZMTH\n\tFn5qtzmS9Clx0n278cHMnWeOifWaqplYlt15U8ptzcuEiosUnSJsZqvl8OcrtAC53hQC\n\tTIU3r/bLeZEBsyFP0wd3Uf+j+pWy0jI7bYLF97wvWaKqQeIv28O+gymLPVsU56cYJ0ah\n\taRet9R73HQSTEYDdARrMFc9j97v/o/ChtFLwtMvgqIsBo4H/TkqsQyTzA55oZ/COPzEm\n\tPmrKGJQJ1NeiFV5jT78hS7QogTAgVVscWybToQIFBOab05ZH9rVP6nl6wGdjca4GmuqV\n\tMjCg==","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;\n\tbh=fyJ+EIIFdok9BYiLVrW7bOJitV14Pb4xA8cAzx1+27U=;\n\tb=H58mMakxrQ2lxYkhC0NORWG9CmYhguX1fAVxOSkhLorqH2YRle+zWUZniiUAvW9MXU\n\tGaLn2leLybW+YgxlVWcqFAeX3635Lzek1zPV7P5hqqTtStIX2cF1198wrQ5Rx2hTHAVf\n\tk1Ur4cfWmhGs91Go+kEJ6gZ/cgIs5tzZ8v4pmV7mtokqiE+xDdSMXCJdsGhQ37gGgWKX\n\tyq++76mtVzmvjPDDQjERKQGaQvU2DFYzM7x+TXC68QlBACFzIAy0SyhWEgGrZ6ioBM4o\n\tk/xRB59HGZ3cJ132jB4srdku/rAvEysr0G/An8F/0neLLqDTwV0iMLtrITa0odk7U7DO\n\tXvTQ==","X-Gm-Message-State":"AOAM531a1c4dvAeRCBBRn2qcLAWI0/WFszjiO5rRh2Zy35IU9UENtJlB\n\t9kl8SXQ9ZD/Lj7mUfHqoetbcw46fpWGsMofwgRy/olVh","X-Google-Smtp-Source":"ABdhPJwL4sU3rfVQ9zS5LBzelxV8gQfbr5HUC6vX3LW5zSLkriQ6MYb/SmPq0qAy92tXP9fzknuHGhGRIjZekO5hcek=","X-Received":"by 2002:a19:ae19:: with SMTP id f25mr2256050lfc.63.1594908229753;\n\tThu, 16 Jul 2020 07:03:49 -0700 (PDT)","MIME-Version":"1.0","References":"<20200715140703.566298-1-naush@raspberrypi.com>\n\t<20200715140703.566298-11-naush@raspberrypi.com>","In-Reply-To":"<20200715140703.566298-11-naush@raspberrypi.com>","From":"Naushir Patuck ","Date":"Thu, 16 Jul 2020 15:03:33 +0100","Message-ID":"","To":"libcamera-devel@lists.libcamera.org","Subject":"Re: [libcamera-devel] [PATCH v2 10/10] libcamera: pipeline:\n\traspberrypi: Handle any externally allocated FrameBuffer","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]