[libcamera-devel] [PATCH v2 10/10] libcamera: pipeline: raspberrypi: Handle any externally allocated FrameBuffer
Naushir Patuck
naush at raspberrypi.com
Thu Jul 16 16:03:33 CEST 2020
On Wed, 15 Jul 2020 at 15:07, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Handle the case where a FrameBuffer that has been externally allocated
> (i.e. not through the v4l2 video device) is passed into a Request.
>
> We must store the buffer pointer in our internal buffer list to identify
> when used, as well as mmap the buffer in the IPA if needed.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 90 +++++++++++--------
> .../pipeline/raspberrypi/rpi_stream.cpp | 5 ++
> .../pipeline/raspberrypi/rpi_stream.h | 1 +
> 3 files changed, 61 insertions(+), 35 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index fc6fbfc3..a32ec687 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -172,8 +172,8 @@ public:
> RPi::RPiDevice<Isp, 4> isp_;
> /* The vector below is just for convenience when iterating over all streams. */
> std::vector<RPi::RPiStream *> streams_;
> - /* Buffers passed to the IPA. */
> - std::vector<IPABuffer> ipaBuffers_;
> + /* Stores the ids of the buffers mapped in the IPA. */
> + std::vector<unsigned int> ipaBufferIds_;
>
> /* VCSM allocation helper. */
> ::RPi::Vcsm vcsm_;
> @@ -193,7 +193,6 @@ public:
> std::queue<FrameBuffer *> bayerQueue_;
> std::queue<FrameBuffer *> embeddedQueue_;
> std::deque<Request *> requestQueue_;
> -
> unsigned int dropFrameCount_;
>
> private:
> @@ -246,6 +245,8 @@ private:
> int queueAllBuffers(Camera *camera);
> int prepareBuffers(Camera *camera);
> void freeBuffers(Camera *camera);
> + void mapBuffers(Camera *camera, const std::vector<FrameBuffer *> &buffers,
> + unsigned int startId);
>
> MediaDevice *unicam_;
> MediaDevice *isp_;
> @@ -730,18 +731,36 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
>
> /* Push all buffers supplied in the Request to the respective streams. */
> for (auto stream : data->streams_) {
> - if (stream->isExternal()) {
> - FrameBuffer *buffer = request->findBuffer(stream);
> + if (!stream->isExternal())
> + continue;
> +
> + FrameBuffer *buffer = request->findBuffer(stream);
> + if (stream->getBufferIndex(buffer) == -1) {
This should check for buffer being valid, else we could dereference a nullptr!
> /*
> - * If we have been asked to drop frames by the IPA, passing
> - * in a nullptr here will queue an internally allocated buffer.
> - * The Request buffer will be stored in another queue to be
> - * queued to the device - in sync with the Request.
> + * This buffer is not recognised, so it must have been allocated
> + * outside the v4l2 device. Store it in the stream buffer list
> + * so we can track it.
> */
> - int ret = stream->queueBuffer(data->dropFrameCount_ ? nullptr : buffer);
> - if (ret)
> - return ret;
> + stream->setExternalBuffer(buffer);
> +
> + /* Also get the IPA to mmap it if needed. */
> + if (stream == &data->unicam_[Unicam::Embedded]) {
> + mapBuffers(camera, { buffer },
> + RPiIpaMask::EMBEDDED_DATA | (stream->getBuffers().size() - 1));
> + } else if (stream == &data->isp_[Isp::Stats]) {
> + mapBuffers(camera, { buffer },
> + RPiIpaMask::STATS | (stream->getBuffers().size() - 1));
> + }
> }
> + /*
> + * If we have been asked to drop frames by the IPA, passing
> + * in a nullptr here will queue an internally allocated buffer.
> + * The Request buffer will be stored in another queue to be
> + * queued to the device - in sync with the Request.
> + */
> + int ret = stream->queueBuffer(data->dropFrameCount_ ? nullptr : buffer);
> + if (ret)
> + return ret;
> }
>
> /* Push the request to the back of the queue. */
> @@ -919,7 +938,6 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> {
> RPiCameraData *data = cameraData(camera);
> - unsigned int index;
> int ret;
>
> /*
> @@ -939,42 +957,44 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> return ret;
> }
>
> + /*
> + * Pass the stats and embedded data buffers to the IPA. No other
> + * buffers need to be passed.
> + */
> + mapBuffers(camera, data->isp_[Isp::Stats].getBuffers(), RPiIpaMask::STATS);
> + mapBuffers(camera, data->unicam_[Unicam::Embedded].getBuffers(), RPiIpaMask::EMBEDDED_DATA);
> +
> + return 0;
> +}
> +
> +void PipelineHandlerRPi::mapBuffers(Camera *camera, const std::vector<FrameBuffer *> &buffers,
> + unsigned int startId)
> +{
> + RPiCameraData *data = cameraData(camera);
> + std::vector<IPABuffer> ipaBuffers;
> + unsigned int id = startId;
> /*
> * Link the FrameBuffers with the index of their position in the vector
> - * stored in the RPi stream object.
> + * stored in the RPi stream object - along with an identifer mask.
> *
> * This will allow us to identify buffers passed between the pipeline
> * handler and the IPA.
> */
> - index = 0;
> - for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | index,
> - .planes = b->planes() });
> - index++;
> + for (auto const &b : buffers) {
> + ipaBuffers.push_back({ .id = id, .planes = b->planes() });
> + data->ipaBufferIds_.push_back(id);
> + id++;
> }
>
> - index = 0;
> - for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {
> - data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | index,
> - .planes = b->planes() });
> - index++;
> - }
> -
> - data->ipa_->mapBuffers(data->ipaBuffers_);
> -
> - return 0;
> + data->ipa_->mapBuffers(ipaBuffers);
> }
>
> void PipelineHandlerRPi::freeBuffers(Camera *camera)
> {
> RPiCameraData *data = cameraData(camera);
>
> - std::vector<unsigned int> ids;
> - for (IPABuffer &ipabuf : data->ipaBuffers_)
> - ids.push_back(ipabuf.id);
> -
> - data->ipa_->unmapBuffers(ids);
> - data->ipaBuffers_.clear();
> + data->ipa_->unmapBuffers(data->ipaBufferIds_);
> + data->ipaBufferIds_.clear();
>
> for (auto const stream : data->streams_)
> stream->releaseBuffers();
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> index 97f87ad7..862a094a 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> @@ -48,6 +48,11 @@ void RPiStream::setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *bu
> [](std::unique_ptr<FrameBuffer> &b) { return b.get(); });
> }
>
> +void RPiStream::setExternalBuffer(FrameBuffer *buffer)
> +{
> + bufferList_.push_back(buffer);
> +}
> +
> const std::vector<FrameBuffer *> &RPiStream::getBuffers() const
> {
> return bufferList_;
> diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> index 16b90fac..266441df 100644
> --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> @@ -41,6 +41,7 @@ public:
> void reset();
> std::string name() const;
> void setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> + void setExternalBuffer(FrameBuffer *buffer);
> const std::vector<FrameBuffer *> &getBuffers() const;
> void releaseBuffers();
> int prepareBuffers(unsigned int count);
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list