diff options
| -rw-r--r-- | cmds/atrace/atrace.cpp | 1 | ||||
| -rw-r--r-- | services/sensorservice/SensorDevice.cpp | 61 | ||||
| -rw-r--r-- | services/sensorservice/SensorDevice.h | 6 | ||||
| -rw-r--r-- | services/surfaceflinger/RegionSamplingThread.cpp | 61 | ||||
| -rw-r--r-- | services/surfaceflinger/RegionSamplingThread.h | 5 | ||||
| -rw-r--r-- | services/surfaceflinger/TransactionCompletedThread.cpp | 17 | ||||
| -rw-r--r-- | services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp | 72 |
7 files changed, 178 insertions, 45 deletions
diff --git a/cmds/atrace/atrace.cpp b/cmds/atrace/atrace.cpp index 7ac0eb7d69..a4b00f8e0c 100644 --- a/cmds/atrace/atrace.cpp +++ b/cmds/atrace/atrace.cpp @@ -890,6 +890,7 @@ static void cleanUpUserspaceTracing() setTagsProperty(0); clearAppProperties(); pokeBinderServices(); + pokeHalServices(); if (g_tracePdx) { ServiceUtility::PokeServices(); diff --git a/services/sensorservice/SensorDevice.cpp b/services/sensorservice/SensorDevice.cpp index fcd4f29adf..717f31769b 100644 --- a/services/sensorservice/SensorDevice.cpp +++ b/services/sensorservice/SensorDevice.cpp @@ -45,7 +45,9 @@ namespace android { ANDROID_SINGLETON_STATIC_INSTANCE(SensorDevice) -static status_t StatusFromResult(Result result) { +namespace { + +status_t statusFromResult(Result result) { switch (result) { case Result::OK: return OK; @@ -71,6 +73,8 @@ enum EventQueueFlagBitsInternal : uint32_t { INTERNAL_WAKE = 1 << 16, }; +} // anonymous namespace + void SensorsHalDeathReceivier::serviceDied( uint64_t /* cookie */, const wp<::android::hidl::base::V1_0::IBase>& /* service */) { @@ -105,7 +109,7 @@ SensorDevice::SensorDevice() initializeSensorList(); mIsDirectReportSupported = - (checkReturn(mSensors->unregisterDirectChannel(-1)) != Result::INVALID_OPERATION); + (checkReturnAndGetStatus(mSensors->unregisterDirectChannel(-1)) != INVALID_OPERATION); } void SensorDevice::initializeSensorList() { @@ -122,7 +126,7 @@ void SensorDevice::initializeSensorList() { convertToSensor(list[i], &sensor); // Sanity check and clamp power if it is 0 (or close) if (sensor.power < minPowerMa) { - ALOGE("Reported power %f not deemed sane, clamping to %f", + ALOGI("Reported power %f not deemed sane, clamping to %f", sensor.power, minPowerMa); sensor.power = minPowerMa; } @@ -217,10 +221,10 @@ SensorDevice::HalConnectionStatus SensorDevice::connectHidlServiceV2_0() { mWakeLockQueue != nullptr && mEventQueueFlag != nullptr && mWakeLockQueueFlag != nullptr); - status_t status = StatusFromResult(checkReturn(mSensors->initialize( + status_t status = checkReturnAndGetStatus(mSensors->initialize( *mEventQueue->getDesc(), *mWakeLockQueue->getDesc(), - new SensorsCallback()))); + new SensorsCallback())); if (status != NO_ERROR) { connectionStatus = HalConnectionStatus::FAILED_TO_CONNECT; @@ -270,6 +274,8 @@ bool SensorDevice::sensorHandlesChanged(const Vector<sensor_t>& oldSensorList, bool didChange = false; if (oldSensorList.size() != newSensorList.size()) { + ALOGI("Sensor list size changed from %zu to %zu", oldSensorList.size(), + newSensorList.size()); didChange = true; } @@ -281,6 +287,7 @@ bool SensorDevice::sensorHandlesChanged(const Vector<sensor_t>& oldSensorList, if (prevSensor.handle == newSensor.handle) { found = true; if (!sensorIsEquivalent(prevSensor, newSensor)) { + ALOGI("Sensor %s not equivalent to previous version", newSensor.name); didChange = true; } } @@ -289,6 +296,7 @@ bool SensorDevice::sensorHandlesChanged(const Vector<sensor_t>& oldSensorList, if (!found) { // Could not find the new sensor in the old list of sensors, the lists must // have changed. + ALOGI("Sensor %s (handle %d) did not exist before", newSensor.name, newSensor.handle); didChange = true; } } @@ -423,7 +431,7 @@ ssize_t SensorDevice::pollHal(sensors_event_t* buffer, size_t count) { convertToSensorEvents(events, dynamicSensorsAdded, buffer); err = (ssize_t)events.size(); } else { - err = StatusFromResult(result); + err = statusFromResult(result); } }); @@ -621,7 +629,7 @@ status_t SensorDevice::activateLocked(void* ident, int handle, int enabled) { if (actuateHardware) { ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w activate handle=%d enabled=%d", handle, enabled); - err = StatusFromResult(checkReturn(mSensors->activate(handle, enabled))); + err = checkReturnAndGetStatus(mSensors->activate(handle, enabled)); ALOGE_IF(err, "Error %s sensor %d (%s)", enabled ? "activating" : "disabling", handle, strerror(-err)); @@ -694,9 +702,8 @@ status_t SensorDevice::batchLocked(void* ident, int handle, int flags, int64_t s if (prevBestBatchParams != info.bestBatchParams) { ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w BATCH 0x%08x %" PRId64 " %" PRId64, handle, info.bestBatchParams.mTSample, info.bestBatchParams.mTBatch); - err = StatusFromResult( - checkReturn(mSensors->batch( - handle, info.bestBatchParams.mTSample, info.bestBatchParams.mTBatch))); + err = checkReturnAndGetStatus(mSensors->batch( + handle, info.bestBatchParams.mTSample, info.bestBatchParams.mTBatch)); if (err != NO_ERROR) { ALOGE("sensor batch failed %p 0x%08x %" PRId64 " %" PRId64 " err=%s", mSensors.get(), handle, info.bestBatchParams.mTSample, @@ -720,7 +727,7 @@ status_t SensorDevice::flush(void* ident, int handle) { if (mSensors == nullptr) return NO_INIT; if (isClientDisabled(ident)) return INVALID_OPERATION; ALOGD_IF(DEBUG_CONNECTIONS, "\t>>> actuating h/w flush %d", handle); - return StatusFromResult(checkReturn(mSensors->flush(handle))); + return checkReturnAndGetStatus(mSensors->flush(handle)); } bool SensorDevice::isClientDisabled(void* ident) { @@ -753,16 +760,14 @@ void SensorDevice::enableAllSensors() { const int sensor_handle = mActivationCount.keyAt(i); ALOGD_IF(DEBUG_CONNECTIONS, "\t>> reenable actuating h/w sensor enable handle=%d ", sensor_handle); - status_t err = StatusFromResult( - checkReturn(mSensors->batch( - sensor_handle, - info.bestBatchParams.mTSample, - info.bestBatchParams.mTBatch))); + status_t err = checkReturnAndGetStatus(mSensors->batch( + sensor_handle, + info.bestBatchParams.mTSample, + info.bestBatchParams.mTBatch)); ALOGE_IF(err, "Error calling batch on sensor %d (%s)", sensor_handle, strerror(-err)); if (err == NO_ERROR) { - err = StatusFromResult( - checkReturn(mSensors->activate(sensor_handle, 1 /* enabled */))); + err = checkReturnAndGetStatus(mSensors->activate(sensor_handle, 1 /* enabled */)); ALOGE_IF(err, "Error activating sensor %d (%s)", sensor_handle, strerror(-err)); } @@ -810,14 +815,13 @@ status_t SensorDevice::injectSensorData( Event ev; convertFromSensorEvent(*injected_sensor_event, &ev); - return StatusFromResult(checkReturn(mSensors->injectSensorData(ev))); + return checkReturnAndGetStatus(mSensors->injectSensorData(ev)); } status_t SensorDevice::setMode(uint32_t mode) { if (mSensors == nullptr) return NO_INIT; - return StatusFromResult( - checkReturn(mSensors->setOperationMode( - static_cast<hardware::sensors::V1_0::OperationMode>(mode)))); + return checkReturnAndGetStatus(mSensors->setOperationMode( + static_cast<hardware::sensors::V1_0::OperationMode>(mode))); } int32_t SensorDevice::registerDirectChannel(const sensors_direct_mem_t* memory) { @@ -855,7 +859,7 @@ int32_t SensorDevice::registerDirectChannel(const sensors_direct_mem_t* memory) if (result == Result::OK) { ret = channelHandle; } else { - ret = StatusFromResult(result); + ret = statusFromResult(result); } })); return ret; @@ -894,12 +898,12 @@ int32_t SensorDevice::configureDirectChannel(int32_t sensorHandle, checkReturn(mSensors->configDirectReport(sensorHandle, channelHandle, rate, [&ret, rate] (auto result, auto token) { if (rate == RateLevel::STOP) { - ret = StatusFromResult(result); + ret = statusFromResult(result); } else { if (result == Result::OK) { ret = token; } else { - ret = StatusFromResult(result); + ret = statusFromResult(result); } } })); @@ -1008,7 +1012,7 @@ void SensorDevice::convertToSensorEvents( } void SensorDevice::handleHidlDeath(const std::string & detail) { - if (!SensorDevice::getInstance().mSensors->supportsMessageQueues()) { + if (!mSensors->supportsMessageQueues()) { // restart is the only option at present. LOG_ALWAYS_FATAL("Abort due to ISensors hidl service failure, detail: %s.", detail.c_str()); } else { @@ -1016,5 +1020,10 @@ void SensorDevice::handleHidlDeath(const std::string & detail) { } } +status_t SensorDevice::checkReturnAndGetStatus(const Return<Result>& ret) { + checkReturn(ret); + return (!ret.isOk()) ? DEAD_OBJECT : statusFromResult(ret); +} + // --------------------------------------------------------------------------- }; // namespace android diff --git a/services/sensorservice/SensorDevice.h b/services/sensorservice/SensorDevice.h index e8685c292c..d2c6994023 100644 --- a/services/sensorservice/SensorDevice.h +++ b/services/sensorservice/SensorDevice.h @@ -211,14 +211,14 @@ private: status_t batchLocked(void* ident, int handle, int flags, int64_t samplingPeriodNs, int64_t maxBatchReportLatencyNs); - static void handleHidlDeath(const std::string &detail); + void handleHidlDeath(const std::string &detail); template<typename T> - static Return<T> checkReturn(Return<T> &&ret) { + void checkReturn(const Return<T>& ret) { if (!ret.isOk()) { handleHidlDeath(ret.description()); } - return std::move(ret); } + status_t checkReturnAndGetStatus(const Return<Result>& ret); //TODO(b/67425500): remove waiter after bug is resolved. sp<SensorDeviceUtils::HidlServiceRegistrationWaiter> mRestartWaiter; diff --git a/services/surfaceflinger/RegionSamplingThread.cpp b/services/surfaceflinger/RegionSamplingThread.cpp index 368426018b..7fa33f597c 100644 --- a/services/surfaceflinger/RegionSamplingThread.cpp +++ b/services/surfaceflinger/RegionSamplingThread.cpp @@ -26,6 +26,8 @@ #include <utils/Trace.h> #include <string> +#include <compositionengine/Display.h> +#include <compositionengine/impl/OutputCompositionState.h> #include "DisplayDevice.h" #include "Layer.h" #include "SurfaceFlinger.h" @@ -264,7 +266,22 @@ float getLuma(float r, float g, float b) { } } // anonymous namespace -float sampleArea(const uint32_t* data, int32_t stride, const Rect& area) { +float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t stride, + uint32_t orientation, const Rect& sample_area) { + if (!sample_area.isValid() || (sample_area.getWidth() > width) || + (sample_area.getHeight() > height)) { + ALOGE("invalid sampling region requested"); + return 0.0f; + } + + // (b/133849373) ROT_90 screencap images produced upside down + auto area = sample_area; + if (orientation & ui::Transform::ROT_90) { + area.top = height - area.top; + area.bottom = height - area.bottom; + std::swap(area.top, area.bottom); + } + std::array<int32_t, 256> brightnessBuckets = {}; const int32_t majoritySampleNum = area.getWidth() * area.getHeight() / 2; @@ -293,18 +310,21 @@ float sampleArea(const uint32_t* data, int32_t stride, const Rect& area) { std::vector<float> RegionSamplingThread::sampleBuffer( const sp<GraphicBuffer>& buffer, const Point& leftTop, - const std::vector<RegionSamplingThread::Descriptor>& descriptors) { + const std::vector<RegionSamplingThread::Descriptor>& descriptors, uint32_t orientation) { void* data_raw = nullptr; buffer->lock(GRALLOC_USAGE_SW_READ_OFTEN, &data_raw); std::shared_ptr<uint32_t> data(reinterpret_cast<uint32_t*>(data_raw), [&buffer](auto) { buffer->unlock(); }); if (!data) return {}; + const int32_t width = buffer->getWidth(); + const int32_t height = buffer->getHeight(); const int32_t stride = buffer->getStride(); std::vector<float> lumas(descriptors.size()); std::transform(descriptors.begin(), descriptors.end(), lumas.begin(), [&](auto const& descriptor) { - return sampleArea(data.get(), stride, descriptor.area - leftTop); + return sampleArea(data.get(), width, height, stride, orientation, + descriptor.area - leftTop); }); return lumas; } @@ -317,6 +337,11 @@ void RegionSamplingThread::captureSample() { return; } + const auto device = mFlinger.getDefaultDisplayDevice(); + const auto display = device->getCompositionDisplay(); + const auto state = display->getState(); + const auto orientation = static_cast<ui::Transform::orientation_flags>(state.orientation); + std::vector<RegionSamplingThread::Descriptor> descriptors; Region sampleRegion; for (const auto& [listener, descriptor] : mDescriptors) { @@ -326,10 +351,28 @@ void RegionSamplingThread::captureSample() { const Rect sampledArea = sampleRegion.bounds(); - sp<const DisplayDevice> device = mFlinger.getDefaultDisplayDevice(); - DisplayRenderArea renderArea(device, sampledArea, sampledArea.getWidth(), - sampledArea.getHeight(), ui::Dataspace::V0_SRGB, - ui::Transform::ROT_0); + auto dx = 0; + auto dy = 0; + switch (orientation) { + case ui::Transform::ROT_90: + dx = device->getWidth(); + break; + case ui::Transform::ROT_180: + dx = device->getWidth(); + dy = device->getHeight(); + break; + case ui::Transform::ROT_270: + dy = device->getHeight(); + break; + default: + break; + } + + ui::Transform t(orientation); + auto screencapRegion = t.transform(sampleRegion); + screencapRegion = screencapRegion.translate(dx, dy); + DisplayRenderArea renderArea(device, screencapRegion.bounds(), sampledArea.getWidth(), + sampledArea.getHeight(), ui::Dataspace::V0_SRGB, orientation); std::unordered_set<sp<IRegionSamplingListener>, SpHash<IRegionSamplingListener>> listeners; @@ -395,8 +438,8 @@ void RegionSamplingThread::captureSample() { } ALOGV("Sampling %zu descriptors", activeDescriptors.size()); - std::vector<float> lumas = sampleBuffer(buffer, sampledArea.leftTop(), activeDescriptors); - + std::vector<float> lumas = + sampleBuffer(buffer, sampledArea.leftTop(), activeDescriptors, orientation); if (lumas.size() != activeDescriptors.size()) { ALOGW("collected %zu median luma values for %zu descriptors", lumas.size(), activeDescriptors.size()); diff --git a/services/surfaceflinger/RegionSamplingThread.h b/services/surfaceflinger/RegionSamplingThread.h index 08134e6acd..3c6fcf3872 100644 --- a/services/surfaceflinger/RegionSamplingThread.h +++ b/services/surfaceflinger/RegionSamplingThread.h @@ -37,7 +37,8 @@ class Scheduler; class SurfaceFlinger; struct SamplingOffsetCallback; -float sampleArea(const uint32_t* data, int32_t stride, const Rect& area); +float sampleArea(const uint32_t* data, int32_t width, int32_t height, int32_t stride, + uint32_t orientation, const Rect& area); class RegionSamplingThread : public IBinder::DeathRecipient { public: @@ -94,7 +95,7 @@ private: }; std::vector<float> sampleBuffer( const sp<GraphicBuffer>& buffer, const Point& leftTop, - const std::vector<RegionSamplingThread::Descriptor>& descriptors); + const std::vector<RegionSamplingThread::Descriptor>& descriptors, uint32_t orientation); void doSample(); void binderDied(const wp<IBinder>& who) override; diff --git a/services/surfaceflinger/TransactionCompletedThread.cpp b/services/surfaceflinger/TransactionCompletedThread.cpp index b1bf4e20d3..5cf8eb1a1d 100644 --- a/services/surfaceflinger/TransactionCompletedThread.cpp +++ b/services/surfaceflinger/TransactionCompletedThread.cpp @@ -219,6 +219,7 @@ void TransactionCompletedThread::threadMain() { while (mKeepRunning) { mConditionVariable.wait(mMutex); + std::vector<ListenerStats> completedListenerStats; // For each listener auto completedTransactionsItr = mCompletedTransactions.begin(); @@ -264,11 +265,27 @@ void TransactionCompletedThread::threadMain() { } else { completedTransactionsItr++; } + + completedListenerStats.push_back(std::move(listenerStats)); } if (mPresentFence) { mPresentFence.clear(); } + + // If everyone else has dropped their reference to a layer and its listener is dead, + // we are about to cause the layer to be deleted. If this happens at the wrong time and + // we are holding mMutex, we will cause a deadlock. + // + // The deadlock happens because this thread is holding on to mMutex and when we delete + // the layer, it grabs SF's mStateLock. A different SF binder thread grabs mStateLock, + // then call's TransactionCompletedThread::run() which tries to grab mMutex. + // + // To avoid this deadlock, we need to unlock mMutex when dropping our last reference to + // to the layer. + mMutex.unlock(); + completedListenerStats.clear(); + mMutex.lock(); } } diff --git a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp index 51d6d7e1df..160f0412d9 100644 --- a/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp +++ b/services/surfaceflinger/tests/unittests/RegionSamplingTest.cpp @@ -17,6 +17,8 @@ #undef LOG_TAG #define LOG_TAG "RegionSamplingTest" +#include <ui/Transform.h> + #include <gmock/gmock.h> #include <gtest/gtest.h> #include <array> @@ -33,18 +35,21 @@ public: static int constexpr kWidth = 98; static int constexpr kStride = 100; static int constexpr kHeight = 29; + static int constexpr kOrientation = ui::Transform::ROT_0; std::array<uint32_t, kHeight * kStride> buffer; Rect const whole_area{0, 0, kWidth, kHeight}; }; TEST_F(RegionSamplingTest, calculate_mean_white) { std::fill(buffer.begin(), buffer.end(), kWhite); - EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(1.0f)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area), + testing::FloatEq(1.0f)); } TEST_F(RegionSamplingTest, calculate_mean_black) { std::fill(buffer.begin(), buffer.end(), kBlack); - EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatEq(0.0f)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area), + testing::FloatEq(0.0f)); } TEST_F(RegionSamplingTest, calculate_mean_partial_region) { @@ -54,7 +59,8 @@ TEST_F(RegionSamplingTest, calculate_mean_partial_region) { whole_area.top + halfway_down}; std::fill(buffer.begin(), buffer.begin() + half, 0); std::fill(buffer.begin() + half, buffer.end(), kWhite); - EXPECT_THAT(sampleArea(buffer.data(), kStride, partial_region), testing::FloatEq(0.0f)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, partial_region), + testing::FloatEq(0.0f)); } TEST_F(RegionSamplingTest, calculate_mean_mixed_values) { @@ -63,15 +69,71 @@ TEST_F(RegionSamplingTest, calculate_mean_mixed_values) { n++; return pixel; }); - EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), testing::FloatNear(0.083f, 0.01f)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area), + testing::FloatNear(0.083f, 0.01f)); } TEST_F(RegionSamplingTest, bimodal_tiebreaker) { std::generate(buffer.begin(), buffer.end(), [n = 0]() mutable { return (n++ % 2) ? kBlack : kWhite; }); // presently there's no tiebreaking strategy in place, accept either of the means - EXPECT_THAT(sampleArea(buffer.data(), kStride, whole_area), + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, whole_area), testing::AnyOf(testing::FloatEq(1.0), testing::FloatEq(0.0f))); } +TEST_F(RegionSamplingTest, bounds_checking) { + std::generate(buffer.begin(), buffer.end(), + [n = 0]() mutable { return (n++ > (kStride * kHeight >> 1)) ? kBlack : kWhite; }); + + Rect invalid_region{0, 0, 4, kHeight + 1}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region), + testing::Eq(0.0)); + + invalid_region = Rect{0, 0, -4, kHeight}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region), + testing::Eq(0.0)); + + invalid_region = Rect{3, 0, 2, 0}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region), + testing::Eq(0.0)); + + invalid_region = Rect{0, 3, 0, 2}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, kOrientation, invalid_region), + testing::Eq(0.0)); +} + +// workaround for b/133849373 +TEST_F(RegionSamplingTest, orientation_90) { + std::generate(buffer.begin(), buffer.end(), + [n = 0]() mutable { return (n++ > (kStride * kHeight >> 1)) ? kBlack : kWhite; }); + + Rect tl_region{0, 0, 4, 4}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0, + tl_region), + testing::Eq(1.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180, + tl_region), + testing::Eq(1.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90, + tl_region), + testing::Eq(0.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270, + tl_region), + testing::Eq(0.0)); + + Rect br_region{kWidth - 4, kHeight - 4, kWidth, kHeight}; + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_0, + br_region), + testing::Eq(0.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_180, + br_region), + testing::Eq(0.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_90, + br_region), + testing::Eq(1.0)); + EXPECT_THAT(sampleArea(buffer.data(), kWidth, kHeight, kStride, ui::Transform::ROT_270, + br_region), + testing::Eq(1.0)); +} + } // namespace android |