<div dir='auto'><div>Hi Laurent,<br><div class="gmail_extra"><br><div class="gmail_quote">On Aug 23, 2020 11:59 PM, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr">CameraDevice instances are managed through shared pointers to support
<br>
hptplug. No reference is however taken on</p></blockquote></div></div></div><div dir="auto">s/hptplug/hotplug/</div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><p dir="ltr"> the device when opened by the
<br>
camera service, which could cause an open device to be deleted if
<br>
unplugged, leading to a crash when the device is next accessed.
<br>

<br>
Fix this by taking a reference when opening a device, and releasing it
<br>
at close time. The reference is stored in the CameraDevice instance
<br>
itself, which is more efficient than storing it in a container in the
<br>
CameraHalManager as that would require lookup operations.
<br>

<br>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
<br>
---
<br>
 src/android/camera_device.cpp | 12 ++++++++++++
<br>
 src/android/camera_device.h   |  4 +++-
<br>
 2 files changed, 15 insertions(+), 1 deletion(-)
<br>

<br>
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
<br>
index de6f86f7cb9b..52468be70781 100644
<br>
--- a/src/android/camera_device.cpp
<br>
+++ b/src/android/camera_device.cpp
<br>
@@ -459,6 +459,12 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
<br>
        camera3Device_.ops = &hal_dev_ops;
<br>
        camera3Device_.priv = this;
<br>
 
<br>
+       /*
<br>
+        * Hold a reference to ourselves, to protect against deletion if the
<br>
+        * camera is unplugged before being closed.
<br>
+        */
<br>
+       self_ = shared_from_this();
<br>
+
<br>
        return 0;
<br>
 }
<br>
 
<br>
@@ -468,6 +474,12 @@ void CameraDevice::close()
<br>
        camera_->release();
<br>
 
<br>
        running_ = false;
<br>
+
<br>
+       /*
<br>
+        * Drop our self reference. From this point the CameraDevice may get
<br>
+        * deleted at any time.
<br>
+        */
<br>
+       self_.reset();
<br>
 }
<br>
 
<br>
 void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks)
<br>
diff --git a/src/android/camera_device.h b/src/android/camera_device.h
<br>
index 3934f194f1b5..fd991c76b90d 100644
<br>
--- a/src/android/camera_device.h
<br>
+++ b/src/android/camera_device.h
<br>
@@ -44,7 +44,8 @@ struct CameraStream {
<br>
        Encoder *jpeg;
<br>
 };
<br>
 
<br>
-class CameraDevice : protected libcamera::Loggable
<br>
+class CameraDevice : public std::enable_shared_from_this<CameraDevice>,
<br>
+                    protected libcamera::Loggable
<br>
 {
<br>
 public:
<br>
        static std::shared_ptr<CameraDevice> create(unsigned int id,
<br>
@@ -104,6 +105,7 @@ private:
<br>
 
<br>
        unsigned int id_;
<br>
        camera3_device_t camera3Device_;
<br>
+       std::shared_ptr<CameraDevice> self_;
<br>
 
<br>
        bool running_;
<br>
        std::shared_ptr<libcamera::Camera> camera_;
<br>
-- 
<br>
Regards,
<br>

<br>
Laurent Pinchart
<br>

<br>
</p>
</blockquote></div>Is this the patch after testing behaviour of a stream-able camera(probably non- UVC) on CrOS with master? </div><div class="gmail_extra" dir="auto"><br></div><div class="gmail_extra" dir="auto">I am not sure what this fix really achieves, part of the problem is because of my build/test setup :(. I believe right now, the cros_camera_service exits("sledge hammer") when a ongoing streaming camera is unplugged, hence I am not able to follow if ::Close() is called anywhere in hot unplug path. By that extension, I am not sure if at all we are going to drop the manual ref we take, as per this patch.</div></div></div>