Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 0bb5e09f authored by Jan Sebechlebsky's avatar Jan Sebechlebsky
Browse files

Fix crash when virtual camera is unregistered during active session. 

Store weak_ptr to VirtualCameraDevice instance in VirtualCameraSession
to detect when the camera was removed as well as to make sure
it's not destroyed when VirtualCameraSession accesses
VirtualCameraDevice.

Bug: 301023410
Test: atest VirtualCameraTest (not closing the session)
Test: atest virtual_camera_tests

Change-Id: Ie87c91f6872b9241dddf64f1a4f1991eb54d3cda
parent fd378c3b

services/camera/virtualcamera/VirtualCameraDevice.cc

+8 −1
Original line number Diff line number Diff line
@@ -322,7 +322,7 @@ ndk::ScopedAStatus VirtualCameraDevice::open( 
  ALOGV("%s", __func__);



  *_aidl_return = ndk::SharedRefBase::make<VirtualCameraSession>(

      *this, in_callback, mVirtualCameraClientCallback);

      sharedFromThis(), in_callback, mVirtualCameraClientCallback);



  return ndk::ScopedAStatus::ok();

};
@@ -367,6 +367,13 @@ std::string VirtualCameraDevice::getCameraName() const { 
  return std::string(kDevicePathPrefix) + std::to_string(mCameraId);

}



std::shared_ptr<VirtualCameraDevice> VirtualCameraDevice::sharedFromThis() {

  // SharedRefBase which BnCameraDevice inherits from breaks

  // std::enable_shared_from_this. This is recommended replacement for

  // shared_from_this() per documentation in binder_interface_utils.h.

  return ref<VirtualCameraDevice>();

}



}  // namespace virtualcamera

}  // namespace companion

}  // namespace android

services/camera/virtualcamera/VirtualCameraDevice.h

+2 −0
Original line number Diff line number Diff line
@@ -98,6 +98,8 @@ class VirtualCameraDevice 
  uint32_t getCameraId() const { return mCameraId; }



 private:

  std::shared_ptr<VirtualCameraDevice> sharedFromThis();



  const uint32_t mCameraId;

  const std::shared_ptr<

      ::aidl::android::companion::virtualcamera::IVirtualCameraCallback>

services/camera/virtualcamera/VirtualCameraSession.cc

+8 −2
Original line number Diff line number Diff line
@@ -153,7 +153,7 @@ HalStream getHalStream(const Stream& stream) { 
}  // namespace



VirtualCameraSession::VirtualCameraSession(

    VirtualCameraDevice& cameraDevice,

    std::shared_ptr<VirtualCameraDevice> cameraDevice,

    std::shared_ptr<ICameraDeviceCallback> cameraDeviceCallback,

    std::shared_ptr<IVirtualCameraCallback> virtualCameraClientCallback)

    : mCameraDevice(cameraDevice),
@@ -201,6 +201,12 @@ ndk::ScopedAStatus VirtualCameraSession::configureStreams( 
    return cameraStatus(Status::ILLEGAL_ARGUMENT);

  }



  std::shared_ptr<VirtualCameraDevice> virtualCamera = mCameraDevice.lock();

  if (virtualCamera == nullptr) {

    ALOGW("%s: configure called on already unregistered camera", __func__);

    return cameraStatus(Status::CAMERA_DISCONNECTED);

  }



  mSessionContext.removeStreamsNotInStreamConfiguration(

      in_requestedConfiguration);
