| Message ID | 20220321102702.1753800-4-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Mon, Mar 21, 2022 at 10:27:00AM +0000, Naushir Patuck via libcamera-devel wrote: > 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@raspberrypi.com> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> > Tested-by: David Plowman <david.plowman@raspberrypi.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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..92043962494b 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), buffersAllocated_(false), ispOutputCount_(0) > { > } > > + ~RPiCameraData() > + { > + freeBuffers(); > + } > + > void freeBuffers(); > void frameStarted(uint32_t sequence); > > @@ -280,6 +285,9 @@ public: > */ > std::optional<int32_t> notifyGainsUnity_; > > + /* Have internal buffers been allocated? */ > + bool buffersAllocated_; > + > 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->buffersAllocated_) { > + /* 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->buffersAllocated_ = true; > } > > /* 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(); > + > + buffersAllocated_ = false; > } > > void RPiCameraData::frameStarted(uint32_t sequence)
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 2281b43fc3ac..92043962494b 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), buffersAllocated_(false), ispOutputCount_(0) { } + ~RPiCameraData() + { + freeBuffers(); + } + void freeBuffers(); void frameStarted(uint32_t sequence); @@ -280,6 +285,9 @@ public: */ std::optional<int32_t> notifyGainsUnity_; + /* Have internal buffers been allocated? */ + bool buffersAllocated_; + 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->buffersAllocated_) { + /* 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->buffersAllocated_ = true; } /* 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(); + + buffersAllocated_ = false; } void RPiCameraData::frameStarted(uint32_t sequence)