Skip to content

Commit

Permalink
[camera] Add protection around DroidMediaCamera pointer in CameraList…
Browse files Browse the repository at this point in the history
…ener

When destroying DroidMediaCamera, CameraListener held inside an ongoing
callback can outlive the destroying instance. To prevent it from
accessing DroidMediaCamera after being destroyed, one needs to remove
the reference to DroidMediaCamera inside the listener before deleting
it.

As this removal happens in a different thread then callbacks, a mutex is
required to prevent race condition. Also, just to be safe, remove the
listener from the Android Camera instance as well.
  • Loading branch information
peat-psuwit committed Mar 4, 2020
1 parent 52143f2 commit a2d02f8
Showing 1 changed file with 39 additions and 1 deletion.
40 changes: 39 additions & 1 deletion droidmediacamera.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ namespace android {

extern "C" {

class CameraListener;

struct _DroidMediaCameraRecordingData
{
android::sp<android::IMemory> mem;
Expand All @@ -89,6 +91,7 @@ struct _DroidMediaCamera
android::sp<DroidMediaBufferQueue> m_queue;
DroidMediaCameraCallbacks m_cb;
void *m_cb_data;
android::sp<CameraListener> listener;
};

class CameraListener : public android::CameraListener {
Expand All @@ -100,6 +103,10 @@ class CameraListener : public android::CameraListener {

void notify(int32_t msgType, int32_t ext1, int32_t ext2)
{
android::Mutex::Autolock _l(m_camLock);
if (!m_cam)
return;

switch (msgType) {
case CAMERA_MSG_SHUTTER:
if (m_cam->m_cb.shutter_cb) {
Expand Down Expand Up @@ -139,6 +146,10 @@ class CameraListener : public android::CameraListener {
void postData(int32_t msgType, const android::sp<android::IMemory>& dataPtr,
camera_frame_metadata_t *metadata)
{
android::Mutex::Autolock _l(m_camLock);
if (!m_cam)
return;

int32_t dataMsgType = msgType & ~CAMERA_MSG_PREVIEW_METADATA;
DroidMediaData mem;

Expand Down Expand Up @@ -200,6 +211,10 @@ class CameraListener : public android::CameraListener {

void postDataTimestamp(nsecs_t timestamp, int32_t msgType, const android::sp<android::IMemory>& dataPtr)
{
android::Mutex::Autolock _l(m_camLock);
if (!m_cam)
return;

switch (msgType) {
case CAMERA_MSG_VIDEO_FRAME:
if (m_cam->m_cb.video_frame_cb) {
Expand Down Expand Up @@ -236,6 +251,13 @@ class CameraListener : public android::CameraListener {

void sendPreviewMetadata(camera_frame_metadata_t *metadata)
{
{
// Opportunistic return before processing the result without a lock.
android::Mutex::Autolock _l(m_camLock);
if (!m_cam)
return;
}

android::Vector<DroidMediaCameraFace> faces;

for (int x = 0; x < metadata->number_of_faces; x++) {
Expand All @@ -256,11 +278,22 @@ class CameraListener : public android::CameraListener {
faces.push_back(face);
}

android::Mutex::Autolock _l(m_camLock);
if (!m_cam)
return;

m_cam->m_cb.preview_metadata_cb(m_cam->m_cb_data, faces.array(), faces.size());
}

void disconnectFromDroidCam()
{
android::Mutex::Autolock _l(m_camLock);
m_cam = NULL;
}

private:
DroidMediaCamera *m_cam;
android::Mutex m_camLock;
};

DroidMediaBufferQueue *droid_media_camera_get_buffer_queue (DroidMediaCamera *camera)
Expand Down Expand Up @@ -339,7 +372,8 @@ DroidMediaCamera *droid_media_camera_connect(int camera_number)

cam->m_queue->attachToCamera(cam->m_camera);

cam->m_camera->setListener(new CameraListener(cam));
cam->listener = new CameraListener(cam);
cam->m_camera->setListener(cam->listener);

return cam;
}
Expand All @@ -350,8 +384,12 @@ bool droid_media_camera_reconnect(DroidMediaCamera *camera) {

void droid_media_camera_disconnect(DroidMediaCamera *camera)
{
camera->m_camera->setListener(NULL);
camera->m_camera->disconnect();

// Ensure that any remaining references won't call to to-be-deleted camera.
camera->listener->disconnectFromDroidCam();

camera->m_queue->setCallbacks(0, 0);

delete camera;
Expand Down

0 comments on commit a2d02f8

Please sign in to comment.