@@ -213,7 +219,7 @@ ndk::ScopedAStatus VirtualCameraSession::configureStreams( 
  int inputWidth;

  int inputHeight;



  if (!mCameraDevice.isStreamCombinationSupported(in_requestedConfiguration)) {

  if (!virtualCamera->isStreamCombinationSupported(in_requestedConfiguration)) {

    ALOGE("%s: Requested stream configuration is not supported", __func__);

    return cameraStatus(Status::ILLEGAL_ARGUMENT);

  }

services/camera/virtualcamera/VirtualCameraSession.h

+3 −2
Original line number Diff line number Diff line
@@ -17,6 +17,7 @@ 
#ifndef ANDROID_COMPANION_VIRTUALCAMERA_VIRTUALCAMERASESSION_H

#define ANDROID_COMPANION_VIRTUALCAMERA_VIRTUALCAMERASESSION_H



#include <atomic>

#include <memory>

#include <set>
@@ -46,7 +47,7 @@ class VirtualCameraSession 
  // When virtualCameraClientCallback is null, the input surface will be filled

  // with test pattern.

  VirtualCameraSession(

      VirtualCameraDevice& mCameraDevice,

      std::shared_ptr<VirtualCameraDevice> mCameraDevice,

      std::shared_ptr<

          ::aidl::android::hardware::camera::device::ICameraDeviceCallback>

          cameraDeviceCallback,
@@ -116,7 +117,7 @@ class VirtualCameraSession 
      const ::aidl::android::hardware::camera::device::CaptureRequest& request)

      EXCLUDES(mLock);



  VirtualCameraDevice& mCameraDevice;

  std::weak_ptr<VirtualCameraDevice> mCameraDevice;



  mutable std::mutex mLock;

services/camera/virtualcamera/tests/VirtualCameraSessionTest.cc

+19 −2
Original line number Diff line number Diff line
@@ -20,8 +20,8 @@ 
#include "VirtualCameraDevice.h"

#include "VirtualCameraSession.h"

#include "aidl/android/companion/virtualcamera/BnVirtualCameraCallback.h"

#include "aidl/android/companion/virtualcamera/IVirtualCameraCallback.h"

#include "aidl/android/companion/virtualcamera/SupportedStreamConfiguration.h"

#include "aidl/android/hardware/camera/common/Status.h"

#include "aidl/android/hardware/camera/device/BnCameraDeviceCallback.h"

#include "aidl/android/hardware/camera/device/StreamConfiguration.h"

#include "aidl/android/hardware/graphics/common/PixelFormat.h"
@@ -44,6 +44,7 @@ constexpr int kCameraId = 42; 
using ::aidl::android::companion::virtualcamera::BnVirtualCameraCallback;

using ::aidl::android::companion::virtualcamera::Format;

using ::aidl::android::companion::virtualcamera::SupportedStreamConfiguration;

using ::aidl::android::hardware::camera::common::Status;

using ::aidl::android::hardware::camera::device::BnCameraDeviceCallback;

using ::aidl::android::hardware::camera::device::BufferRequest;

using ::aidl::android::hardware::camera::device::BufferRequestStatus;
@@ -110,7 +111,7 @@ class VirtualCameraSessionTest : public ::testing::Test { 
                                         .pixelFormat = Format::YUV_420_888}},

        mMockVirtualCameraClientCallback);

    mVirtualCameraSession = ndk::SharedRefBase::make<VirtualCameraSession>(

        *mVirtualCameraDevice, mMockCameraDeviceCallback,

        mVirtualCameraDevice, mMockCameraDeviceCallback,

        mMockVirtualCameraClientCallback);



    // Explicitly defining default actions below to prevent gmock from
@@ -223,6 +224,22 @@ TEST_F(VirtualCameraSessionTest, onProcessCaptureRequestTriggersClientCallback) 
  EXPECT_THAT(aidlReturn, Eq(requests.size()));

}



TEST_F(VirtualCameraSessionTest, configureAfterCameraRelease) {

  StreamConfiguration streamConfiguration;

  streamConfiguration.streams = {

      createStream(kStreamId, kWidth, kHeight, PixelFormat::YCBCR_420_888)};

  std::vector<HalStream> halStreams;



  // Release virtual camera.

  mVirtualCameraDevice.reset();



  // Expect configuration attempt returns CAMERA_DISCONNECTED service specific code.

  EXPECT_THAT(

      mVirtualCameraSession->configureStreams(streamConfiguration, &halStreams)

          .getServiceSpecificError(),

      Eq(static_cast<int32_t>(Status::CAMERA_DISCONNECTED)));

}



}  // namespace

}  // namespace virtualcamera

}  // namespace companion