[libcamera-devel] [PATCH v2 5/8] android: CameraDevice: Return const shared_ptr& by camera()

[Hirokazu Honda]

On Thu, Mar 25, 2021 at 5:43 PM Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> Hi Hiro,
>
> On Wed, Mar 24, 2021 at 04:07:54PM +0900, Hirokazu Honda wrote:
> > CameraDevice::camera() originally returns shared_ptr. It is
> > mandatory to make a copy by calling camera() in this way. There
> > is no need of copying if a caller needs the reference of the
> > camera like const shared_ptr<Camera> cam = camera(). That is, it
> > is a caller that copying is required. This changes the return
> > type of camera() to const shared_ptr&, so that we are able to
> > reduce one redundant copy in the above case.
> >
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> As I see CameraDevice::camera() being only used in
> src/android/camera_hal_manager.cpp:   return cam == camera->camera();
>
> This makes indeed sense. It might not be the greatest API as it's easy
> to assume it gets a shared_ptr<> to share ownership.. Probably we
> should just return a Camera* ?
>

There is one more call.
FrameBufferAllocator takes shared_ptr.
allocator_ = std::make_unique<FrameBufferAllocator>(cameraDevice_->camera());

So this should not be std::shared_ptr<>&.

-Hiro

> This is an improvement anyway, so
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
>
> Thanks
>    j
>
> > ---
> >  src/android/camera_device.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 555b33e7..14e26b4d 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -43,7 +43,7 @@ public:
> >
> >       unsigned int id() const { return id_; }
> >       camera3_device_t *camera3Device() { return &camera3Device_; }
> > -     std::shared_ptr<libcamera::Camera> camera() const { return camera_; }
> > +     const std::shared_ptr<libcamera::Camera> &camera() const { return camera_; }
> >       libcamera::CameraConfiguration *cameraConfiguration() const
> >       {
> >               return config_.get();
> > --
> > 2.31.0.291.g576ba9dcdaf-goog
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel