Merge "Revert "Camera: Fix race condition between initialize and statusCallback"" into rvc-qpr-dev
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp
index 32d118d..f115fd8 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.cpp
+++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp
@@ -1344,6 +1344,20 @@
}
}
+ // cameraDeviceStatusChange callbacks may be called (and causing new devices added)
+ // before setCallback returns
+ hardware::Return<Status> status = interface->setCallback(this);
+ if (!status.isOk()) {
+ ALOGE("%s: Transaction error setting up callbacks with camera provider '%s': %s",
+ __FUNCTION__, mProviderName.c_str(), status.description().c_str());
+ return DEAD_OBJECT;
+ }
+ if (status != Status::OK) {
+ ALOGE("%s: Unable to register callbacks with camera provider '%s'",
+ __FUNCTION__, mProviderName.c_str());
+ return mapToStatusT(status);
+ }
+
hardware::Return<bool> linked = interface->linkToDeath(this, /*cookie*/ mId);
if (!linked.isOk()) {
ALOGE("%s: Transaction error in linking to camera provider '%s' death: %s",
@@ -1372,7 +1386,6 @@
return res;
}
- Status status;
// Get initial list of camera devices, if any
std::vector<std::string> devices;
hardware::Return<void> ret = interface->getCameraIdList([&status, this, &devices](
@@ -1437,22 +1450,6 @@
}
}
- // cameraDeviceStatusChange callbacks may be called (and causing new devices added)
- // before setCallback returns. setCallback must be called after addDevice so that
- // the physical camera status callback can look up available regular
- // cameras.
- hardware::Return<Status> st = interface->setCallback(this);
- if (!st.isOk()) {
- ALOGE("%s: Transaction error setting up callbacks with camera provider '%s': %s",
- __FUNCTION__, mProviderName.c_str(), st.description().c_str());
- return DEAD_OBJECT;
- }
- if (st != Status::OK) {
- ALOGE("%s: Unable to register callbacks with camera provider '%s'",
- __FUNCTION__, mProviderName.c_str());
- return mapToStatusT(st);
- }
-
ALOGI("Camera provider %s ready with %zu camera devices",
mProviderName.c_str(), mDevices.size());
@@ -1783,9 +1780,10 @@
CameraDeviceStatus newStatus) {
sp<StatusListener> listener;
std::string id;
- bool initialized = false;
{
std::lock_guard<std::mutex> lock(mLock);
+ if (!mInitialized) return hardware::Void();
+
bool known = false;
for (auto& deviceInfo : mDevices) {
if (deviceInfo->mName == cameraDeviceName) {
@@ -1802,10 +1800,9 @@
__FUNCTION__, id.c_str(), physicalCameraDeviceName.c_str());
return hardware::Void();
}
- ALOGI("Camera device %s physical device %s status is now %s, was %s",
+ ALOGI("Camera device %s physical device %s status is now %s",
cameraDeviceName.c_str(), physicalCameraDeviceName.c_str(),
- deviceStatusToString(newStatus), deviceStatusToString(
- deviceInfo->mPhysicalStatus[physicalCameraDeviceName]));
+ deviceStatusToString(newStatus));
known = true;
break;
}
@@ -1818,13 +1815,12 @@
return hardware::Void();
}
listener = mManager->getStatusListener();
- initialized = mInitialized;
}
// Call without lock held to allow reentrancy into provider manager
// Don't send the callback if providerInfo hasn't been initialized.
// CameraService will initialize device status after provider is
// initialized
- if (listener != nullptr && initialized) {
+ if (listener != nullptr) {
String8 physicalId(physicalCameraDeviceName.c_str());
listener->onDeviceStatusChanged(String8(id.c_str()),
physicalId, newStatus);
diff --git a/services/camera/libcameraservice/common/CameraProviderManager.h b/services/camera/libcameraservice/common/CameraProviderManager.h
index 25d3639..281cb3d 100644
--- a/services/camera/libcameraservice/common/CameraProviderManager.h
+++ b/services/camera/libcameraservice/common/CameraProviderManager.h
@@ -444,8 +444,6 @@
const hardware::camera::common::V1_0::CameraResourceCost mResourceCost;
hardware::camera::common::V1_0::CameraDeviceStatus mStatus;
- std::map<std::string, hardware::camera::common::V1_0::CameraDeviceStatus>
- mPhysicalStatus;
wp<ProviderInfo> mParentProvider;
diff --git a/services/mediametrics/AudioAnalytics.cpp b/services/mediametrics/AudioAnalytics.cpp
index 29801a4..d78d1e3 100644
--- a/services/mediametrics/AudioAnalytics.cpp
+++ b/services/mediametrics/AudioAnalytics.cpp
@@ -43,7 +43,7 @@
#endif
// Maximum length of a device name.
-static constexpr size_t STATSD_DEVICE_NAME_MAX_LENGTH = 32;
+// static constexpr size_t STATSD_DEVICE_NAME_MAX_LENGTH = 32; // unused since we suppress
// Transmit Enums to statsd in integer or strings (this must match the atoms.proto)
static constexpr bool STATSD_USE_INT_FOR_ENUM = false;
@@ -66,6 +66,8 @@
static constexpr int PREVIOUS_STATE_EXPIRE_SEC = 60 * 60; // 1 hour.
+static constexpr const char * SUPPRESSED = "SUPPRESSED";
+
/*
* For logging purposes, we list all of the MediaMetrics atom fields,
* which can then be associated with consecutive arguments to the statsd write.
@@ -448,6 +450,8 @@
std::string outputDeviceNames;
if (outputDevices.find("AUDIO_DEVICE_OUT_BLUETOOTH") != std::string::npos) {
isBluetooth = true;
+ outputDeviceNames = SUPPRESSED;
+#if 0 // TODO(b/161554630) sanitize name
mAudioAnalytics.mAnalyticsState->timeMachine().get(
"audio.device.bt_a2dp", AMEDIAMETRICS_PROP_NAME, &outputDeviceNames);
// Remove | if present
@@ -455,6 +459,7 @@
if (outputDeviceNames.size() > STATSD_DEVICE_NAME_MAX_LENGTH) {
outputDeviceNames.resize(STATSD_DEVICE_NAME_MAX_LENGTH); // truncate
}
+#endif
}
switch (itemType) {
@@ -775,7 +780,7 @@
std::lock_guard l(mLock);
mA2dpConnectionRequestNs = atNs;
++mA2dpConnectionRequests;
- mA2dpDeviceName = name;
+ mA2dpDeviceName = SUPPRESSED; // TODO(b/161554630) sanitize name
}
ALOGD("(key=%s) a2dp connection name:%s request atNs:%lld",
key.c_str(), name.c_str(), (long long)atNs);
diff --git a/services/mediametrics/AudioPowerUsage.cpp b/services/mediametrics/AudioPowerUsage.cpp
index cca6b41..33dfa8fa 100644
--- a/services/mediametrics/AudioPowerUsage.cpp
+++ b/services/mediametrics/AudioPowerUsage.cpp
@@ -200,6 +200,34 @@
return true;
}
+bool AudioPowerUsage::saveAsItems_l(
+ int32_t device, int64_t duration_ns, int32_t type, double average_vol)
+{
+ ALOGV("%s: (%#x, %d, %lld, %f)", __func__, device, type,
+ (long long)duration_ns, average_vol );
+ if (duration_ns == 0) {
+ return true; // skip duration 0 usage
+ }
+ if (device == 0) {
+ return true; //ignore unknown device
+ }
+
+ bool ret = false;
+ const int32_t input_bit = device & INPUT_DEVICE_BIT;
+ int32_t device_bits = device ^ input_bit;
+
+ while (device_bits != 0) {
+ int32_t tmp_device = device_bits & -device_bits; // get lowest bit
+ device_bits ^= tmp_device; // clear lowest bit
+ tmp_device |= input_bit; // restore input bit
+ ret = saveAsItem_l(tmp_device, duration_ns, type, average_vol);
+
+ ALOGV("%s: device %#x recorded, remaining device_bits = %#x", __func__,
+ tmp_device, device_bits);
+ }
+ return ret;
+}
+
void AudioPowerUsage::checkTrackRecord(
const std::shared_ptr<const mediametrics::Item>& item, bool isTrack)
{
@@ -245,7 +273,7 @@
ALOGV("device = %s => %d", device_strings.c_str(), device);
}
std::lock_guard l(mLock);
- saveAsItem_l(device, deviceTimeNs, type, deviceVolume);
+ saveAsItems_l(device, deviceTimeNs, type, deviceVolume);
}
void AudioPowerUsage::checkMode(const std::shared_ptr<const mediametrics::Item>& item)
@@ -262,7 +290,7 @@
if (durationNs > 0) {
mDeviceVolume = (mDeviceVolume * double(mVolumeTimeNs - mDeviceTimeNs) +
mVoiceVolume * double(endCallNs - mVolumeTimeNs)) / durationNs;
- saveAsItem_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
+ saveAsItems_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
}
} else if (mode == "AUDIO_MODE_IN_CALL") { // entering call mode
mStartCallNs = item->getTimestamp(); // advisory only
@@ -321,7 +349,7 @@
if (durationNs > 0) {
mDeviceVolume = (mDeviceVolume * double(mVolumeTimeNs - mDeviceTimeNs) +
mVoiceVolume * double(endDeviceNs - mVolumeTimeNs)) / durationNs;
- saveAsItem_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
+ saveAsItems_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
}
// reset statistics
mDeviceVolume = 0;
diff --git a/services/mediametrics/AudioPowerUsage.h b/services/mediametrics/AudioPowerUsage.h
index 446ff4f..b705a6a 100644
--- a/services/mediametrics/AudioPowerUsage.h
+++ b/services/mediametrics/AudioPowerUsage.h
@@ -85,6 +85,8 @@
REQUIRES(mLock);
static void sendItem(const std::shared_ptr<const mediametrics::Item>& item);
void collect();
+ bool saveAsItems_l(int32_t device, int64_t duration, int32_t type, double average_vol)
+ REQUIRES(mLock);
AudioAnalytics * const mAudioAnalytics;
const bool mDisabled;