[v2,2/2] pipeline: rpi: Rework internal buffer allocations

25603 diff mbox series

In order to clear the V4L2VideoDevice cache, we must call
V4L2VideoDevice::releaseBuffers() when stopping the camera. To do this
without releasing the allocated buffers, we have to rework the internal
buffer allocation logic.

Remove calls to V4L2VideoDevice::importBuffers() and releaseBuffers()
from within the RPi::Stream class. Instead, move these calls to the
PipelineHandlerBase class, where we can call releaseBuffers(), i.e. clear
the V4L2VideoDevice cache unconditionally on stop without de-allocating
the internal buffers.

The loigc Stream::clearBuffers() can be then moved into releaseBuffers()
which will actually de-allocate the internal buffers when required.

Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/265
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # rpi4
---
 .../pipeline/rpi/common/pipeline_base.cpp     |  8 ++++++-
 .../pipeline/rpi/common/rpi_stream.cpp        | 23 ++++++-------------
 .../pipeline/rpi/common/rpi_stream.h          |  1 -
 3 files changed, 14 insertions(+), 18 deletions(-)

Hi Naush

On Thu, Dec 18, 2025 at 12:31:24PM +0000, Naushir Patuck wrote:
> In order to clear the V4L2VideoDevice cache, we must call
> V4L2VideoDevice::releaseBuffers() when stopping the camera. To do this
> without releasing the allocated buffers, we have to rework the internal
> buffer allocation logic.
>
> Remove calls to V4L2VideoDevice::importBuffers() and releaseBuffers()
> from within the RPi::Stream class. Instead, move these calls to the
> PipelineHandlerBase class, where we can call releaseBuffers(), i.e. clear
> the V4L2VideoDevice cache unconditionally on stop without de-allocating
> the internal buffers.
>
> The loigc Stream::clearBuffers() can be then moved into releaseBuffers()
> which will actually de-allocate the internal buffers when required.
>
> Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/265
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # rpi4

Thanks
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

> ---
>  .../pipeline/rpi/common/pipeline_base.cpp     |  8 ++++++-
>  .../pipeline/rpi/common/rpi_stream.cpp        | 23 ++++++-------------
>  .../pipeline/rpi/common/rpi_stream.h          |  1 -
>  3 files changed, 14 insertions(+), 18 deletions(-)
>
> diff --git a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> index 2b61b5d241c5..aa0af367d301 100644
> --- a/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> +++ b/src/libcamera/pipeline/rpi/common/pipeline_base.cpp
> @@ -719,8 +719,10 @@ void PipelineHandlerBase::stopDevice(Camera *camera)
>  	data->state_ = CameraData::State::Stopped;
>  	data->platformStop();
>
> -	for (auto const stream : data->streams_)
> +	for (auto const stream : data->streams_) {
>  		stream->dev()->streamOff();
> +		stream->dev()->releaseBuffers();
> +	}
>
>  	/* Disable SOF event generation. */
>  	data->frontendDevice()->setFrameStartEnabled(false);
> @@ -901,6 +903,10 @@ int PipelineHandlerBase::queueAllBuffers(Camera *camera)
>  	int ret;
>
>  	for (auto const stream : data->streams_) {
> +		ret = stream->dev()->importBuffers(VIDEO_MAX_FRAME);
> +		if (ret < 0)
> +			return ret;
> +
>  		if (stream->getFlags() & StreamFlag::External)
>  			continue;
>
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> index e73f4b7d31af..f420400dfe18 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.cpp
> @@ -12,9 +12,6 @@
>
>  #include <libcamera/base/log.h>
>
> -/* Maximum number of buffer slots to allocate in the V4L2 device driver. */
> -static constexpr unsigned int maxV4L2BufferCount = 32;
> -
>  namespace libcamera {
>
>  LOG_DEFINE_CATEGORY(RPISTREAM)
> @@ -108,7 +105,7 @@ void Stream::setExportedBuffer(FrameBuffer *buffer)
>
>  int Stream::allocateBuffers(unsigned int count)
>  {
> -	int ret;
> +	int ret = 0;
>
>  	if (!(flags_ & StreamFlag::ImportOnly)) {
>  		/* Export some frame buffers for internal use. */
> @@ -121,7 +118,7 @@ int Stream::allocateBuffers(unsigned int count)
>  		resetBuffers();
>  	}
>
> -	return dev_->importBuffers(maxV4L2BufferCount);
> +	return ret;
>  }
>
>  int Stream::queueBuffer(FrameBuffer *buffer)
> @@ -243,8 +240,11 @@ int Stream::queueAllBuffers()
>
>  void Stream::releaseBuffers()
>  {
> -	dev_->releaseBuffers();
> -	clearBuffers();
> +	availableBuffers_ = std::queue<FrameBuffer *>{};
> +	requestBuffers_ = std::queue<FrameBuffer *>{};
> +	internalBuffers_.clear();
> +	bufferMap_.clear();
> +	id_ = 0;
>  }
>
>  void Stream::bufferEmplace(unsigned int id, FrameBuffer *buffer)
> @@ -257,15 +257,6 @@ void Stream::bufferEmplace(unsigned int id, FrameBuffer *buffer)
>  				   std::forward_as_tuple(buffer, false));
>  }
>
> -void Stream::clearBuffers()
> -{
> -	availableBuffers_ = std::queue<FrameBuffer *>{};
> -	requestBuffers_ = std::queue<FrameBuffer *>{};
> -	internalBuffers_.clear();
> -	bufferMap_.clear();
> -	id_ = 0;
> -}
> -
>  int Stream::queueToDevice(FrameBuffer *buffer)
>  {
>  	LOG(RPISTREAM, Debug) << "Queuing buffer " << getBufferId(buffer)
> diff --git a/src/libcamera/pipeline/rpi/common/rpi_stream.h b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> index c267447e5ab5..300a352a7d39 100644
> --- a/src/libcamera/pipeline/rpi/common/rpi_stream.h
> +++ b/src/libcamera/pipeline/rpi/common/rpi_stream.h
> @@ -140,7 +140,6 @@ public:
>
>  private:
>  	void bufferEmplace(unsigned int id, FrameBuffer *buffer);
> -	void clearBuffers();
>  	int queueToDevice(FrameBuffer *buffer);
>
>  	StreamFlags flags_;
> --
> 2.51.0
>