[libcamera-devel] [PATCH v3 06/10] libcamera: pipeline: raspberrypi: Rework stream buffer logic for zero-copy
Naushir Patuck
naush at raspberrypi.com
Mon Jul 20 10:30:26 CEST 2020
Hi Niklas,
Thanks for the review.
On Sat, 18 Jul 2020 at 16:33, Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> Hi Naushir,
>
> Thanks for your work.
>
> On 2020-07-17 09:54:06 +0100, Naushir Patuck wrote:
> > Stop using v4l2_videodevice::allocateBuffer() for internal buffers and
> > instead export/import all buffers. This allows the pipeline to return
> > any stream buffer requested by the application as zero-copy.
> >
> > Advertise the Unicam Image stream as the RAW capture stream now.
> >
> > The RPiStream object now maintains a new list of buffers that are
> > available to queue into a device. This is needed to distinguish between
> > FrameBuffers allocated for internal use vs externally provided buffers.
> > When a Request comes in, if a buffer is not provided for an exported
> > stream, we re-use a buffer from this list.
> >
> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>
> I really like this patch. I'm no expert on the raspberrypi pipeline so I
> think it needs a second review of someone who knows it better then me.
> But save a small nit bellow I see nothing that is odd, so with the nit
> fixed,
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > ---
> > .../pipeline/raspberrypi/raspberrypi.cpp | 228 ++++++++++--------
> > .../pipeline/raspberrypi/rpi_stream.cpp | 122 +++++++---
> > .../pipeline/raspberrypi/rpi_stream.h | 30 ++-
> > 3 files changed, 226 insertions(+), 154 deletions(-)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index ca8434a3..f63bf497 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -199,7 +199,8 @@ private:
> > void checkRequestCompleted();
> > void tryRunPipeline();
> > void tryFlushQueues();
> > - FrameBuffer *updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp, V4L2VideoDevice *dev);
> > + FrameBuffer *updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp,
> > + RPi::RPiStream *stream);
> >
> > unsigned int ispOutputCount_;
> > };
> > @@ -520,8 +521,15 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> > StreamConfiguration &cfg = config->at(i);
> >
> > if (isRaw(cfg.pixelFormat)) {
> > - cfg.setStream(&data->isp_[Isp::Input]);
> > - data->isp_[Isp::Input].setExternal(true);
> > + cfg.setStream(&data->unicam_[Unicam::Image]);
> > + /*
> > + * We must set both Unicam streams as external, even
> > + * though the application may only request RAW frames.
> > + * This is because we match timestamps on both streams
> > + * to synchronise buffers.
> > + */
> > + data->unicam_[Unicam::Image].setExternal(true);
> > + data->unicam_[Unicam::Embedded].setExternal(true);
> > continue;
> > }
> >
> > @@ -624,7 +632,7 @@ int PipelineHandlerRPi::exportFrameBuffers(Camera *camera, Stream *stream,
> > unsigned int count = stream->configuration().bufferCount;
> > int ret = s->dev()->exportBuffers(count, buffers);
> >
> > - s->setExternalBuffers(buffers);
> > + s->setExportedBuffers(buffers);
> >
> > return ret;
> > }
> > @@ -724,14 +732,23 @@ int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
> > if (data->state_ == RPiCameraData::State::Stopped)
> > return -EINVAL;
> >
> > - /* Ensure all external streams have associated buffers! */
> > - for (auto &stream : data->isp_) {
> > - if (!stream.isExternal())
> > - continue;
> > + LOG(RPI, Debug) << "queueRequestDevice: New request.";
> >
> > - if (!request->findBuffer(&stream)) {
> > - LOG(RPI, Error) << "Attempt to queue request with invalid stream.";
> > - return -ENOENT;
> > + /* Push all buffers supplied in the Request to the respective streams. */
> > + for (auto stream : data->streams_) {
> > + if (stream->isExternal()) {
> > + FrameBuffer *buffer = request->findBuffer(stream);
> > + /*
> > + * A nullptr in buffer means that we should queue an internally
> > + * allocated buffer.
> > + *
> > + * The below queueBuffer() call will do nothing if there are not
> > + * enough internal buffers allocated, but this will be handled by
> > + * queuing the request for buffers in the RPiStream object.
> > + */
> > + int ret = stream->queueBuffer(buffer);
> > + if (ret)
> > + return ret;
> > }
> > }
> >
> > @@ -820,12 +837,10 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)
> > /* Initialize the camera properties. */
> > data->properties_ = data->sensor_->properties();
> >
> > - /*
> > - * List the available output streams.
> > - * Currently cannot do Unicam streams!
> > - */
> > + /*List the available streams an application may request. */
> > std::set<Stream *> streams;
> > - streams.insert(&data->isp_[Isp::Input]);
> > + streams.insert(&data->unicam_[Unicam::Image]);
> > + streams.insert(&data->unicam_[Unicam::Embedded]);
> > streams.insert(&data->isp_[Isp::Output0]);
> > streams.insert(&data->isp_[Isp::Output1]);
> > streams.insert(&data->isp_[Isp::Stats]);
> > @@ -843,9 +858,28 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera)
> > int ret;
> >
> > for (auto const stream : data->streams_) {
> > - ret = stream->queueBuffers();
> > - if (ret < 0)
> > - return ret;
> > + if (!stream->isExternal()) {
> > + ret = stream->queueAllBuffers();
> > + if (ret < 0)
> > + return ret;
> > + } else {
> > + /*
> > + * For external streams, we must queue up a set of internal
> > + * buffers to handle the number of drop frames requested by
> > + * the IPA. This is done by passing nullptr in queueBuffer().
> > + *
> > + * The below queueBuffer() call will do nothing if there
> > + * are not enough internal buffers allocated, but this will
> > + * be handled by queuing the request for buffers in the
> > + * RPiStream object.
> > + */
> > + unsigned int i;
> > + for (i = 0; i < data->dropFrameCount_; i++) {
> > + int ret = stream->queueBuffer(nullptr);
> > + if (ret)
> > + return ret;
> > + }
> > + }
> > }
> >
> > return 0;
> > @@ -859,7 +893,8 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> > /*
> > * Decide how many internal buffers to allocate. For now, simply look
> > * at how many external buffers will be provided. Will need to improve
> > - * this logic.
> > + * this logic. However, we really must have all stream allocate the same
> > + * number of buffers to simplify error handling in queueRequestDevice().
> > */
> > unsigned int maxBuffers = 0;
> > for (const Stream *s : camera->streams())
> > @@ -867,33 +902,9 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> > maxBuffers = std::max(maxBuffers, s->configuration().bufferCount);
> >
> > for (auto const stream : data->streams_) {
> > - if (stream->isExternal() || stream->isImporter()) {
> > - /*
> > - * If a stream is marked as external reserve memory to
> > - * prepare to import as many buffers are requested in
> > - * the stream configuration.
> > - *
> > - * If a stream is an internal stream with importer
> > - * role, reserve as many buffers as possible.
> > - */
> > - unsigned int count = stream->isExternal()
> > - ? stream->configuration().bufferCount
> > - : maxBuffers;
> > - ret = stream->importBuffers(count);
> > - if (ret < 0)
> > - return ret;
> > - } else {
> > - /*
> > - * If the stream is an internal exporter allocate and
> > - * export as many buffers as possible to its internal
> > - * pool.
> > - */
> > - ret = stream->allocateBuffers(maxBuffers);
> > - if (ret < 0) {
> > - freeBuffers(camera);
> > - return ret;
> > - }
> > - }
> > + ret = stream->prepareBuffers(maxBuffers);
> > + if (ret < 0)
> > + return ret;
> > }
> >
> > /*
> > @@ -901,7 +912,7 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> > * the IPA and RPI_IPA_EVENT_SIGNAL_ISP_PREPARE event.
> > */
> > count = 0;
> > - for (auto const &b : *data->unicam_[Unicam::Image].getBuffers()) {
> > + for (auto const &b : data->unicam_[Unicam::Image].getBuffers()) {
> > b->setCookie(count++);
> > }
> >
> > @@ -910,14 +921,14 @@ int PipelineHandlerRPi::prepareBuffers(Camera *camera)
> > * the IPA.
> > */
> > count = 0;
> > - for (auto const &b : *data->isp_[Isp::Stats].getBuffers()) {
> > + for (auto const &b : data->isp_[Isp::Stats].getBuffers()) {
> > b->setCookie(count++);
> > data->ipaBuffers_.push_back({ .id = RPiIpaMask::STATS | b->cookie(),
> > .planes = b->planes() });
> > }
> >
> > count = 0;
> > - for (auto const &b : *data->unicam_[Unicam::Embedded].getBuffers()) {
> > + for (auto const &b : data->unicam_[Unicam::Embedded].getBuffers()) {
> > b->setCookie(count++);
> > data->ipaBuffers_.push_back({ .id = RPiIpaMask::EMBEDDED_DATA | b->cookie(),
> > .planes = b->planes() });
> > @@ -1089,7 +1100,7 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> > switch (action.operation) {
> > case RPI_IPA_ACTION_STATS_METADATA_COMPLETE: {
> > unsigned int bufferId = action.data[0];
> > - FrameBuffer *buffer = isp_[Isp::Stats].getBuffers()->at(bufferId).get();
> > + FrameBuffer *buffer = isp_[Isp::Stats].getBuffers().at(bufferId);
> >
> > handleStreamBuffer(buffer, &isp_[Isp::Stats]);
> > /* Fill the Request metadata buffer with what the IPA has provided */
> > @@ -1100,19 +1111,19 @@ void RPiCameraData::queueFrameAction(unsigned int frame, const IPAOperationData
> >
> > case RPI_IPA_ACTION_EMBEDDED_COMPLETE: {
> > unsigned int bufferId = action.data[0];
> > - FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers()->at(bufferId).get();
> > + FrameBuffer *buffer = unicam_[Unicam::Embedded].getBuffers().at(bufferId);
> > handleStreamBuffer(buffer, &unicam_[Unicam::Embedded]);
> > break;
> > }
> >
> > case RPI_IPA_ACTION_RUN_ISP: {
> > unsigned int bufferId = action.data[0];
> > - FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers()->at(bufferId).get();
> > + FrameBuffer *buffer = unicam_[Unicam::Image].getBuffers().at(bufferId);
> >
> > LOG(RPI, Debug) << "Input re-queue to ISP, buffer id " << buffer->cookie()
> > << ", timestamp: " << buffer->metadata().timestamp;
> >
> > - isp_[Isp::Input].dev()->queueBuffer(buffer);
> > + isp_[Isp::Input].queueBuffer(buffer);
> > ispOutputCount_ = 0;
> > break;
> > }
> > @@ -1213,22 +1224,27 @@ void RPiCameraData::ispOutputDequeue(FrameBuffer *buffer)
> > << ", buffer id " << buffer->cookie()
> > << ", timestamp: " << buffer->metadata().timestamp;
> >
> > - handleStreamBuffer(buffer, stream);
> > -
> > /*
> > - * Increment the number of ISP outputs generated.
> > - * This is needed to track dropped frames.
> > + * ISP statistics buffer must not be re-queued or sent back to the
> > + * application until after the IPA signals so.
> > */
> > - ispOutputCount_++;
> > -
> > - /* If this is a stats output, hand it to the IPA now. */
> > if (stream == &isp_[Isp::Stats]) {
> > IPAOperationData op;
> > op.operation = RPI_IPA_EVENT_SIGNAL_STAT_READY;
> > op.data = { RPiIpaMask::STATS | buffer->cookie() };
> > ipa_->processEvent(op);
> > + } else {
> > + /* Any other ISP output can be handed back to the application now. */
> > + handleStreamBuffer(buffer, stream);
> > }
> >
> > + /*
> > + * Increment the number of ISP outputs generated.
> > + * This is needed to track dropped frames.
> > + */
> > + ispOutputCount_++;
> > +
> > +
> > handleState();
> > }
> >
> > @@ -1241,8 +1257,12 @@ void RPiCameraData::clearIncompleteRequests()
> > */
> > for (auto const request : requestQueue_) {
> > for (auto const stream : streams_) {
> > - if (stream->isExternal())
> > - stream->dev()->queueBuffer(request->findBuffer(stream));
> > + if (!stream->isExternal())
> > + continue;
> > +
> > + FrameBuffer *buffer = request->findBuffer(stream);
> > + if (buffer)
> > + stream->queueBuffer(buffer);
> > }
> > }
> >
> > @@ -1271,7 +1291,7 @@ void RPiCameraData::clearIncompleteRequests()
> > * Has the buffer already been handed back to the
> > * request? If not, do so now.
> > */
> > - if (buffer->request())
> > + if (buffer && buffer->request())
> > pipe_->completeBuffer(camera_, request, buffer);
> > }
> >
> > @@ -1283,30 +1303,24 @@ void RPiCameraData::clearIncompleteRequests()
> > void RPiCameraData::handleStreamBuffer(FrameBuffer *buffer, RPi::RPiStream *stream)
> > {
> > if (stream->isExternal()) {
> > - if (!dropFrameCount_) {
> > - Request *request = buffer->request();
> > + Request *request = requestQueue_.front();
> > +
> > + if (!dropFrameCount_ && request->findBuffer(stream) == buffer) {
> > + /*
> > + * Tag the buffer as completed, returning it to the
> > + * application.
> > + */
> > pipe_->completeBuffer(camera_, request, buffer);
> > + } else {
> > + /*
> > + * This buffer was not part of the Request, so we can
> > + * recycle it.
> > + */
> > + stream->returnBuffer(buffer);
> > }
> > } else {
> > - /* Special handling for RAW buffer Requests.
> > - *
> > - * The ISP input stream is alway an import stream, but if the
> > - * current Request has been made for a buffer on the stream,
> > - * simply memcpy to the Request buffer and requeue back to the
> > - * device.
> > - */
> > - if (stream == &unicam_[Unicam::Image] && !dropFrameCount_) {
> > - const Stream *rawStream = static_cast<const Stream *>(&isp_[Isp::Input]);
> > - Request *request = requestQueue_.front();
> > - FrameBuffer *raw = request->findBuffer(const_cast<Stream *>(rawStream));
> > - if (raw) {
> > - raw->copyFrom(buffer);
> > - pipe_->completeBuffer(camera_, request, raw);
> > - }
> > - }
> > -
> > - /* Simply requeue the buffer. */
> > - stream->dev()->queueBuffer(buffer);
> > + /* Simply re-queue the buffer to the requested stream. */
> > + stream->queueBuffer(buffer);
> > }
> > }
> >
> > @@ -1390,7 +1404,7 @@ void RPiCameraData::tryRunPipeline()
> > * current bayer buffer will be removed and re-queued to the driver.
> > */
> > embeddedBuffer = updateQueue(embeddedQueue_, bayerBuffer->metadata().timestamp,
> > - unicam_[Unicam::Embedded].dev());
> > + &unicam_[Unicam::Embedded]);
> >
> > if (!embeddedBuffer) {
> > LOG(RPI, Debug) << "Could not find matching embedded buffer";
> > @@ -1409,7 +1423,7 @@ void RPiCameraData::tryRunPipeline()
> >
> > embeddedBuffer = embeddedQueue_.front();
> > bayerBuffer = updateQueue(bayerQueue_, embeddedBuffer->metadata().timestamp,
> > - unicam_[Unicam::Image].dev());
> > + &unicam_[Unicam::Image]);
> >
> > if (!bayerBuffer) {
> > LOG(RPI, Debug) << "Could not find matching bayer buffer - ending.";
> > @@ -1417,11 +1431,7 @@ void RPiCameraData::tryRunPipeline()
> > }
> > }
> >
> > - /*
> > - * Take the first request from the queue and action the IPA.
> > - * Unicam buffers for the request have already been queued as they come
> > - * in.
> > - */
> > + /* Take the first request from the queue and action the IPA. */
> > Request *request = requestQueue_.front();
> >
> > /*
> > @@ -1433,12 +1443,6 @@ void RPiCameraData::tryRunPipeline()
> > op.controls = { request->controls() };
> > ipa_->processEvent(op);
> >
> > - /* Queue up any ISP buffers passed into the request. */
> > - for (auto &stream : isp_) {
> > - if (stream.isExternal())
> > - stream.dev()->queueBuffer(request->findBuffer(&stream));
> > - }
> > -
> > /* Ready to use the buffers, pop them off the queue. */
> > bayerQueue_.pop();
> > embeddedQueue_.pop();
> > @@ -1468,32 +1472,42 @@ void RPiCameraData::tryFlushQueues()
> > * and give a chance for the hardware to return to lock-step. We do have
> > * to drop all interim frames.
> > */
> > - if (unicam_[Unicam::Image].getBuffers()->size() == bayerQueue_.size() &&
> > - unicam_[Unicam::Embedded].getBuffers()->size() == embeddedQueue_.size()) {
> > + if (unicam_[Unicam::Image].getBuffers().size() == bayerQueue_.size() &&
> > + unicam_[Unicam::Embedded].getBuffers().size() == embeddedQueue_.size()) {
> > + /* This cannot happen when Unicam streams are external. */
> > + assert(!unicam_[Unicam::Image].isExternal());
> > +
> > LOG(RPI, Warning) << "Flushing all buffer queues!";
> >
> > while (!bayerQueue_.empty()) {
> > - unicam_[Unicam::Image].dev()->queueBuffer(bayerQueue_.front());
> > + unicam_[Unicam::Image].queueBuffer(bayerQueue_.front());
> > bayerQueue_.pop();
> > }
> >
> > while (!embeddedQueue_.empty()) {
> > - unicam_[Unicam::Embedded].dev()->queueBuffer(embeddedQueue_.front());
> > + unicam_[Unicam::Embedded].queueBuffer(embeddedQueue_.front());
> > embeddedQueue_.pop();
> > }
> > }
> > }
> >
> > FrameBuffer *RPiCameraData::updateQueue(std::queue<FrameBuffer *> &q, uint64_t timestamp,
> > - V4L2VideoDevice *dev)
> > + RPi::RPiStream *stream)
> > {
> > + /*
> > + * If the unicam streams are external (both have to the same), then we
> > + * can only return out the top buffer in the queue, and assume they have
> > + * been synced by queuing at the same time. We cannot drop these frames,
> > + * as they may have been provided externally.
> > + */
> > while (!q.empty()) {
> > FrameBuffer *b = q.front();
> > - if (b->metadata().timestamp < timestamp) {
> > + if (!stream->isExternal() && b->metadata().timestamp < timestamp) {
> > q.pop();
> > - dev->queueBuffer(b);
> > - LOG(RPI, Warning) << "Dropping input frame!";
> > - } else if (b->metadata().timestamp == timestamp) {
> > + stream->queueBuffer(b);
> > + LOG(RPI, Warning) << "Dropping unmatched input frame in stream "
> > + << stream->name();
> > + } else if (stream->isExternal() || b->metadata().timestamp == timestamp) {
> > /* The calling function will pop the item from the queue. */
> > return b;
> > } else {
> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > index 57e5cf72..53a335e3 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp
> > @@ -21,30 +21,20 @@ V4L2VideoDevice *RPiStream::dev() const
> >
> > void RPiStream::setExternal(bool external)
> > {
> > + /* Import streams cannot be external. */
> > + assert(!external || !importOnly_);
> > external_ = external;
> > }
> >
> > bool RPiStream::isExternal() const
> > {
> > - /*
> > - * Import streams cannot be external.
> > - *
> > - * RAW capture is a special case where we simply copy the RAW
> > - * buffer out of the request. All other buffer handling happens
> > - * as if the stream is internal.
> > - */
> > - return external_ && !importOnly_;
> > -}
> > -
> > -bool RPiStream::isImporter() const
> > -{
> > - return importOnly_;
> > + return external_;
> > }
> >
> > void RPiStream::reset()
> > {
> > external_ = false;
> > - internalBuffers_.clear();
> > + clearBuffers();
> > }
> >
> > std::string RPiStream::name() const
> > @@ -52,65 +42,123 @@ std::string RPiStream::name() const
> > return name_;
> > }
> >
> > -void RPiStream::setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > +void RPiStream::setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> > {
> > - externalBuffers_ = buffers;
> > + std::transform(buffers->begin(), buffers->end(), std::back_inserter(bufferList_),
> > + [](std::unique_ptr<FrameBuffer> &b) { return b.get(); });
> > }
> >
> > -const std::vector<std::unique_ptr<FrameBuffer>> *RPiStream::getBuffers() const
> > +const std::vector<FrameBuffer *> &RPiStream::getBuffers() const
> > {
> > - return external_ ? externalBuffers_ : &internalBuffers_;
> > + return bufferList_;
> > }
> >
> > void RPiStream::releaseBuffers()
> > {
> > dev_->releaseBuffers();
> > - if (!external_ && !importOnly_)
> > - internalBuffers_.clear();
> > + clearBuffers();
> > }
> >
> > -int RPiStream::importBuffers(unsigned int count)
> > +int RPiStream::prepareBuffers(unsigned int count)
> > {
> > + int ret;
> > +
> > + if (!importOnly_) {
> > + if (count) {
> > + /* Export some frame buffers for internal use. */
> > + ret = dev_->exportBuffers(count, &internalBuffers_);
> > + if (ret < 0)
> > + return ret;
> > +
> > + /* Add these exported buffers to the internal/external buffer list. */
> > + setExportedBuffers(&internalBuffers_);
> > +
> > + /* Add these buffers to the queue of internal usable buffers. */
> > + for (auto const &buffer : internalBuffers_)
> > + availableBuffers_.push(buffer.get());
> > + }
> > +
> > + /* We must import all internal/external exported buffers. */
> > + count = bufferList_.size();
> > + }
> > +
> > return dev_->importBuffers(count);
> > }
> >
> > -int RPiStream::allocateBuffers(unsigned int count)
> > +int RPiStream::queueAllBuffers()
> > {
> > - return dev_->allocateBuffers(count, &internalBuffers_);
> > -}
> > + int ret;
> >
> > -int RPiStream::queueBuffers()
> > -{
> > if (external_)
> > return 0;
> >
> > - for (auto &b : internalBuffers_) {
> > - int ret = dev_->queueBuffer(b.get());
> > - if (ret) {
> > - LOG(RPISTREAM, Error) << "Failed to queue buffers for "
> > - << name_;
> > + while (!availableBuffers_.empty()) {
> > + ret = queueBuffer(availableBuffers_.front());
> > + if (ret < 0)
> > return ret;
> > - }
> > +
> > + availableBuffers_.pop();
> > }
> >
> > return 0;
> > }
> >
> > -bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const
> > +int RPiStream::queueBuffer(FrameBuffer *buffer)
> > +{
> > + /*
> > + * A nullptr buffer implies an external stream, but no external
> > + * buffer has been supplied. So, pick one from the availableBuffers_
> > + * queue.
> > + */
> > + if (!buffer) {
> > + if (availableBuffers_.empty()) {
> > + LOG(RPISTREAM, Warning) << "No buffers available for "
> > + << name_;
> > + return -EINVAL;
> > + }
> > +
> > + buffer = availableBuffers_.front();
> > + availableBuffers_.pop();
> > + }
> > +
> > + LOG(RPISTREAM, Debug) << "Queuing buffer " << buffer->cookie()
> > + << " for " << name_;
> > +
> > + int ret = dev_->queueBuffer(buffer);
> > + if (ret) {
> > + LOG(RPISTREAM, Error) << "Failed to queue buffer for "
> > + << name_;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +void RPiStream::returnBuffer(FrameBuffer *buffer)
> > {
> > - auto start = external_ ? externalBuffers_->begin() : internalBuffers_.begin();
> > - auto end = external_ ? externalBuffers_->end() : internalBuffers_.end();
> > + /* This can only be called for external streams. */
> > + assert(external_);
> > +
> > + availableBuffers_.push(buffer);
> > +}
> >
> > +bool RPiStream::findFrameBuffer(FrameBuffer *buffer) const
> > +{
> > if (importOnly_)
> > return false;
> >
> > - if (std::find_if(start, end,
> > - [buffer](std::unique_ptr<FrameBuffer> const &ref) { return ref.get() == buffer; }) != end)
> > + if (std::find(bufferList_.begin(), bufferList_.end(), buffer) != bufferList_.end())
> > return true;
>
> Cant this be made simpler by ?
>
> if (bufferList_.find(buffer) != bufferList_.end())
>
bufferList_ is a std::vector, so in this case I must use std::find here.
> >
> > return false;
> > }
> >
> > +void RPiStream::clearBuffers()
> > +{
> > + availableBuffers_ = std::queue<FrameBuffer *>{};
> > + internalBuffers_.clear();
> > + bufferList_.clear();
> > +}
> > +
> > } /* namespace RPi */
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > index 40fff81d..019e236d 100644
> > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h
> > @@ -38,21 +38,22 @@ public:
> > V4L2VideoDevice *dev() const;
> > void setExternal(bool external);
> > bool isExternal() const;
> > - bool isImporter() const;
> > void reset();
> > std::string name() const;
> > - void setExternalBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > - const std::vector<std::unique_ptr<FrameBuffer>> *getBuffers() const;
> > + void setExportedBuffers(std::vector<std::unique_ptr<FrameBuffer>> *buffers);
> > + const std::vector<FrameBuffer *> &getBuffers() const;
> > void releaseBuffers();
> > - int importBuffers(unsigned int count);
> > - int allocateBuffers(unsigned int count);
> > - int queueBuffers();
> > + int prepareBuffers(unsigned int count);
> > + int queueAllBuffers();
> > + int queueBuffer(FrameBuffer *buffer);
> > + void returnBuffer(FrameBuffer *buffer);
> > bool findFrameBuffer(FrameBuffer *buffer) const;
> >
> > private:
> > + void clearBuffers();
> > /*
> > * Indicates that this stream is active externally, i.e. the buffers
> > - * are provided by the application.
> > + * might be provided by (and returned to) the application.
> > */
> > bool external_;
> > /* Indicates that this stream only imports buffers, e.g. ISP input. */
> > @@ -61,10 +62,19 @@ private:
> > std::string name_;
> > /* The actual device stream. */
> > std::unique_ptr<V4L2VideoDevice> dev_;
> > - /* Internally allocated framebuffers associated with this device stream. */
> > + /* All framebuffers associated with this device stream. */
> > + std::vector<FrameBuffer *> bufferList_;
> > + /*
> > + * List of frame buffer that we can use if none have been provided by
> > + * the application for external streams. This is populated by the
> > + * buffers exported internally.
> > + */
> > + std::queue<FrameBuffer *> availableBuffers_;
> > + /*
> > + * This is a list of buffers exported internally. Need to keep this around
> > + * as the stream needs to maintain ownership of these buffers.
> > + */
> > std::vector<std::unique_ptr<FrameBuffer>> internalBuffers_;
> > - /* Externally allocated framebuffers associated with this device stream. */
> > - std::vector<std::unique_ptr<FrameBuffer>> *externalBuffers_;
> > };
> >
> > /*
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
Regards,
Naush
More information about the libcamera-devel
mailing list