[libcamera-devel] [PATCH v2 3/6] pipeline: raspberrypi: Free buffers in the RPiCamera destructor and re-configure
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Mar 18 15:23:40 CET 2022
Quoting Naushir Patuck via libcamera-devel (2022-03-17 14:08:24)
> Currently, all framebuffer allocations get freed and cleared on a stop in
> PipelineHandlerRPi::stopDevice(). If PipelineHandlerRPi::start() is then called
> without an intermediate PipelineHandlerRPi::configure(), it will re-allocate and
> prepare all the buffers again, which is unnecessary.
>
> Fix this by not freeing the buffer in PipelineHandlerRPi::stopDevice(), but
> insted doing it in PipelineHandlerRPi::configure(), as the buffers might have
> to be resized.
>
> Add a flag to indicate that buffer allocations need to be done on the next
> call to PipelineHandlerRPi::start().
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> Tested-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 33 +++++++++++++------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 2281b43fc3ac..082273828894 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -185,10 +185,15 @@ public:
> RPiCameraData(PipelineHandler *pipe)
> : Camera::Private(pipe), state_(State::Stopped),
> supportsFlips_(false), flipsAlterBayerOrder_(false),
> - dropFrameCount_(0), ispOutputCount_(0)
> + dropFrameCount_(0), reallocate_(true), ispOutputCount_(0)
> {
> }
>
> + ~RPiCameraData()
> + {
> + freeBuffers();
> + }
> +
> void freeBuffers();
> void frameStarted(uint32_t sequence);
>
> @@ -280,6 +285,9 @@ public:
> */
> std::optional<int32_t> notifyGainsUnity_;
>
> + /* Has this camera been reconfigured? */
> + bool reallocate_;
> +
> private:
> void checkRequestCompleted();
> void fillRequestMetadata(const ControlList &bufferControls,
> @@ -682,7 +690,8 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config)
> RPiCameraData *data = cameraData(camera);
> int ret;
>
> - /* Start by resetting the Unicam and ISP stream states. */
> + /* Start by freeing all buffers and reset the Unicam and ISP stream states. */
> + data->freeBuffers();
> for (auto const stream : data->streams_)
> stream->reset();
>
> @@ -982,12 +991,16 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls)
> RPiCameraData *data = cameraData(camera);
> int ret;
>
> - /* Allocate buffers for internal pipeline usage. */
> - ret = prepareBuffers(camera);
> - if (ret) {
> - LOG(RPI, Error) << "Failed to allocate buffers";
> - stop(camera);
> - return ret;
> + if (data->reallocate_) {
On the first iteration, this isn't a reallocation, so I would have
probably used a variable called 'hasBuffers_', and
if (!data->hasBuffers_)
or something to declare the state rather than the action.
But this looks like it works all the same.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> + /* Allocate buffers for internal pipeline usage. */
> + ret = prepareBuffers(camera);
> + if (ret) {
> + LOG(RPI, Error) << "Failed to allocate buffers";
> + data->freeBuffers();
> + stop(camera);
> + return ret;
> + }
> + data->reallocate_ = false;
> }
>
> /* Check if a ScalerCrop control was specified. */
> @@ -1055,8 +1068,6 @@ void PipelineHandlerRPi::stopDevice(Camera *camera)
>
> /* Stop the IPA. */
> data->ipa_->stop();
> -
> - data->freeBuffers();
> }
>
> int PipelineHandlerRPi::queueRequestDevice(Camera *camera, Request *request)
> @@ -1461,6 +1472,8 @@ void RPiCameraData::freeBuffers()
>
> for (auto const stream : streams_)
> stream->releaseBuffers();
> +
> + reallocate_ = true;
> }
>
> void RPiCameraData::frameStarted(uint32_t sequence)
> --
> 2.25.1
>
More information about the libcamera-devel
mailing list