diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 4641c76f..72502c36 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -341,6 +341,25 @@ public: void releaseDevice(Camera *camera) override; private: + /* + * The number of buffers to allocate internally for transferring raw + * frames from the Unicam Image stream to the ISP Input stream. It is + * defined such that: + * - one buffer is being written to in Unicam Image + * - one buffer is being processed in ISP Input + * - one extra buffer is queued to Unicam Image + */ + static constexpr unsigned int kInternalRawBufferCount = 3; + + /* + * The number of buffers to allocate internally for the external + * streams. They're used to drop the first few frames while the IPA + * algorithms converge. It is defined such that: + * - one buffer is being used on the stream + * - one extra buffer is queued + */ + static constexpr unsigned int kInternalDropoutBufferCount = 2; + RPiCameraData *cameraData(Camera *camera) { return static_cast(camera->_d()); @@ -1221,21 +1240,23 @@ int PipelineHandlerRPi::registerCamera(MediaDevice *unicam, MediaDevice *isp, Me return -ENOENT; /* Locate and open the unicam video streams. */ - data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicamImage); + data->unicam_[Unicam::Image] = RPi::Stream("Unicam Image", unicamImage, + kInternalRawBufferCount); /* An embedded data node will not be present if the sensor does not support it. */ MediaEntity *unicamEmbedded = unicam->getEntityByName("unicam-embedded"); if (unicamEmbedded) { - data->unicam_[Unicam::Embedded] = RPi::Stream("Unicam Embedded", unicamEmbedded); + data->unicam_[Unicam::Embedded] = + RPi::Stream("Unicam Embedded", unicamEmbedded, kInternalRawBufferCount); data->unicam_[Unicam::Embedded].dev()->bufferReady.connect(data.get(), &RPiCameraData::unicamBufferDequeue); } /* Tag the ISP input stream as an import stream. */ - data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, true); - data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1); - data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2); - data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3); + data->isp_[Isp::Input] = RPi::Stream("ISP Input", ispOutput0, 0, kInternalRawBufferCount, true); + data->isp_[Isp::Output0] = RPi::Stream("ISP Output0", ispCapture1, kInternalDropoutBufferCount); + data->isp_[Isp::Output1] = RPi::Stream("ISP Output1", ispCapture2, kInternalDropoutBufferCount); + data->isp_[Isp::Stats] = RPi::Stream("ISP Stats", ispCapture3, kInternalDropoutBufferCount); /* Wire up all the buffer connections. */ data->unicam_[Unicam::Image].dev()->dequeueTimeout.connect(data.get(), &RPiCameraData::unicamTimeout); @@ -1428,57 +1449,11 @@ int PipelineHandlerRPi::queueAllBuffers(Camera *camera) int PipelineHandlerRPi::prepareBuffers(Camera *camera) { RPiCameraData *data = cameraData(camera); - unsigned int numRawBuffers = 0; int ret; - for (Stream *s : camera->streams()) { - if (isRaw(s->configuration().pixelFormat)) { - numRawBuffers = s->configuration().bufferCount; - break; - } - } - - /* Decide how many internal buffers to allocate. */ + /* Allocate internal buffers. */ for (auto const stream : data->streams_) { - unsigned int numBuffers; - /* - * For Unicam, allocate a minimum of 4 buffers as we want - * to avoid any frame drops. - */ - constexpr unsigned int minBuffers = 4; - if (stream == &data->unicam_[Unicam::Image]) { - /* - * If an application has configured a RAW stream, allocate - * additional buffers to make up the minimum, but ensure - * we have at least 2 sets of internal buffers to use to - * minimise frame drops. - */ - numBuffers = std::max(2, minBuffers - numRawBuffers); - } else if (stream == &data->isp_[Isp::Input]) { - /* - * ISP input buffers are imported from Unicam, so follow - * similar logic as above to count all the RAW buffers - * available. - */ - numBuffers = numRawBuffers + std::max(2, minBuffers - numRawBuffers); - - } else if (stream == &data->unicam_[Unicam::Embedded]) { - /* - * Embedded data buffers are (currently) for internal use, - * so allocate the minimum required to avoid frame drops. - */ - numBuffers = minBuffers; - } else { - /* - * Since the ISP runs synchronous with the IPA and requests, - * we only ever need one set of internal buffers. Any buffers - * the application wants to hold onto will already be exported - * through PipelineHandlerRPi::exportFrameBuffers(). - */ - numBuffers = 1; - } - - ret = stream->prepareBuffers(numBuffers); + ret = stream->prepareBuffers(); if (ret < 0) return ret; } diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp index 2bb10f25..cde04efa 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp @@ -84,14 +84,24 @@ void Stream::removeExternalBuffer(FrameBuffer *buffer) bufferMap_.erase(id); } -int Stream::prepareBuffers(unsigned int count) +int Stream::prepareBuffers() { + unsigned int slotCount; int ret; if (!importOnly_) { - if (count) { + /* + * External streams overallocate buffer slots in order to handle + * the buffers queued from applications without degrading + * performance. Internal streams only need to have enough buffer + * slots to have the internal buffers queued. + */ + slotCount = isExternal() ? kRPIExternalBufferSlotCount + : internalBufferCount_; + + if (internalBufferCount_) { /* Export some frame buffers for internal use. */ - ret = dev_->exportBuffers(count, &internalBuffers_); + ret = dev_->exportBuffers(internalBufferCount_, &internalBuffers_); if (ret < 0) return ret; @@ -100,23 +110,16 @@ int Stream::prepareBuffers(unsigned int count) resetBuffers(); } - /* We must import all internal/external exported buffers. */ - count = bufferMap_.size(); + } else { + /* + * An importOnly stream doesn't have its own internal buffers, + * so it needs to have the number of buffer slots to reserve + * explicitly declared. + */ + slotCount = bufferSlotCount_; } - /* - * If this is an external stream, we must allocate slots for buffers that - * might be externally allocated. We have no indication of how many buffers - * may be used, so this might overallocate slots in the buffer cache. - * Similarly, if this stream is only importing buffers, we do the same. - * - * \todo Find a better heuristic, or, even better, an exact solution to - * this issue. - */ - if (isExternal() || importOnly_) - count = count * 2; - - return dev_->importBuffers(count); + return dev_->importBuffers(slotCount); } int Stream::queueBuffer(FrameBuffer *buffer) diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h index b8bd79cf..e63bf02b 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h @@ -42,9 +42,13 @@ public: { } - Stream(const char *name, MediaEntity *dev, bool importOnly = false) + Stream(const char *name, MediaEntity *dev, + unsigned int internalBufferCount, + unsigned int bufferSlotCount = 0, bool importOnly = false) : external_(false), importOnly_(importOnly), name_(name), - dev_(std::make_unique(dev)), id_(BufferMask::MaskID) + dev_(std::make_unique(dev)), id_(BufferMask::MaskID), + internalBufferCount_(internalBufferCount), + bufferSlotCount_(bufferSlotCount) { } @@ -63,7 +67,7 @@ public: void setExternalBuffer(FrameBuffer *buffer); void removeExternalBuffer(FrameBuffer *buffer); - int prepareBuffers(unsigned int count); + int prepareBuffers(); int queueBuffer(FrameBuffer *buffer); void returnBuffer(FrameBuffer *buffer); @@ -71,6 +75,8 @@ public: void releaseBuffers(); private: + static constexpr unsigned int kRPIExternalBufferSlotCount = 16; + class IdGenerator { public: @@ -133,6 +139,18 @@ private: /* All frame buffers associated with this device stream. */ BufferMap bufferMap_; + /* + * The number of buffers that should be allocated internally for this + * stream. + */ + unsigned int internalBufferCount_; + + /* + * The number of buffer slots that should be reserved for this stream. + * Only relevant for importOnly streams. + */ + unsigned int bufferSlotCount_; + /* * List of frame buffers that we can use if none have been provided by * the application for external streams. This is populated by the