[libcamera-devel] [PATCH 3/8] libcamera: frame_buffer_allocator: Add clear()

[Hirokazu Honda]

On Thu, Sep 10, 2020 at 8:19 PM Niklas Söderlund
<niklas.soderlund at ragnatech.se> wrote:
>
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2020-09-09 17:54:52 +0200, Jacopo Mondi wrote:
> > Add a clear() method to the FrameBufferAllocator class that
> > frees all the buffers previously reserved by the allocator.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > ---
> >  include/libcamera/framebuffer_allocator.h | 1 +
> >  src/libcamera/framebuffer_allocator.cpp   | 8 ++++++++
> >  2 files changed, 9 insertions(+)
> >
> > diff --git a/include/libcamera/framebuffer_allocator.h b/include/libcamera/framebuffer_allocator.h
> > index 78f1353964eb..2a4d538a0cb2 100644
> > --- a/include/libcamera/framebuffer_allocator.h
> > +++ b/include/libcamera/framebuffer_allocator.h
> > @@ -28,6 +28,7 @@ public:
> >
> >       int allocate(Stream *stream);
> >       int free(Stream *stream);
> > +     void clear();
> >
> >       bool allocated() const { return !buffers_.empty(); }
> >       const std::vector<std::unique_ptr<FrameBuffer>> &buffers(Stream *stream) const;
> > diff --git a/src/libcamera/framebuffer_allocator.cpp b/src/libcamera/framebuffer_allocator.cpp
> > index 2fbba37a1b0b..7ed80011c845 100644
> > --- a/src/libcamera/framebuffer_allocator.cpp
> > +++ b/src/libcamera/framebuffer_allocator.cpp
> > @@ -125,6 +125,14 @@ int FrameBufferAllocator::free(Stream *stream)
> >       return 0;
> >  }
> >
> > +/**
> > + * \brief Free all the buffers previously allocated
> > + */
> > +void FrameBufferAllocator::clear()
> > +{
> > +     buffers_.clear();
> > +}
> > +

This is the first time I looked at FrameBufferAllocator.
Those comments are not so related to this patch.
I didn't get the difference between free() and clear() from the name.
I would rename free() to erase(). If it sounds good to you, can I ask
you to do it later?
Besides, clear() in dtor seems to be unnecessary.

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

> >  /**
> >   * \fn FrameBufferAllocator::allocated()
> >   * \brief Check if the allocator has allocated buffers for any stream
> > --
> > 2.28.0
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel