[libcamera-devel] [PATCH v3 4/8] android: camera_device: Clear allocator at configureStream

[Jacopo Mondi]

Hi Laurent,

On Tue, Sep 29, 2020 at 04:53:31AM +0300, Laurent Pinchart wrote:
> On Tue, Sep 29, 2020 at 04:51:32AM +0300, Laurent Pinchart wrote:
> > Hi Jacopo,
> >
> > Thank you for the patch.
> >
> > On Tue, Sep 22, 2020 at 11:47:34AM +0200, Jacopo Mondi wrote:
> > > The configureStream operation might be called by the Android framework
> > > in two successive capture session without going through a close().
> >
> > With s/session/sessions/,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> Not necessarily for this patch, but shouldn't we also free buffers (and
> probably clear streams_ too) in CameraDevice::close() ?

I guess it doesn't hurt, indeed. When the camera is closed it's
indeed relevant to release all the allocated buffers, even if any new
allocation happens at configureStream() time, where we already call
freeAll().

Regarding streams_, being that internal memory released at
configureStream() time before any other CameraStream is created, I
think it's less relevant... I can make a separate patch to do so on
top of this one.


>
> > > Clear all the allocated buffers before configuring the camera streams.
> > >
> > > Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > > Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  src/android/camera_device.cpp | 7 ++++---
> > >  1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > index 98b8159ebd8c..42fb9ea4e113 100644
> > > --- a/src/android/camera_device.cpp
> > > +++ b/src/android/camera_device.cpp
> > > @@ -1182,12 +1182,13 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> > >  	}
> > >
> > >  	/*
> > > -	 * Clear and remove any existing configuration from previous calls, and
> > > -	 * ensure the required entries are available without further
> > > -	 * reallocation.
> > > +	 * Clear and remove any existing configuration and memory allocated from
> > > +	 * previous calls, and ensure the required entries are available without
> > > +	 * further reallocation.
> > >  	 */
> > >  	streams_.clear();
> > >  	streams_.reserve(stream_list->num_streams);
> > > +	allocator_.clear();
> > >
> > >  	/* First handle all non-MJPEG streams. */
> > >  	camera3_stream_t *jpegStream = nullptr;
>
> --
> Regards,
>
> Laurent Pinchart