From 90e6369928665598bf407e1ceb0ae47d39c80a64 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Fri, 25 Oct 2019 14:13:01 -0700 Subject: [PATCH] cameraserver: filter out non-HAL3 devices from HIDL addListener. Bug: 143192708 Test: Use vendor clients of cameraserver Test: GCA (sanity) Change-Id: Ic31f71887b55f3d838ca35274a5f65802ea50584 Signed-off-by: Jayant Chowdhary --- .../camera/libcameraservice/CameraService.cpp | 23 +++++++++++++++---- .../hidl/HidlCameraService.cpp | 8 +++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index a02178e890..db26848cd8 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -3448,10 +3448,21 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId, ALOGE("%s: Invalid camera id %s, skipping", __FUNCTION__, cameraId.string()); return; } - + bool supportsHAL3 = false; + // supportsCameraApi also holds mInterfaceMutex, we can't call it in the + // HIDL onStatusChanged wrapper call (we'll hold mStatusListenerLock and + // mInterfaceMutex together, which can lead to deadlocks) + binder::Status sRet = + supportsCameraApi(String16(cameraId), hardware::ICameraService::API_VERSION_2, + &supportsHAL3); + if (!sRet.isOk()) { + ALOGW("%s: Failed to determine if device supports HAL3 %s, supportsCameraApi call failed", + __FUNCTION__, cameraId.string()); + return; + } // Update the status for this camera state, then send the onStatusChangedCallbacks to each // of the listeners with both the mStatusStatus and mStatusListenerLock held - state->updateStatus(status, cameraId, rejectSourceStates, [this, &deviceKind] + state->updateStatus(status, cameraId, rejectSourceStates, [this, &deviceKind, &supportsHAL3] (const String8& cameraId, StatusInternal status) { if (status != StatusInternal::ENUMERATING) { @@ -3473,9 +3484,11 @@ void CameraService::updateStatus(StatusInternal status, const String8& cameraId, Mutex::Autolock lock(mStatusListenerLock); for (auto& listener : mListenerList) { - if (shouldSkipStatusUpdates(deviceKind, listener->isVendorListener(), - listener->getListenerPid(), listener->getListenerUid())) { - ALOGV("Skipping camera discovery callback for system-only camera %s", + bool isVendorListener = listener->isVendorListener(); + if (shouldSkipStatusUpdates(deviceKind, isVendorListener, + listener->getListenerPid(), listener->getListenerUid()) || + (isVendorListener && !supportsHAL3)) { + ALOGV("Skipping discovery callback for system-only camera/HAL1 device %s", cameraId.c_str()); continue; } diff --git a/services/camera/libcameraservice/hidl/HidlCameraService.cpp b/services/camera/libcameraservice/hidl/HidlCameraService.cpp index 74cfe4267a..1daa035aec 100644 --- a/services/camera/libcameraservice/hidl/HidlCameraService.cpp +++ b/services/camera/libcameraservice/hidl/HidlCameraService.cpp @@ -191,6 +191,14 @@ Return HidlCameraService::addListener(const sp& hC _hidl_cb(status, {}); return Void(); } + cameraStatusAndIds.erase(std::remove_if(cameraStatusAndIds.begin(), cameraStatusAndIds.end(), + [this](const hardware::CameraStatus& s) { + bool supportsHAL3 = false; + binder::Status sRet = + mAidlICameraService->supportsCameraApi(String16(s.cameraId), + hardware::ICameraService::API_VERSION_2, &supportsHAL3); + return !sRet.isOk() || !supportsHAL3; + }), cameraStatusAndIds.end()); hidl_vec hCameraStatusAndIds; //Convert cameraStatusAndIds to HIDL and call callback convertToHidl(cameraStatusAndIds, &hCameraStatusAndIds);