[libcamera-devel] [PATCH v4 1/2] android: CameraDevice: Add stop()

[Hirokazu Honda]

Hi Kieran,

On Wed, Mar 31, 2021 at 3:48 AM Kieran Bingham
<kieran.bingham at ideasonboard.com> wrote:
>
> Hi Hiro,
>
> On 30/03/2021 06:46, Hirokazu Honda wrote:
> > This adds CameraDevice::stop(), which cleans up the member
> > variables of CameraDevice. It is called in CameraDevice::close()
> > and CameraDevice::configureStreams().
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> >  src/android/camera_device.cpp | 19 +++++++++++--------
> >  src/android/camera_device.h   |  2 ++
> >  2 files changed, 13 insertions(+), 8 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index ae693664..14299429 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -657,11 +657,18 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
> >
> >  void CameraDevice::close()
> >  {
> > -     streams_.clear();
>
> Here CameraDevice::close() no longer clears the stream_, and it's not
> added anywhere else.
>
> Is this intentional?
>
> It seems like a functional change which is not mentioned by the commit
> message and stands out a bit.
>
> Is it handled automatically elsewhere already?
>

That's definitely my fault. I will add revert it on close() in the next patch.
Thanks for finding.

>
> > +     stop();
> > +
> > +     camera_->release();
> > +}
> > +
> > +void CameraDevice::stop()
> > +{
> > +     if (!running_)
> > +             return;
> >
> >       worker_.stop();
> >       camera_->stop();
> > -     camera_->release();
> >
> >       running_ = false;
> >  }
> > @@ -1545,12 +1552,8 @@ PixelFormat CameraDevice::toPixelFormat(int format) const
> >   */
> >  int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list)
> >  {
> > -     /* Before any configuration attempt, stop the camera if it's running. */
> > -     if (running_) {
> > -             worker_.stop();
> > -             camera_->stop();
> > -             running_ = false;
> > -     }
> > +     /* Before any configuration attempt, stop the camera. */
> > +     stop();
> >
> >       /*
> >        * Generate an empty configuration, and construct a StreamConfiguration
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 11bdfec8..39cf95ad 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -85,6 +85,8 @@ private:
> >               int androidFormat;
> >       };
> >
> > +     void stop();
> > +
> >       int initializeStreamConfigurations();
> >       std::vector<libcamera::Size>
> >       getYUVResolutions(libcamera::CameraConfiguration *cameraConfig,
> >
>
> --
> Regards
> --
> Kieran