| Message ID | 20221109104940.5799-1-david.plowman@raspberrypi.com |
|---|---|
| State | Accepted |
| Commit | 75e7befb1667f620410f0f15a10eccb32d7df66d |
| Headers | show |
| Series |
|
| Related | show |
Quoting David Plowman via libcamera-devel (2022-11-09 10:49:40) > This reverts commit 30d704732badc675f72fe73d14749669cb645c23. > > It turns out that this commit causes some regressions and is in fact > unnecessary because the related commit "libcamera: v4l2_videodevice: > Guard against releasing unallocated buffers" > (a2bdff6d0b67475492ac7cf9318866b6d89a28fd) fixes the problem > completely (if the buffers were never allocated, the video device > avoids trying to free them even if the pipeline handler asks). > > The reason for the regressions is that in this new (broken) scheme we > would never call clearBuffers() on all the streams if the internal > buffers were never allocated (i.e. buffersAllocated_ is never > set). This causes the stream's bufferMap_ list to get longer and > longer if there are multiple back-to-back calls to configure, and > dev_->importBuffers() will ultimately to fail. > > So either we need to think more carefully about how to stop the > pipeline handler from freeing buffers that it doesn't own, or we just > leave it as the other commit resolves the problem on its own. In the > interim, simply reverting this commit certainly seems like the best > solution. > > Fixes: 30d704732bad ("pipeline: raspberrypi: Do not unconditionally free buffers on close") > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> No harm with a revert if it's not right. Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > index 1b599fcc..343f8cb2 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1506,9 +1506,6 @@ void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffer > > void RPiCameraData::freeBuffers() > { > - if (!buffersAllocated_) > - return; > - > if (ipa_) { > /* > * Copy the buffer ids from the unordered_set to a vector to > -- > 2.30.2 >
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 1b599fcc..343f8cb2 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1506,9 +1506,6 @@ void PipelineHandlerRPi::mapBuffers(Camera *camera, const RPi::BufferMap &buffer void RPiCameraData::freeBuffers() { - if (!buffersAllocated_) - return; - if (ipa_) { /* * Copy the buffer ids from the unordered_set to a vector to
This reverts commit 30d704732badc675f72fe73d14749669cb645c23. It turns out that this commit causes some regressions and is in fact unnecessary because the related commit "libcamera: v4l2_videodevice: Guard against releasing unallocated buffers" (a2bdff6d0b67475492ac7cf9318866b6d89a28fd) fixes the problem completely (if the buffers were never allocated, the video device avoids trying to free them even if the pipeline handler asks). The reason for the regressions is that in this new (broken) scheme we would never call clearBuffers() on all the streams if the internal buffers were never allocated (i.e. buffersAllocated_ is never set). This causes the stream's bufferMap_ list to get longer and longer if there are multiple back-to-back calls to configure, and dev_->importBuffers() will ultimately to fail. So either we need to think more carefully about how to stop the pipeline handler from freeing buffers that it doesn't own, or we just leave it as the other commit resolves the problem on its own. In the interim, simply reverting this commit certainly seems like the best solution. Fixes: 30d704732bad ("pipeline: raspberrypi: Do not unconditionally free buffers on close") Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Reviewed-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 3 --- 1 file changed, 3 deletions(-)