| Message ID | 20231209022227.595197-1-pobrn@protonmail.com |
|---|---|
| State | Accepted |
| Commit | b1b798ef68ea84b02e986b8463442f3fad2ce269 |
| Headers | show |
| Series |
|
| Related | show |
Quoting Barnabás Pőcze via libcamera-devel (2023-12-09 02:22:28) > The vector in question is destroyed when the item in the `buffers_` map > is destroyed as a result of the `erase()` call. A vector's destructor > already does all the things that `clear()` does, > so calling it earlier is not needed. This looks correct to me. I guess it's a micro-optimisation, but it seems valid. Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > --- > src/libcamera/framebuffer_allocator.cpp | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp > index dabd9219..94389735 100644 > --- a/src/libcamera/framebuffer_allocator.cpp > +++ b/src/libcamera/framebuffer_allocator.cpp > @@ -121,8 +121,6 @@ int FrameBufferAllocator::free(Stream *stream) > if (iter == buffers_.end()) > return -EINVAL; > > - std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second; > - buffers.clear(); > buffers_.erase(iter); > > return 0; > -- > 2.43.0 > >
On Tue, Jan 09, 2024 at 01:11:19PM +0000, Kieran Bingham via libcamera-devel wrote: > Quoting Barnabás Pőcze via libcamera-devel (2023-12-09 02:22:28) > > The vector in question is destroyed when the item in the `buffers_` map > > is destroyed as a result of the `erase()` call. A vector's destructor > > already does all the things that `clear()` does, > > so calling it earlier is not needed. > > This looks correct to me. I guess it's a micro-optimisation, but it > seems valid. It may have been there before commit a4be7bb5ff4d4dce1fdc942a103f6360dad91f11 Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Date: Sun Jan 19 21:42:09 2020 +0200 libcamera: camera: Move private data members to private implementation to make sure the FrameBuffer instances get destroyed before freeing the underlying buffers. That commit changed the order of the operations and freed the underlying buffers first, apparently without causing issues. In any case, there's no need for a separate .clear() now. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > --- > > src/libcamera/framebuffer_allocator.cpp | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp > > index dabd9219..94389735 100644 > > --- a/src/libcamera/framebuffer_allocator.cpp > > +++ b/src/libcamera/framebuffer_allocator.cpp > > @@ -121,8 +121,6 @@ int FrameBufferAllocator::free(Stream *stream) > > if (iter == buffers_.end()) > > return -EINVAL; > > > > - std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second; > > - buffers.clear(); > > buffers_.erase(iter); > > > > return 0;
diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp index dabd9219..94389735 100644 --- a/src/libcamera/framebuffer_allocator.cpp +++ b/src/libcamera/framebuffer_allocator.cpp @@ -121,8 +121,6 @@ int FrameBufferAllocator::free(Stream *stream) if (iter == buffers_.end()) return -EINVAL; - std::vector<std::unique_ptr<FrameBuffer>> &buffers = iter->second; - buffers.clear(); buffers_.erase(iter); return 0;
The vector in question is destroyed when the item in the `buffers_` map is destroyed as a result of the `erase()` call. A vector's destructor already does all the things that `clear()` does, so calling it earlier is not needed. Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> --- src/libcamera/framebuffer_allocator.cpp | 2 -- 1 file changed, 2 deletions(-)