| Message ID | 20220321102702.1753800-5-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Quoting Naushir Patuck via libcamera-devel (2022-03-21 10:27:01) > The original use of RPi::Stream::reset() was to clear the external flag state > and free/clear out the framebuffers for the stream. However, the latter is now > done through PipelineHandlerRPi::configure(). Rework > PipelineHandlerRPi::configure() to call RPi::Stream::setExternal() instead of > RPi::Stream::reset() to achieve the same thing. > > Repurpose RPi::Stream::reset() to instead reset the state of the buffer handling > logic, where all internally allocated buffers are put back into the queue of > available buffers. As such, rename the function to RPi::Stream::resetbuffers() > This resetbuffers() is now called from PipelineHandlerRPi::start(), allowing the /resetbuffers/resetBuffers/ twice above. But could be handled while applying. > pipeline handler to correctly deal with start()/stop()/start() sequences and > reusing the buffers allocated on the first 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> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++-- > src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 13 ++++++------- > src/libcamera/pipeline/raspberrypi/rpi_stream.h | 2 +- > 3 files changed, 12 insertions(+), 10 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 92043962494b..0ff6acdceb66 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -693,7 +693,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > /* Start by freeing all buffers and reset the Unicam and ISP stream states. */ > data->freeBuffers(); > for (auto const stream : data->streams_) > - stream->reset(); > + stream->setExternal(false); > > BayerFormat::Packing packing = BayerFormat::Packing::CSI2; > Size maxSize, sensorSize; > @@ -991,6 +991,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) > RPiCameraData *data = cameraData(camera); > int ret; > > + for (auto const stream : data->streams_) > + stream->resetBuffers(); > + > if (!data->buffersAllocated_) { > /* Allocate buffers for internal pipeline usage. */ > ret = prepareBuffers(camera); > @@ -1031,7 +1034,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); > > /* > - * Reset the delayed controls with the gain and exposure values set by > + * resetBuffers the delayed controls with the gain and exposure values set by Possibly a little too much of a match on the find/replace? Could easily be dropped while applying. > * the IPA. > */ > data->delayedCtrls_->reset(); > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > index f446e1ce66c7..7a93efaa29da 100644 > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > @@ -26,10 +26,12 @@ std::string Stream::name() const > return name_; > } > > -void Stream::reset() > +void Stream::resetBuffers() > { > - external_ = false; > - clearBuffers(); > + /* Add all internal buffers to the queue of usable buffers. */ > + availableBuffers_ = {}; I think this is ok, but I get a bit worried when I see blanket buffer state changes like that. > + for (auto const &buffer : internalBuffers_) > + availableBuffers_.push(buffer.get()); Is there any other action that is required to update the state of the buffer if it was currently in use when this gets called? (Currently I think the only call site is below so it's fine). With the above fixes (that can be fixed while applying if suitable) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > } > > void Stream::setExternal(bool external) > @@ -97,10 +99,7 @@ int Stream::prepareBuffers(unsigned int count) > > /* 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()); > + resetBuffers(); > } > > /* We must import all internal/external exported buffers. */ > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > index d6f49d34f8c8..c37f7e82eef6 100644 > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > @@ -45,7 +45,7 @@ public: > V4L2VideoDevice *dev() const; > std::string name() const; > bool isImporter() const; > - void reset(); > + void resetBuffers(); > > void setExternal(bool external); > bool isExternal() const; > -- > 2.25.1 >
Quoting Kieran Bingham (2022-03-21 11:51:37) > Quoting Naushir Patuck via libcamera-devel (2022-03-21 10:27:01) > > The original use of RPi::Stream::reset() was to clear the external flag state > > and free/clear out the framebuffers for the stream. However, the latter is now > > done through PipelineHandlerRPi::configure(). Rework > > PipelineHandlerRPi::configure() to call RPi::Stream::setExternal() instead of > > RPi::Stream::reset() to achieve the same thing. > > > > Repurpose RPi::Stream::reset() to instead reset the state of the buffer handling > > logic, where all internally allocated buffers are put back into the queue of > > available buffers. As such, rename the function to RPi::Stream::resetbuffers() > > This resetbuffers() is now called from PipelineHandlerRPi::start(), allowing the > > /resetbuffers/resetBuffers/ twice above. But could be handled while > applying. > > > > pipeline handler to correctly deal with start()/stop()/start() sequences and > > reusing the buffers allocated on the first 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> > > --- > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++-- > > src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 13 ++++++------- > > src/libcamera/pipeline/raspberrypi/rpi_stream.h | 2 +- > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > index 92043962494b..0ff6acdceb66 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -693,7 +693,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) > > /* Start by freeing all buffers and reset the Unicam and ISP stream states. */ > > data->freeBuffers(); > > for (auto const stream : data->streams_) > > - stream->reset(); > > + stream->setExternal(false); > > > > BayerFormat::Packing packing = BayerFormat::Packing::CSI2; > > Size maxSize, sensorSize; > > @@ -991,6 +991,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) > > RPiCameraData *data = cameraData(camera); > > int ret; > > > > + for (auto const stream : data->streams_) > > + stream->resetBuffers(); > > + > > if (!data->buffersAllocated_) { > > /* Allocate buffers for internal pipeline usage. */ > > ret = prepareBuffers(camera); > > @@ -1031,7 +1034,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) > > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); > > > > /* > > - * Reset the delayed controls with the gain and exposure values set by > > + * resetBuffers the delayed controls with the gain and exposure values set by > > Possibly a little too much of a match on the find/replace? > > Could easily be dropped while applying. > > > > * the IPA. > > */ > > data->delayedCtrls_->reset(); > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > index f446e1ce66c7..7a93efaa29da 100644 > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > @@ -26,10 +26,12 @@ std::string Stream::name() const > > return name_; > > } > > > > -void Stream::reset() > > +void Stream::resetBuffers() > > { > > - external_ = false; > > - clearBuffers(); > > + /* Add all internal buffers to the queue of usable buffers. */ > > + availableBuffers_ = {}; > > I think this is ok, but I get a bit worried when I see blanket buffer > state changes like that. > > > + for (auto const &buffer : internalBuffers_) > > + availableBuffers_.push(buffer.get()); > > Is there any other action that is required to update the state of the > buffer if it was currently in use when this gets called? (Currently I > think the only call site is below so it's fine). Aha, it's not the only call site, there's the one up in start() as well of course. So at start() time, is there any possiblity that some buffer isn't already in availableBuffers_ ? and if not ... why not ? Is there some worry about buffer state on calls to stop() not moving buffers into the availableBuffers_ list? -- Kieran > With the above fixes (that can be fixed while applying if suitable) > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > } > > > > void Stream::setExternal(bool external) > > @@ -97,10 +99,7 @@ int Stream::prepareBuffers(unsigned int count) > > > > /* 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()); > > + resetBuffers(); > > } > > > > /* We must import all internal/external exported buffers. */ > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > index d6f49d34f8c8..c37f7e82eef6 100644 > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > @@ -45,7 +45,7 @@ public: > > V4L2VideoDevice *dev() const; > > std::string name() const; > > bool isImporter() const; > > - void reset(); > > + void resetBuffers(); > > > > void setExternal(bool external); > > bool isExternal() const; > > -- > > 2.25.1 > >
Hi Kieran, On Mon, 21 Mar 2022 at 11:55, Kieran Bingham < kieran.bingham@ideasonboard.com> wrote: > Quoting Kieran Bingham (2022-03-21 11:51:37) > > Quoting Naushir Patuck via libcamera-devel (2022-03-21 10:27:01) > > > The original use of RPi::Stream::reset() was to clear the external > flag state > > > and free/clear out the framebuffers for the stream. However, the > latter is now > > > done through PipelineHandlerRPi::configure(). Rework > > > PipelineHandlerRPi::configure() to call RPi::Stream::setExternal() > instead of > > > RPi::Stream::reset() to achieve the same thing. > > > > > > Repurpose RPi::Stream::reset() to instead reset the state of the > buffer handling > > > logic, where all internally allocated buffers are put back into the > queue of > > > available buffers. As such, rename the function to > RPi::Stream::resetbuffers() > > > This resetbuffers() is now called from PipelineHandlerRPi::start(), > allowing the > > > > /resetbuffers/resetBuffers/ twice above. But could be handled while > > applying. > > > > > > > pipeline handler to correctly deal with start()/stop()/start() > sequences and > > > reusing the buffers allocated on the first 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> > > > --- > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++-- > > > src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 13 ++++++------- > > > src/libcamera/pipeline/raspberrypi/rpi_stream.h | 2 +- > > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > index 92043962494b..0ff6acdceb66 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > @@ -693,7 +693,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > CameraConfiguration *config) > > > /* Start by freeing all buffers and reset the Unicam and ISP > stream states. */ > > > data->freeBuffers(); > > > for (auto const stream : data->streams_) > > > - stream->reset(); > > > + stream->setExternal(false); > > > > > > BayerFormat::Packing packing = BayerFormat::Packing::CSI2; > > > Size maxSize, sensorSize; > > > @@ -991,6 +991,9 @@ int PipelineHandlerRPi::start(Camera *camera, > const ControlList *controls) > > > RPiCameraData *data = cameraData(camera); > > > int ret; > > > > > > + for (auto const stream : data->streams_) > > > + stream->resetBuffers(); > > > + > > > if (!data->buffersAllocated_) { > > > /* Allocate buffers for internal pipeline usage. */ > > > ret = prepareBuffers(camera); > > > @@ -1031,7 +1034,7 @@ int PipelineHandlerRPi::start(Camera *camera, > const ControlList *controls) > > > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); > > > > > > /* > > > - * Reset the delayed controls with the gain and exposure > values set by > > > + * resetBuffers the delayed controls with the gain and > exposure values set by > > > > Possibly a little too much of a match on the find/replace? > > > > Could easily be dropped while applying. > :-( Sorry, my brain is functioning like it's still Sunday this morning. I'll fix this up and post an update. > > > > > > > * the IPA. > > > */ > > > data->delayedCtrls_->reset(); > > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > index f446e1ce66c7..7a93efaa29da 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > @@ -26,10 +26,12 @@ std::string Stream::name() const > > > return name_; > > > } > > > > > > -void Stream::reset() > > > +void Stream::resetBuffers() > > > { > > > - external_ = false; > > > - clearBuffers(); > > > + /* Add all internal buffers to the queue of usable buffers. */ > > > + availableBuffers_ = {}; > > > > I think this is ok, but I get a bit worried when I see blanket buffer > > state changes like that. > > > > > + for (auto const &buffer : internalBuffers_) > > > + availableBuffers_.push(buffer.get()); > > > > Is there any other action that is required to update the state of the > > buffer if it was currently in use when this gets called? (Currently I > > think the only call site is below so it's fine). > > Aha, it's not the only call site, there's the one up in start() as well > of course. > > So at start() time, is there any possiblity that some buffer isn't > already in availableBuffers_ ? and if not ... why not ? > At start time, all internal buffers would have been allocated, so any/every buffer will be listed in availableBuffers_. > Is there some worry about buffer state on calls to stop() not moving > buffers into the availableBuffers_ list? > I think this is ok, as the resetBuffers() call on start() will (as it says on the tin) reset all buffer states and mark them as available again, if that makes sense. Regards, Naush > > -- > Kieran > > > > With the above fixes (that can be fixed while applying if suitable) > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > } > > > > > > void Stream::setExternal(bool external) > > > @@ -97,10 +99,7 @@ int Stream::prepareBuffers(unsigned int count) > > > > > > /* 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()); > > > + resetBuffers(); > > > } > > > > > > /* We must import all internal/external exported > buffers. */ > > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > index d6f49d34f8c8..c37f7e82eef6 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > @@ -45,7 +45,7 @@ public: > > > V4L2VideoDevice *dev() const; > > > std::string name() const; > > > bool isImporter() const; > > > - void reset(); > > > + void resetBuffers(); > > > > > > void setExternal(bool external); > > > bool isExternal() const; > > > -- > > > 2.25.1 > > > >
Quoting Naushir Patuck (2022-03-21 12:24:01) > Hi Kieran, > > On Mon, 21 Mar 2022 at 11:55, Kieran Bingham < > kieran.bingham@ideasonboard.com> wrote: > > > Quoting Kieran Bingham (2022-03-21 11:51:37) > > > Quoting Naushir Patuck via libcamera-devel (2022-03-21 10:27:01) > > > > The original use of RPi::Stream::reset() was to clear the external > > flag state > > > > and free/clear out the framebuffers for the stream. However, the > > latter is now > > > > done through PipelineHandlerRPi::configure(). Rework > > > > PipelineHandlerRPi::configure() to call RPi::Stream::setExternal() > > instead of > > > > RPi::Stream::reset() to achieve the same thing. > > > > > > > > Repurpose RPi::Stream::reset() to instead reset the state of the > > buffer handling > > > > logic, where all internally allocated buffers are put back into the > > queue of > > > > available buffers. As such, rename the function to > > RPi::Stream::resetbuffers() > > > > This resetbuffers() is now called from PipelineHandlerRPi::start(), > > allowing the > > > > > > /resetbuffers/resetBuffers/ twice above. But could be handled while > > > applying. > > > > > > > > > > pipeline handler to correctly deal with start()/stop()/start() > > sequences and > > > > reusing the buffers allocated on the first 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> > > > > --- > > > > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++-- > > > > src/libcamera/pipeline/raspberrypi/rpi_stream.cpp | 13 ++++++------- > > > > src/libcamera/pipeline/raspberrypi/rpi_stream.h | 2 +- > > > > 3 files changed, 12 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > index 92043962494b..0ff6acdceb66 100644 > > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > > @@ -693,7 +693,7 @@ int PipelineHandlerRPi::configure(Camera *camera, > > CameraConfiguration *config) > > > > /* Start by freeing all buffers and reset the Unicam and ISP > > stream states. */ > > > > data->freeBuffers(); > > > > for (auto const stream : data->streams_) > > > > - stream->reset(); > > > > + stream->setExternal(false); > > > > > > > > BayerFormat::Packing packing = BayerFormat::Packing::CSI2; > > > > Size maxSize, sensorSize; > > > > @@ -991,6 +991,9 @@ int PipelineHandlerRPi::start(Camera *camera, > > const ControlList *controls) > > > > RPiCameraData *data = cameraData(camera); > > > > int ret; > > > > > > > > + for (auto const stream : data->streams_) > > > > + stream->resetBuffers(); > > > > + > > > > if (!data->buffersAllocated_) { > > > > /* Allocate buffers for internal pipeline usage. */ > > > > ret = prepareBuffers(camera); > > > > @@ -1031,7 +1034,7 @@ int PipelineHandlerRPi::start(Camera *camera, > > const ControlList *controls) > > > > data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); > > > > > > > > /* > > > > - * Reset the delayed controls with the gain and exposure > > values set by > > > > + * resetBuffers the delayed controls with the gain and > > exposure values set by > > > > > > Possibly a little too much of a match on the find/replace? > > > > > > Could easily be dropped while applying. > > > > :-( > > Sorry, my brain is functioning like it's still Sunday this morning. I'll > fix this up and > post an update. > > > > > > > > > > > > * the IPA. > > > > */ > > > > data->delayedCtrls_->reset(); > > > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > > index f446e1ce66c7..7a93efaa29da 100644 > > > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp > > > > @@ -26,10 +26,12 @@ std::string Stream::name() const > > > > return name_; > > > > } > > > > > > > > -void Stream::reset() > > > > +void Stream::resetBuffers() > > > > { > > > > - external_ = false; > > > > - clearBuffers(); > > > > + /* Add all internal buffers to the queue of usable buffers. */ > > > > + availableBuffers_ = {}; > > > > > > I think this is ok, but I get a bit worried when I see blanket buffer > > > state changes like that. > > > > > > > + for (auto const &buffer : internalBuffers_) > > > > + availableBuffers_.push(buffer.get()); > > > > > > Is there any other action that is required to update the state of the > > > buffer if it was currently in use when this gets called? (Currently I > > > think the only call site is below so it's fine). > > > > Aha, it's not the only call site, there's the one up in start() as well > > of course. > > > > So at start() time, is there any possiblity that some buffer isn't > > already in availableBuffers_ ? and if not ... why not ? > > > > At start time, all internal buffers would have been allocated, so any/every > buffer will be listed in availableBuffers_. > > > > Is there some worry about buffer state on calls to stop() not moving > > buffers into the availableBuffers_ list? > > > > I think this is ok, as the resetBuffers() call on start() will (as it says > on the tin) > reset all buffer states and mark them as available again, if that makes > sense. Yes, that makes sense, but I would call this the 'shotgun defence', and shoots everything instead of just the target required, so I wonder if there is a corresponding ASSERT() (or non-fatal print) that is worth adding to catch a case when buffers were not in the expected state. The clear and re-add will put them all in the right list - but if that was preceeded by an unknown state, it can be worth noting it. Not essential though, it's only if this is a potential for identifying buffer state leaks or otherwise catching when buffers didn't go through the 'normal/expected' flow. -- Kieran > Regards, > Naush > > > > > > -- > > Kieran > > > > > > > With the above fixes (that can be fixed while applying if suitable) > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > > > > > } > > > > > > > > void Stream::setExternal(bool external) > > > > @@ -97,10 +99,7 @@ int Stream::prepareBuffers(unsigned int count) > > > > > > > > /* 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()); > > > > + resetBuffers(); > > > > } > > > > > > > > /* We must import all internal/external exported > > buffers. */ > > > > diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > > index d6f49d34f8c8..c37f7e82eef6 100644 > > > > --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > > +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h > > > > @@ -45,7 +45,7 @@ public: > > > > V4L2VideoDevice *dev() const; > > > > std::string name() const; > > > > bool isImporter() const; > > > > - void reset(); > > > > + void resetBuffers(); > > > > > > > > void setExternal(bool external); > > > > bool isExternal() const; > > > > -- > > > > 2.25.1 > > > > > >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 92043962494b..0ff6acdceb66 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -693,7 +693,7 @@ int PipelineHandlerRPi::configure(Camera *camera, CameraConfiguration *config) /* Start by freeing all buffers and reset the Unicam and ISP stream states. */ data->freeBuffers(); for (auto const stream : data->streams_) - stream->reset(); + stream->setExternal(false); BayerFormat::Packing packing = BayerFormat::Packing::CSI2; Size maxSize, sensorSize; @@ -991,6 +991,9 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) RPiCameraData *data = cameraData(camera); int ret; + for (auto const stream : data->streams_) + stream->resetBuffers(); + if (!data->buffersAllocated_) { /* Allocate buffers for internal pipeline usage. */ ret = prepareBuffers(camera); @@ -1031,7 +1034,7 @@ int PipelineHandlerRPi::start(Camera *camera, const ControlList *controls) data->unicam_[Unicam::Image].dev()->setFrameStartEnabled(true); /* - * Reset the delayed controls with the gain and exposure values set by + * resetBuffers the delayed controls with the gain and exposure values set by * the IPA. */ data->delayedCtrls_->reset(); diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp index f446e1ce66c7..7a93efaa29da 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.cpp @@ -26,10 +26,12 @@ std::string Stream::name() const return name_; } -void Stream::reset() +void Stream::resetBuffers() { - external_ = false; - clearBuffers(); + /* Add all internal buffers to the queue of usable buffers. */ + availableBuffers_ = {}; + for (auto const &buffer : internalBuffers_) + availableBuffers_.push(buffer.get()); } void Stream::setExternal(bool external) @@ -97,10 +99,7 @@ int Stream::prepareBuffers(unsigned int count) /* 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()); + resetBuffers(); } /* We must import all internal/external exported buffers. */ diff --git a/src/libcamera/pipeline/raspberrypi/rpi_stream.h b/src/libcamera/pipeline/raspberrypi/rpi_stream.h index d6f49d34f8c8..c37f7e82eef6 100644 --- a/src/libcamera/pipeline/raspberrypi/rpi_stream.h +++ b/src/libcamera/pipeline/raspberrypi/rpi_stream.h @@ -45,7 +45,7 @@ public: V4L2VideoDevice *dev() const; std::string name() const; bool isImporter() const; - void reset(); + void resetBuffers(); void setExternal(bool external); bool isExternal() const;