From 6fac52325372298c094d3fd1ae0710dfff5e5796 Mon Sep 17 00:00:00 2001 From: Huihong Luo Date: Mon, 22 Nov 2021 16:05:23 -0800 Subject: Migrate IDisplayEventConnection interface to AIDL This addresses security vulnerabilities due to hard coded binder interface. Bug: 195660647 Test: atest services/surfaceflinger/tests/unittests/SchedulerTest.cpp Change-Id: I948e97e37056286d54623ca6232580187b138e62 --- libs/gui/DisplayEventReceiver.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'libs/gui/DisplayEventReceiver.cpp') diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp index 03b33c7330..b916e48f79 100644 --- a/libs/gui/DisplayEventReceiver.cpp +++ b/libs/gui/DisplayEventReceiver.cpp @@ -19,7 +19,6 @@ #include #include -#include #include #include -- cgit v1.2.3-59-g8ed1b From ef2e21fa7cdd37dbca56f98da49c5406ae7414d2 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Tue, 1 Feb 2022 14:51:34 -0800 Subject: Add method to get current vsync from sf directly. Bug: 205721584 Test: atest libsurfaceflinger_unittest Test: atest DisplayEventReceiverTest Change-Id: I38d4bd20bc2f2ad7ff964c3d613c28919478c0fc --- libs/gui/DisplayEventReceiver.cpp | 15 +++++ .../aidl/android/gui/IDisplayEventConnection.aidl | 6 ++ libs/gui/aidl/android/gui/VsyncEventData.aidl | 19 ++++++ libs/gui/include/gui/DisplayEventReceiver.h | 5 ++ .../surfaceflinger/Scheduler/DispSyncSource.cpp | 12 +++- services/surfaceflinger/Scheduler/DispSyncSource.h | 9 ++- services/surfaceflinger/Scheduler/EventThread.cpp | 76 ++++++++++++++++++---- services/surfaceflinger/Scheduler/EventThread.h | 17 +++++ .../surfaceflinger/Scheduler/InjectVSyncSource.h | 1 + services/surfaceflinger/Scheduler/Scheduler.cpp | 3 +- .../fuzzer/surfaceflinger_scheduler_fuzzer.cpp | 3 +- .../fuzzer/surfaceflinger_scheduler_fuzzer.h | 2 + services/surfaceflinger/tests/Android.bp | 1 + .../tests/DisplayEventReceiver_test.cpp | 53 +++++++++++++++ .../tests/unittests/DispSyncSourceTest.cpp | 50 +++++++++----- .../tests/unittests/EventThreadTest.cpp | 51 ++++++++++++++- .../tests/unittests/mock/MockEventThread.h | 2 + 17 files changed, 290 insertions(+), 35 deletions(-) create mode 100644 libs/gui/aidl/android/gui/VsyncEventData.aidl create mode 100644 services/surfaceflinger/tests/DisplayEventReceiver_test.cpp (limited to 'libs/gui/DisplayEventReceiver.cpp') diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp index b916e48f79..36e7d80d5e 100644 --- a/libs/gui/DisplayEventReceiver.cpp +++ b/libs/gui/DisplayEventReceiver.cpp @@ -20,6 +20,7 @@ #include #include +#include #include @@ -79,6 +80,20 @@ status_t DisplayEventReceiver::requestNextVsync() { return NO_INIT; } +status_t DisplayEventReceiver::getLatestVsyncEventData(VsyncEventData* outVsyncEventData) const { + if (mEventConnection != nullptr) { + VsyncEventData vsyncEventData; + auto status = mEventConnection->getLatestVsyncEventData(&vsyncEventData); + if (!status.isOk()) { + ALOGE("Failed to get latest vsync event data: %s", status.exceptionMessage().c_str()); + return status.transactionError(); + } + *outVsyncEventData = vsyncEventData; + return NO_ERROR; + } + return NO_INIT; +} + ssize_t DisplayEventReceiver::getEvents(DisplayEventReceiver::Event* events, size_t count) { return DisplayEventReceiver::getEvents(mDataChannel.get(), events, count); diff --git a/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl index 9f41593539..e9a6a0c642 100644 --- a/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl +++ b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl @@ -17,6 +17,7 @@ package android.gui; import android.gui.BitTube; +import android.gui.VsyncEventData; /** @hide */ interface IDisplayEventConnection { @@ -38,4 +39,9 @@ interface IDisplayEventConnection { * requestNextVsync() schedules the next vsync event. It has no effect if the vsync rate is > 0. */ oneway void requestNextVsync(); // Asynchronous + + /* + * getLatestVsyncEventData() gets the latest vsync event data. + */ + VsyncEventData getLatestVsyncEventData(); } diff --git a/libs/gui/aidl/android/gui/VsyncEventData.aidl b/libs/gui/aidl/android/gui/VsyncEventData.aidl new file mode 100644 index 0000000000..7343515d25 --- /dev/null +++ b/libs/gui/aidl/android/gui/VsyncEventData.aidl @@ -0,0 +1,19 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.gui; + +parcelable VsyncEventData cpp_header "gui/VsyncEventData.h"; diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index db41c32549..4e04db3cde 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -172,6 +172,11 @@ public: */ status_t requestNextVsync(); + /** + * getLatestVsyncEventData() gets the latest vsync event data. + */ + status_t getLatestVsyncEventData(VsyncEventData* outVsyncEventData) const; + private: sp mEventConnection; std::unique_ptr mDataChannel; diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.cpp b/services/surfaceflinger/Scheduler/DispSyncSource.cpp index c593340625..747032be0d 100644 --- a/services/surfaceflinger/Scheduler/DispSyncSource.cpp +++ b/services/surfaceflinger/Scheduler/DispSyncSource.cpp @@ -23,6 +23,7 @@ #include #include "EventThread.h" +#include "VSyncTracker.h" #include "VsyncController.h" namespace android::scheduler { @@ -114,7 +115,7 @@ private: std::chrono::nanoseconds mLastCallTime GUARDED_BY(mMutex) = 0ns; }; -DispSyncSource::DispSyncSource(scheduler::VSyncDispatch& vSyncDispatch, +DispSyncSource::DispSyncSource(VSyncDispatch& vSyncDispatch, VSyncTracker& vSyncTracker, std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration, bool traceVsync, const char* name) @@ -122,6 +123,7 @@ DispSyncSource::DispSyncSource(scheduler::VSyncDispatch& vSyncDispatch, mValue(base::StringPrintf("VSYNC-%s", name), 0), mTraceVsync(traceVsync), mVsyncOnLabel(base::StringPrintf("VsyncOn-%s", name)), + mVSyncTracker(vSyncTracker), mWorkDuration(base::StringPrintf("VsyncWorkDuration-%s", name), workDuration), mReadyDuration(readyDuration) { mCallbackRepeater = @@ -184,6 +186,14 @@ void DispSyncSource::onVsyncCallback(nsecs_t vsyncTime, nsecs_t targetWakeupTime } } +VSyncSource::VSyncData DispSyncSource::getLatestVSyncData() const { + std::lock_guard lock(mVsyncMutex); + nsecs_t expectedPresentTime = mVSyncTracker.nextAnticipatedVSyncTimeFrom( + systemTime() + mWorkDuration.get().count() + mReadyDuration.count()); + nsecs_t deadline = expectedPresentTime - mWorkDuration.get().count() - mReadyDuration.count(); + return {expectedPresentTime, deadline}; +} + void DispSyncSource::dump(std::string& result) const { std::lock_guard lock(mVsyncMutex); StringAppendF(&result, "DispSyncSource: %s(%s)\n", mName, mEnabled ? "enabled" : "disabled"); diff --git a/services/surfaceflinger/Scheduler/DispSyncSource.h b/services/surfaceflinger/Scheduler/DispSyncSource.h index 2fce235546..edcd3ac709 100644 --- a/services/surfaceflinger/Scheduler/DispSyncSource.h +++ b/services/surfaceflinger/Scheduler/DispSyncSource.h @@ -24,11 +24,13 @@ namespace android::scheduler { class CallbackRepeater; +class VSyncTracker; class DispSyncSource final : public VSyncSource { public: - DispSyncSource(VSyncDispatch& vSyncDispatch, std::chrono::nanoseconds workDuration, - std::chrono::nanoseconds readyDuration, bool traceVsync, const char* name); + DispSyncSource(VSyncDispatch& vSyncDispatch, VSyncTracker& vSyncTracker, + std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration, + bool traceVsync, const char* name); ~DispSyncSource() override; @@ -38,6 +40,7 @@ public: void setCallback(VSyncSource::Callback* callback) override; void setDuration(std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration) override; + VSyncData getLatestVSyncData() const override; void dump(std::string&) const override; @@ -50,6 +53,8 @@ private: const bool mTraceVsync; const std::string mVsyncOnLabel; + const VSyncTracker& mVSyncTracker; + std::unique_ptr mCallbackRepeater; std::mutex mCallbackMutex; diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index adc1009731..2d0da4643d 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -181,11 +181,17 @@ binder::Status EventThreadConnection::setVsyncRate(int rate) { } binder::Status EventThreadConnection::requestNextVsync() { - ATRACE_NAME("requestNextVsync"); + ATRACE_CALL(); mEventThread->requestNextVsync(this); return binder::Status::ok(); } +binder::Status EventThreadConnection::getLatestVsyncEventData(VsyncEventData* outVsyncEventData) { + ATRACE_CALL(); + *outVsyncEventData = mEventThread->getLatestVsyncEventData(this); + return binder::Status::ok(); +} + status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& event) { constexpr auto toStatus = [](ssize_t size) { return size < 0 ? status_t(size) : status_t(NO_ERROR); @@ -330,6 +336,15 @@ void EventThread::requestNextVsync(const sp& connection) } } +VsyncEventData EventThread::getLatestVsyncEventData( + const sp& connection) const { + nsecs_t frameInterval = mGetVsyncPeriodFunction(connection->mOwnerUid); + VsyncEventData vsyncEventData; + vsyncEventData.frameInterval = frameInterval; + generateFrameTimeline(vsyncEventData, frameInterval, systemTime(SYSTEM_TIME_MONOTONIC)); + return vsyncEventData; +} + void EventThread::onScreenReleased() { std::lock_guard lock(mMutex); if (!mVSyncState || mVSyncState->synthetic) { @@ -561,27 +576,64 @@ int64_t EventThread::generateToken(nsecs_t timestamp, nsecs_t deadlineTimestamp, return FrameTimelineInfo::INVALID_VSYNC_ID; } -void EventThread::generateFrameTimeline(DisplayEventReceiver::Event& event) const { +void EventThread::generateFrameTimeline( + nsecs_t frameInterval, nsecs_t timestamp, nsecs_t preferredExpectedVSyncTimestamp, + nsecs_t preferredDeadlineTimestamp, + std::function setPreferredFrameTimelineIndex, + std::function + setFrameTimeline) const { // Add 1 to ensure the preferredFrameTimelineIndex entry (when multiplier == 0) is included. - for (int multiplier = -VsyncEventData::kFrameTimelinesLength + 1, currentIndex = 0; + for (int64_t multiplier = -VsyncEventData::kFrameTimelinesLength + 1, currentIndex = 0; currentIndex < VsyncEventData::kFrameTimelinesLength; multiplier++) { - nsecs_t deadline = event.vsync.deadlineTimestamp + multiplier * event.vsync.frameInterval; + nsecs_t deadline = preferredDeadlineTimestamp + multiplier * frameInterval; // Valid possible frame timelines must have future values. - if (deadline > event.header.timestamp) { + if (deadline > timestamp) { if (multiplier == 0) { - event.vsync.preferredFrameTimelineIndex = currentIndex; + setPreferredFrameTimelineIndex(currentIndex); } - nsecs_t expectedVSync = - event.vsync.expectedVSyncTimestamp + multiplier * event.vsync.frameInterval; - event.vsync.frameTimelines[currentIndex] = - {.vsyncId = generateToken(event.header.timestamp, deadline, expectedVSync), - .deadlineTimestamp = deadline, - .expectedVSyncTimestamp = expectedVSync}; + nsecs_t expectedVSyncTimestamp = + preferredExpectedVSyncTimestamp + multiplier * frameInterval; + setFrameTimeline(currentIndex, + generateToken(timestamp, deadline, expectedVSyncTimestamp), + expectedVSyncTimestamp, deadline); currentIndex++; } } } +void EventThread::generateFrameTimeline(DisplayEventReceiver::Event& event) const { + generateFrameTimeline( + event.vsync.frameInterval, event.header.timestamp, event.vsync.expectedVSyncTimestamp, + event.vsync.deadlineTimestamp, + [&](int index) { event.vsync.preferredFrameTimelineIndex = index; }, + [&](int64_t index, int64_t vsyncId, nsecs_t expectedVSyncTimestamp, + nsecs_t deadlineTimestamp) { + event.vsync.frameTimelines[index] = {.vsyncId = vsyncId, + .deadlineTimestamp = deadlineTimestamp, + .expectedVSyncTimestamp = + expectedVSyncTimestamp}; + }); +} + +void EventThread::generateFrameTimeline(VsyncEventData& out, const nsecs_t frameInterval, + const nsecs_t timestamp) const { + VSyncSource::VSyncData vsyncData; + { + std::lock_guard lock(mMutex); + vsyncData = mVSyncSource->getLatestVSyncData(); + } + generateFrameTimeline( + frameInterval, timestamp, vsyncData.expectedVSyncTimestamp, vsyncData.deadlineTimestamp, + [&](int index) { out.preferredFrameTimelineIndex = index; }, + [&](int64_t index, int64_t vsyncId, nsecs_t expectedVSyncTimestamp, + nsecs_t deadlineTimestamp) { + out.frameTimelines[index] = + VsyncEventData::FrameTimeline(vsyncId, deadlineTimestamp, + expectedVSyncTimestamp); + }); +} + void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, const DisplayEventConsumers& consumers) { for (const auto& consumer : consumers) { diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index c3b9129744..a858169e65 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -45,6 +45,8 @@ namespace frametimeline { class TokenManager; } // namespace frametimeline +using gui::VsyncEventData; + // --------------------------------------------------------------------------- using ResyncCallback = std::function; @@ -81,6 +83,7 @@ public: virtual void setCallback(Callback* callback) = 0; virtual void setDuration(std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration) = 0; + virtual VSyncData getLatestVSyncData() const = 0; virtual void dump(std::string& result) const = 0; }; @@ -96,6 +99,7 @@ public: binder::Status stealReceiveChannel(gui::BitTube* outChannel) override; binder::Status setVsyncRate(int rate) override; binder::Status requestNextVsync() override; // asynchronous + binder::Status getLatestVsyncEventData(VsyncEventData* outVsyncEventData) override; // Called in response to requestNextVsync. const ResyncCallback resyncCallback; @@ -145,6 +149,8 @@ public: virtual void setVsyncRate(uint32_t rate, const sp& connection) = 0; // Requests the next vsync. If resetIdleTimer is set to true, it resets the idle timer. virtual void requestNextVsync(const sp& connection) = 0; + virtual VsyncEventData getLatestVsyncEventData( + const sp& connection) const = 0; // Retrieves the number of event connections tracked by this EventThread. virtual size_t getEventThreadConnectionCount() = 0; @@ -169,6 +175,8 @@ public: status_t registerDisplayEventConnection(const sp& connection) override; void setVsyncRate(uint32_t rate, const sp& connection) override; void requestNextVsync(const sp& connection) override; + VsyncEventData getLatestVsyncEventData( + const sp& connection) const override; // called before the screen is turned off from main thread void onScreenReleased() override; @@ -211,6 +219,15 @@ private: int64_t generateToken(nsecs_t timestamp, nsecs_t deadlineTimestamp, nsecs_t expectedVSyncTimestamp) const; void generateFrameTimeline(DisplayEventReceiver::Event& event) const; + void generateFrameTimeline(VsyncEventData& out, const nsecs_t frameInterval, + const nsecs_t timestamp) const; + void generateFrameTimeline( + nsecs_t frameInterval, nsecs_t timestamp, nsecs_t preferredExpectedVSyncTimestamp, + nsecs_t preferredDeadlineTimestamp, + std::function setPreferredFrameTimelineIndex, + std::function + setFrameTimeline) const; const std::unique_ptr mVSyncSource GUARDED_BY(mMutex); frametimeline::TokenManager* const mTokenManager; diff --git a/services/surfaceflinger/Scheduler/InjectVSyncSource.h b/services/surfaceflinger/Scheduler/InjectVSyncSource.h index 7b93f1eb13..760a4ee886 100644 --- a/services/surfaceflinger/Scheduler/InjectVSyncSource.h +++ b/services/surfaceflinger/Scheduler/InjectVSyncSource.h @@ -46,6 +46,7 @@ public: const char* getName() const override { return "inject"; } void setVSyncEnabled(bool) override {} void setDuration(std::chrono::nanoseconds, std::chrono::nanoseconds) override {} + VSyncData getLatestVSyncData() const override { return {}; } void dump(std::string&) const override {} private: diff --git a/services/surfaceflinger/Scheduler/Scheduler.cpp b/services/surfaceflinger/Scheduler/Scheduler.cpp index 665d36982a..de27bd1823 100644 --- a/services/surfaceflinger/Scheduler/Scheduler.cpp +++ b/services/surfaceflinger/Scheduler/Scheduler.cpp @@ -134,7 +134,8 @@ void Scheduler::createVsyncSchedule(FeatureFlags features) { std::unique_ptr Scheduler::makePrimaryDispSyncSource( const char* name, std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration, bool traceVsync) { - return std::make_unique(getVsyncDispatch(), workDuration, + return std::make_unique(mVsyncSchedule->getDispatch(), + mVsyncSchedule->getTracker(), workDuration, readyDuration, traceVsync, name); } diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp index 06bbfd26c2..09ffb02b21 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp +++ b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.cpp @@ -100,8 +100,9 @@ void SchedulerFuzzer::fuzzEventThread() { void SchedulerFuzzer::fuzzDispSyncSource() { std::unique_ptr vSyncDispatch = std::make_unique(); + std::unique_ptr vSyncTracker = std::make_unique(); std::unique_ptr dispSyncSource = std::make_unique< - scheduler::DispSyncSource>(*vSyncDispatch, + scheduler::DispSyncSource>(*vSyncDispatch, *vSyncTracker, (std::chrono::nanoseconds) mFdp.ConsumeIntegral() /*workDuration*/, (std::chrono::nanoseconds)mFdp.ConsumeIntegral() diff --git a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h index 89cf819614..84b391246e 100644 --- a/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h +++ b/services/surfaceflinger/fuzzer/surfaceflinger_scheduler_fuzzer.h @@ -103,6 +103,8 @@ public: void setDuration(std::chrono::nanoseconds /* workDuration */, std::chrono::nanoseconds /* readyDuration */) override {} + VSyncData getLatestVSyncData() const override { return {}; } + void dump(std::string& /* result */) const override {} }; diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp index 7b86229556..2ede8309fb 100644 --- a/services/surfaceflinger/tests/Android.bp +++ b/services/surfaceflinger/tests/Android.bp @@ -31,6 +31,7 @@ cc_test { "Credentials_test.cpp", "DereferenceSurfaceControl_test.cpp", "DisplayConfigs_test.cpp", + "DisplayEventReceiver_test.cpp", "EffectLayer_test.cpp", "InvalidHandles_test.cpp", "LayerCallback_test.cpp", diff --git a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp new file mode 100644 index 0000000000..01adbc8962 --- /dev/null +++ b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp @@ -0,0 +1,53 @@ +/* + * Copyright (C) 2022 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include + +namespace android { + +class DisplayEventReceiverTest : public ::testing::Test { +public: + void SetUp() override { EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.initCheck()); } + + DisplayEventReceiver mDisplayEventReceiver; +}; + +TEST_F(DisplayEventReceiverTest, getLatestVsyncEventData) { + const nsecs_t now = systemTime(); + VsyncEventData vsyncEventData; + EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.getLatestVsyncEventData(&vsyncEventData)); + + EXPECT_NE(std::numeric_limits::max(), vsyncEventData.preferredFrameTimelineIndex); + EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) + << "Deadline timestamp should be greater than frame time"; + for (size_t i = 0; i < vsyncEventData.frameTimelines.size(); i++) { + EXPECT_NE(FrameTimelineInfo::INVALID_VSYNC_ID, vsyncEventData.frameTimelines[i].id); + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + vsyncEventData.frameTimelines[i].deadlineTimestamp) + << "Expected vsync timestamp should be greater than deadline"; + if (i > 0) { + EXPECT_GT(vsyncEventData.frameTimelines[i].deadlineTimestamp, + vsyncEventData.frameTimelines[i - 1].deadlineTimestamp) + << "Deadline timestamp out of order for frame timeline " << i; + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + vsyncEventData.frameTimelines[i - 1].expectedPresentTime) + << "Expected vsync timestamp out of order for frame timeline " << i; + } + } +} + +} // namespace android \ No newline at end of file diff --git a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp index 5a0ea352d5..0b6b475222 100644 --- a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp +++ b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp @@ -28,6 +28,7 @@ #include "AsyncCallRecorder.h" #include "Scheduler/DispSyncSource.h" #include "Scheduler/VSyncDispatch.h" +#include "mock/MockVSyncTracker.h" namespace android { namespace { @@ -125,12 +126,13 @@ protected: DispSyncSourceTest(); ~DispSyncSourceTest() override; - void createDispSync(); + void SetUp() override; void createDispSyncSource(); void onVSyncEvent(nsecs_t when, VSyncSource::VSyncData) override; std::unique_ptr mVSyncDispatch; + std::unique_ptr mVSyncTracker; std::unique_ptr mDispSyncSource; AsyncCallRecorder mVSyncEventCallRecorder; @@ -154,20 +156,21 @@ DispSyncSourceTest::~DispSyncSourceTest() { ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name()); } +void DispSyncSourceTest::SetUp() { + mVSyncDispatch = std::make_unique(); + mVSyncTracker = std::make_unique(); +} + void DispSyncSourceTest::onVSyncEvent(nsecs_t when, VSyncSource::VSyncData vsyncData) { ALOGD("onVSyncEvent: %" PRId64, when); mVSyncEventCallRecorder.recordCall(when, vsyncData); } -void DispSyncSourceTest::createDispSync() { - mVSyncDispatch = std::make_unique(); -} - void DispSyncSourceTest::createDispSyncSource() { - mDispSyncSource = - std::make_unique(*mVSyncDispatch, mWorkDuration, - mReadyDuration, true, mName.c_str()); + mDispSyncSource = std::make_unique(*mVSyncDispatch, *mVSyncTracker, + mWorkDuration, mReadyDuration, + true, mName.c_str()); mDispSyncSource->setCallback(this); } @@ -176,13 +179,10 @@ void DispSyncSourceTest::createDispSyncSource() { */ TEST_F(DispSyncSourceTest, createDispSync) { - createDispSync(); EXPECT_TRUE(mVSyncDispatch); } TEST_F(DispSyncSourceTest, createDispSyncSource) { - createDispSync(); - InSequence seq; EXPECT_CALL(*mVSyncDispatch, registerCallback(_, mName)).WillOnce(Return(mFakeToken)); EXPECT_CALL(*mVSyncDispatch, cancel(mFakeToken)) @@ -194,8 +194,6 @@ TEST_F(DispSyncSourceTest, createDispSyncSource) { } TEST_F(DispSyncSourceTest, noCallbackAfterInit) { - createDispSync(); - InSequence seq; EXPECT_CALL(*mVSyncDispatch, registerCallback(_, mName)).Times(1); EXPECT_CALL(*mVSyncDispatch, cancel(_)).Times(1); @@ -210,8 +208,6 @@ TEST_F(DispSyncSourceTest, noCallbackAfterInit) { } TEST_F(DispSyncSourceTest, waitForCallbacks) { - createDispSync(); - InSequence seq; EXPECT_CALL(*mVSyncDispatch, registerCallback(_, mName)).Times(1); EXPECT_CALL(*mVSyncDispatch, @@ -239,8 +235,6 @@ TEST_F(DispSyncSourceTest, waitForCallbacks) { } TEST_F(DispSyncSourceTest, waitForCallbacksWithDurationChange) { - createDispSync(); - InSequence seq; EXPECT_CALL(*mVSyncDispatch, registerCallback(_, mName)).Times(1); EXPECT_CALL(*mVSyncDispatch, @@ -296,5 +290,27 @@ TEST_F(DispSyncSourceTest, waitForCallbacksWithDurationChange) { EXPECT_CALL(*mVSyncDispatch, unregisterCallback(_)).Times(1); } +TEST_F(DispSyncSourceTest, getLatestVsyncData) { + const nsecs_t now = systemTime(); + const nsecs_t vsyncInternalDuration = mWorkDuration.count() + mReadyDuration.count(); + EXPECT_CALL(*mVSyncTracker, nextAnticipatedVSyncTimeFrom(_)) + .WillOnce(Return(now + vsyncInternalDuration + 1)); + { + InSequence seq; + EXPECT_CALL(*mVSyncDispatch, registerCallback(_, mName)).Times(1); + EXPECT_CALL(*mVSyncDispatch, cancel(_)).Times(1); + EXPECT_CALL(*mVSyncDispatch, unregisterCallback(_)).Times(1); + } + + createDispSyncSource(); + EXPECT_TRUE(mDispSyncSource); + + const auto vsyncData = mDispSyncSource->getLatestVSyncData(); + ASSERT_GT(vsyncData.deadlineTimestamp, now); + ASSERT_GT(vsyncData.expectedVSyncTimestamp, vsyncData.deadlineTimestamp); + EXPECT_EQ(vsyncData.deadlineTimestamp, + vsyncData.expectedVSyncTimestamp - vsyncInternalDuration); +} + } // namespace } // namespace android diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index e5f7b03732..cc0a40f74b 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -59,6 +59,7 @@ public: void(std::chrono::nanoseconds workDuration, std::chrono::nanoseconds readyDuration)); MOCK_METHOD1(pauseVsyncCallback, void(bool)); + MOCK_METHOD(VSyncSource::VSyncData, getLatestVSyncData, (), (const, override)); MOCK_CONST_METHOD1(dump, void(std::string&)); }; @@ -257,7 +258,7 @@ void EventThreadTest::expectVsyncEventFrameTimelinesCorrect(nsecs_t expectedTime ASSERT_TRUE(args.has_value()) << " did not receive an event for timestamp " << expectedTimestamp; const auto& event = std::get<0>(args.value()); - for (int i = 0; i < gui::VsyncEventData::kFrameTimelinesLength; i++) { + for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { auto prediction = mTokenManager->getPredictionsForToken(event.vsync.frameTimelines[i].vsyncId); EXPECT_TRUE(prediction.has_value()); @@ -332,6 +333,8 @@ void EventThreadTest::expectUidFrameRateMappingEventReceivedByConnection( namespace { +using namespace testing; + /* ------------------------------------------------------------------------ * Test cases */ @@ -399,6 +402,52 @@ TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesCorrect) { expectVsyncEventFrameTimelinesCorrect(123, 789); } +TEST_F(EventThreadTest, getLatestVsyncEventData) { + const nsecs_t now = systemTime(); + const nsecs_t preferredDeadline = now + 10000000; + const nsecs_t preferredExpectedVSyncTimestamp = now + 20000000; + const VSyncSource::VSyncData preferredData = {preferredExpectedVSyncTimestamp, + preferredDeadline}; + EXPECT_CALL(*mVSyncSource, getLatestVSyncData()).WillOnce(Return(preferredData)); + + VsyncEventData vsyncEventData = mThread->getLatestVsyncEventData(mConnection); + EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) + << "Deadline timestamp should be greater than frame time"; + for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + auto prediction = + mTokenManager->getPredictionsForToken(vsyncEventData.frameTimelines[i].id); + EXPECT_TRUE(prediction.has_value()); + EXPECT_EQ(prediction.value().endTime, vsyncEventData.frameTimelines[i].deadlineTimestamp) + << "Deadline timestamp does not match cached value"; + EXPECT_EQ(prediction.value().presentTime, + vsyncEventData.frameTimelines[i].expectedPresentTime) + << "Expected vsync timestamp does not match cached value"; + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + vsyncEventData.frameTimelines[i].deadlineTimestamp) + << "Expected vsync timestamp should be greater than deadline"; + + if (i > 0) { + EXPECT_GT(vsyncEventData.frameTimelines[i].deadlineTimestamp, + vsyncEventData.frameTimelines[i - 1].deadlineTimestamp) + << "Deadline timestamp out of order for frame timeline " << i; + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + vsyncEventData.frameTimelines[i - 1].expectedPresentTime) + << "Expected vsync timestamp out of order for frame timeline " << i; + } + + // Vsync ID order lines up with registration into test token manager. + EXPECT_EQ(i, vsyncEventData.frameTimelines[i].id) + << "Vsync ID incorrect for frame timeline " << i; + if (i == vsyncEventData.preferredFrameTimelineIndex) { + EXPECT_EQ(vsyncEventData.frameTimelines[i].deadlineTimestamp, preferredDeadline) + << "Preferred deadline timestamp incorrect" << i; + EXPECT_EQ(vsyncEventData.frameTimelines[i].expectedPresentTime, + preferredExpectedVSyncTimestamp) + << "Preferred expected vsync timestamp incorrect" << i; + } + } +} + TEST_F(EventThreadTest, setVsyncRateZeroPostsNoVSyncEventsToThatConnection) { // Create a first connection, register it, and request a vsync rate of zero. ConnectionEventRecorder firstConnectionEventRecorder{0}; diff --git a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h index d25973e1ce..c5ca86a651 100644 --- a/services/surfaceflinger/tests/unittests/mock/MockEventThread.h +++ b/services/surfaceflinger/tests/unittests/mock/MockEventThread.h @@ -44,6 +44,8 @@ public: status_t(const sp &)); MOCK_METHOD2(setVsyncRate, void(uint32_t, const sp &)); MOCK_METHOD1(requestNextVsync, void(const sp &)); + MOCK_METHOD(VsyncEventData, getLatestVsyncEventData, + (const sp &), (const)); MOCK_METHOD1(requestLatestConfig, void(const sp &)); MOCK_METHOD1(pauseVsyncCallback, void(bool)); MOCK_METHOD0(getEventThreadConnectionCount, size_t()); -- cgit v1.2.3-59-g8ed1b From 745f7b59ae19400240af4f390ea3c7f7a962a497 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Tue, 8 Feb 2022 19:36:37 -0800 Subject: DisplayEventReceiver: improve error handling Reset the state in case of a binder error and log the error. Bug: 213926330 Test: boot Change-Id: I1b926d08689d9d6e4b382ce18e4a5ecb9156e564 --- libs/gui/DisplayEventReceiver.cpp | 17 +++++++++++------ libs/gui/include/gui/DisplayEventReceiver.h | 1 + 2 files changed, 12 insertions(+), 6 deletions(-) (limited to 'libs/gui/DisplayEventReceiver.cpp') diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp index b916e48f79..1bef2b7893 100644 --- a/libs/gui/DisplayEventReceiver.cpp +++ b/libs/gui/DisplayEventReceiver.cpp @@ -39,7 +39,13 @@ DisplayEventReceiver::DisplayEventReceiver( mEventConnection = sf->createDisplayEventConnection(vsyncSource, eventRegistration); if (mEventConnection != nullptr) { mDataChannel = std::make_unique(); - mEventConnection->stealReceiveChannel(mDataChannel.get()); + const auto status = mEventConnection->stealReceiveChannel(mDataChannel.get()); + if (!status.isOk()) { + ALOGE("stealReceiveChannel failed: %s", status.toString8().c_str()); + mInitError = std::make_optional(status.transactionError()); + mDataChannel.reset(); + mEventConnection.clear(); + } } } } @@ -50,12 +56,11 @@ DisplayEventReceiver::~DisplayEventReceiver() { status_t DisplayEventReceiver::initCheck() const { if (mDataChannel != nullptr) return NO_ERROR; - return NO_INIT; + return mInitError.has_value() ? mInitError.value() : NO_INIT; } int DisplayEventReceiver::getFd() const { - if (mDataChannel == nullptr) - return NO_INIT; + if (mDataChannel == nullptr) return mInitError.has_value() ? mInitError.value() : NO_INIT; return mDataChannel->getFd(); } @@ -68,7 +73,7 @@ status_t DisplayEventReceiver::setVsyncRate(uint32_t count) { mEventConnection->setVsyncRate(count); return NO_ERROR; } - return NO_INIT; + return mInitError.has_value() ? mInitError.value() : NO_INIT; } status_t DisplayEventReceiver::requestNextVsync() { @@ -76,7 +81,7 @@ status_t DisplayEventReceiver::requestNextVsync() { mEventConnection->requestNextVsync(); return NO_ERROR; } - return NO_INIT; + return mInitError.has_value() ? mInitError.value() : NO_INIT; } ssize_t DisplayEventReceiver::getEvents(DisplayEventReceiver::Event* events, diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index db41c32549..130347e445 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -175,6 +175,7 @@ public: private: sp mEventConnection; std::unique_ptr mDataChannel; + std::optional mInitError; }; // ---------------------------------------------------------------------------- -- cgit v1.2.3-59-g8ed1b From b9c5a77a31d16212d87335abd5909d4d78a2b981 Mon Sep 17 00:00:00 2001 From: Rachel Lee Date: Fri, 4 Feb 2022 21:17:37 -0800 Subject: Use VsyncEventData in DisplayEventReceiver::Event. Clean-up that re-uses VsyncEventData so it's easier to maintain. The ParcelableVsyncEventData is used in AIDL. It is separated from VsyncEventData because of the union in DisplayEventReceiver::Event and Parcelable has non-trivial constructors. Bug: 218563993 Test: atest ChoreographerNativeTest Test: atest libsurfaceflinger_unittest Test: atest libgui_test Change-Id: I3ebeb1c7826300c27c4a12d4dba6fbd16305e9e1 --- libs/gui/DisplayEventDispatcher.cpp | 21 +--- libs/gui/DisplayEventReceiver.cpp | 7 +- libs/gui/VsyncEventData.cpp | 60 +++++------- .../aidl/android/gui/IDisplayEventConnection.aidl | 4 +- .../aidl/android/gui/ParcelableVsyncEventData.aidl | 19 ++++ libs/gui/aidl/android/gui/VsyncEventData.aidl | 19 ---- libs/gui/include/gui/DisplayEventReceiver.h | 14 +-- libs/gui/include/gui/VsyncEventData.h | 56 +++++------ libs/gui/tests/DisplayEventStructLayout_test.cpp | 30 +++++- libs/gui/tests/VsyncEventData_test.cpp | 53 ++++------ libs/nativedisplay/AChoreographer.cpp | 23 ++--- services/surfaceflinger/Scheduler/EventThread.cpp | 109 +++++++++------------ services/surfaceflinger/Scheduler/EventThread.h | 20 ++-- services/surfaceflinger/Scheduler/MessageQueue.cpp | 3 +- .../tests/DisplayEventReceiver_test.cpp | 15 +-- .../surfaceflinger/tests/LayerCallback_test.cpp | 5 +- .../tests/unittests/DispSyncSourceTest.cpp | 10 +- .../tests/unittests/EventThreadTest.cpp | 65 ++++++------ 18 files changed, 239 insertions(+), 294 deletions(-) create mode 100644 libs/gui/aidl/android/gui/ParcelableVsyncEventData.aidl delete mode 100644 libs/gui/aidl/android/gui/VsyncEventData.aidl (limited to 'libs/gui/DisplayEventReceiver.cpp') diff --git a/libs/gui/DisplayEventDispatcher.cpp b/libs/gui/DisplayEventDispatcher.cpp index 26db59bcff..39d380d9e1 100644 --- a/libs/gui/DisplayEventDispatcher.cpp +++ b/libs/gui/DisplayEventDispatcher.cpp @@ -126,7 +126,7 @@ int DisplayEventDispatcher::handleEvent(int, int events, void*) { ALOGV("dispatcher %p ~ Vsync pulse: timestamp=%" PRId64 ", displayId=%s, count=%d, vsyncId=%" PRId64, this, ns2ms(vsyncTimestamp), to_string(vsyncDisplayId).c_str(), vsyncCount, - vsyncEventData.id); + vsyncEventData.preferredVsyncId()); mWaitingForVsync = false; mLastVsyncCount = vsyncCount; dispatchVsync(vsyncTimestamp, vsyncDisplayId, vsyncCount, vsyncEventData); @@ -146,18 +146,6 @@ int DisplayEventDispatcher::handleEvent(int, int events, void*) { return 1; // keep the callback } -void DisplayEventDispatcher::populateFrameTimelines(const DisplayEventReceiver::Event& event, - VsyncEventData* outVsyncEventData) const { - for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { - DisplayEventReceiver::Event::VSync::FrameTimeline receiverTimeline = - event.vsync.frameTimelines[i]; - outVsyncEventData->frameTimelines[i] = - VsyncEventData::FrameTimeline(receiverTimeline.vsyncId, - receiverTimeline.deadlineTimestamp, - receiverTimeline.expectedVSyncTimestamp); - } -} - bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp, PhysicalDisplayId* outDisplayId, uint32_t* outCount, @@ -178,12 +166,7 @@ bool DisplayEventDispatcher::processPendingEvents(nsecs_t* outTimestamp, *outTimestamp = ev.header.timestamp; *outDisplayId = ev.header.displayId; *outCount = ev.vsync.count; - outVsyncEventData->id = ev.vsync.vsyncId; - outVsyncEventData->deadlineTimestamp = ev.vsync.deadlineTimestamp; - outVsyncEventData->frameInterval = ev.vsync.frameInterval; - outVsyncEventData->preferredFrameTimelineIndex = - ev.vsync.preferredFrameTimelineIndex; - populateFrameTimelines(ev, outVsyncEventData); + *outVsyncEventData = ev.vsync.vsyncData; break; case DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG: dispatchHotplug(ev.header.timestamp, ev.header.displayId, ev.hotplug.connected); diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp index 36e7d80d5e..ec0be87340 100644 --- a/libs/gui/DisplayEventReceiver.cpp +++ b/libs/gui/DisplayEventReceiver.cpp @@ -80,15 +80,14 @@ status_t DisplayEventReceiver::requestNextVsync() { return NO_INIT; } -status_t DisplayEventReceiver::getLatestVsyncEventData(VsyncEventData* outVsyncEventData) const { +status_t DisplayEventReceiver::getLatestVsyncEventData( + ParcelableVsyncEventData* outVsyncEventData) const { if (mEventConnection != nullptr) { - VsyncEventData vsyncEventData; - auto status = mEventConnection->getLatestVsyncEventData(&vsyncEventData); + auto status = mEventConnection->getLatestVsyncEventData(outVsyncEventData); if (!status.isOk()) { ALOGE("Failed to get latest vsync event data: %s", status.exceptionMessage().c_str()); return status.transactionError(); } - *outVsyncEventData = vsyncEventData; return NO_ERROR; } return NO_INIT; diff --git a/libs/gui/VsyncEventData.cpp b/libs/gui/VsyncEventData.cpp index aad81d0198..23f0921e99 100644 --- a/libs/gui/VsyncEventData.cpp +++ b/libs/gui/VsyncEventData.cpp @@ -23,52 +23,46 @@ namespace android::gui { -status_t VsyncEventData::readFromParcel(const Parcel* parcel) { +int64_t VsyncEventData::preferredVsyncId() const { + return frameTimelines[preferredFrameTimelineIndex].vsyncId; +} + +int64_t VsyncEventData::preferredDeadlineTimestamp() const { + return frameTimelines[preferredFrameTimelineIndex].deadlineTimestamp; +} + +int64_t VsyncEventData::preferredExpectedPresentationTime() const { + return frameTimelines[preferredFrameTimelineIndex].expectedPresentationTime; +} + +status_t ParcelableVsyncEventData::readFromParcel(const Parcel* parcel) { if (parcel == nullptr) { ALOGE("%s: Null parcel", __func__); return BAD_VALUE; } - SAFE_PARCEL(parcel->readInt64, &id) - SAFE_PARCEL(parcel->readInt64, &deadlineTimestamp); - SAFE_PARCEL(parcel->readInt64, &frameInterval); + SAFE_PARCEL(parcel->readInt64, &vsync.frameInterval); uint64_t uintPreferredFrameTimelineIndex; SAFE_PARCEL(parcel->readUint64, &uintPreferredFrameTimelineIndex); - preferredFrameTimelineIndex = static_cast(uintPreferredFrameTimelineIndex); - - std::vector timelines; - SAFE_PARCEL(parcel->readParcelableVector, &timelines); - std::copy_n(timelines.begin(), timelines.size(), frameTimelines.begin()); + vsync.preferredFrameTimelineIndex = static_cast(uintPreferredFrameTimelineIndex); - return OK; -} -status_t VsyncEventData::writeToParcel(Parcel* parcel) const { - SAFE_PARCEL(parcel->writeInt64, id) - SAFE_PARCEL(parcel->writeInt64, deadlineTimestamp); - SAFE_PARCEL(parcel->writeInt64, frameInterval); - SAFE_PARCEL(parcel->writeUint64, preferredFrameTimelineIndex); - SAFE_PARCEL(parcel->writeParcelableVector, - std::vector(frameTimelines.begin(), frameTimelines.end())); - - return OK; -} -status_t VsyncEventData::FrameTimeline::readFromParcel(const Parcel* parcel) { - if (parcel == nullptr) { - ALOGE("%s: Null parcel", __func__); - return BAD_VALUE; + for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].vsyncId); + SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].deadlineTimestamp); + SAFE_PARCEL(parcel->readInt64, &vsync.frameTimelines[i].expectedPresentationTime); } - SAFE_PARCEL(parcel->readInt64, &id) - SAFE_PARCEL(parcel->readInt64, &deadlineTimestamp); - SAFE_PARCEL(parcel->readInt64, &expectedPresentTime); - return OK; } -status_t VsyncEventData::FrameTimeline::writeToParcel(Parcel* parcel) const { - SAFE_PARCEL(parcel->writeInt64, id); - SAFE_PARCEL(parcel->writeInt64, deadlineTimestamp); - SAFE_PARCEL(parcel->writeInt64, expectedPresentTime); +status_t ParcelableVsyncEventData::writeToParcel(Parcel* parcel) const { + SAFE_PARCEL(parcel->writeInt64, vsync.frameInterval); + SAFE_PARCEL(parcel->writeUint64, vsync.preferredFrameTimelineIndex); + for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].vsyncId); + SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].deadlineTimestamp); + SAFE_PARCEL(parcel->writeInt64, vsync.frameTimelines[i].expectedPresentationTime); + } return OK; } diff --git a/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl index e9a6a0c642..9781ca96f4 100644 --- a/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl +++ b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl @@ -17,7 +17,7 @@ package android.gui; import android.gui.BitTube; -import android.gui.VsyncEventData; +import android.gui.ParcelableVsyncEventData; /** @hide */ interface IDisplayEventConnection { @@ -43,5 +43,5 @@ interface IDisplayEventConnection { /* * getLatestVsyncEventData() gets the latest vsync event data. */ - VsyncEventData getLatestVsyncEventData(); + ParcelableVsyncEventData getLatestVsyncEventData(); } diff --git a/libs/gui/aidl/android/gui/ParcelableVsyncEventData.aidl b/libs/gui/aidl/android/gui/ParcelableVsyncEventData.aidl new file mode 100644 index 0000000000..ba76671f8f --- /dev/null +++ b/libs/gui/aidl/android/gui/ParcelableVsyncEventData.aidl @@ -0,0 +1,19 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.gui; + +parcelable ParcelableVsyncEventData cpp_header "gui/VsyncEventData.h"; diff --git a/libs/gui/aidl/android/gui/VsyncEventData.aidl b/libs/gui/aidl/android/gui/VsyncEventData.aidl deleted file mode 100644 index 7343515d25..0000000000 --- a/libs/gui/aidl/android/gui/VsyncEventData.aidl +++ /dev/null @@ -1,19 +0,0 @@ -/* - * Copyright 2021 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package android.gui; - -parcelable VsyncEventData cpp_header "gui/VsyncEventData.h"; diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h index 4e04db3cde..ddc109e52a 100644 --- a/libs/gui/include/gui/DisplayEventReceiver.h +++ b/libs/gui/include/gui/DisplayEventReceiver.h @@ -35,6 +35,7 @@ namespace android { // ---------------------------------------------------------------------------- using gui::IDisplayEventConnection; +using gui::ParcelableVsyncEventData; using gui::VsyncEventData; namespace gui { @@ -76,16 +77,7 @@ public: struct VSync { uint32_t count; - nsecs_t expectedVSyncTimestamp __attribute__((aligned(8))); - nsecs_t deadlineTimestamp __attribute__((aligned(8))); - nsecs_t frameInterval __attribute__((aligned(8))); - int64_t vsyncId; - size_t preferredFrameTimelineIndex __attribute__((aligned(8))); - struct FrameTimeline { - nsecs_t expectedVSyncTimestamp __attribute__((aligned(8))); - nsecs_t deadlineTimestamp __attribute__((aligned(8))); - int64_t vsyncId; - } frameTimelines[VsyncEventData::kFrameTimelinesLength]; + VsyncEventData vsyncData; }; struct Hotplug { @@ -175,7 +167,7 @@ public: /** * getLatestVsyncEventData() gets the latest vsync event data. */ - status_t getLatestVsyncEventData(VsyncEventData* outVsyncEventData) const; + status_t getLatestVsyncEventData(ParcelableVsyncEventData* outVsyncEventData) const; private: sp mEventConnection; diff --git a/libs/gui/include/gui/VsyncEventData.h b/libs/gui/include/gui/VsyncEventData.h index abac61c334..8e99539fe9 100644 --- a/libs/gui/include/gui/VsyncEventData.h +++ b/libs/gui/include/gui/VsyncEventData.h @@ -21,52 +21,44 @@ #include namespace android::gui { -struct VsyncEventData : public Parcelable { +// Plain Old Data (POD) vsync data structure. For example, it can be easily used in the +// DisplayEventReceiver::Event union. +struct VsyncEventData { // Max amount of frame timelines is arbitrarily set to be reasonable. static constexpr int64_t kFrameTimelinesLength = 7; - // The Vsync Id corresponsing to this vsync event. This will be used to - // populate ISurfaceComposer::setFrameTimelineVsync and - // SurfaceComposerClient::setFrameTimelineVsync - // TODO(b/198191703): Remove when JNI DisplayEventReceiver uses frameTimelines array. - int64_t id = FrameTimelineInfo::INVALID_VSYNC_ID; - - // The deadline in CLOCK_MONOTONIC that the app needs to complete its - // frame by (both on the CPU and the GPU) - // TODO(b/198191703): Remove when JNI DisplayEventReceiver uses frameTimelines array. - int64_t deadlineTimestamp = std::numeric_limits::max(); - // The current frame interval in ns when this frame was scheduled. - int64_t frameInterval = 0; + int64_t frameInterval; - struct FrameTimeline : public Parcelable { - FrameTimeline() = default; - FrameTimeline(int64_t id, int64_t deadlineTimestamp, int64_t expectedPresentTime) - : id(id), - deadlineTimestamp(deadlineTimestamp), - expectedPresentTime(expectedPresentTime) {} + // Index into the frameTimelines that represents the platform's preferred frame timeline. + uint32_t preferredFrameTimelineIndex; + struct alignas(8) FrameTimeline { // The Vsync Id corresponsing to this vsync event. This will be used to // populate ISurfaceComposer::setFrameTimelineVsync and // SurfaceComposerClient::setFrameTimelineVsync - int64_t id = FrameTimelineInfo::INVALID_VSYNC_ID; + int64_t vsyncId; - // The deadline in CLOCK_MONOTONIC that the app needs to complete its - // frame by (both on the CPU and the GPU) - int64_t deadlineTimestamp = std::numeric_limits::max(); + // The deadline in CLOCK_MONOTONIC in nanos that the app needs to complete its + // frame by (both on the CPU and the GPU). + int64_t deadlineTimestamp; - // The anticipated Vsync present time. - int64_t expectedPresentTime = 0; + // The anticipated Vsync presentation time in nanos. + int64_t expectedPresentationTime; + } frameTimelines[kFrameTimelinesLength]; // Sorted possible frame timelines. - status_t readFromParcel(const Parcel*) override; - status_t writeToParcel(Parcel*) const override; - }; + // Gets the preferred frame timeline's vsync ID. + int64_t preferredVsyncId() const; - // Sorted possible frame timelines. - std::array frameTimelines; + // Gets the preferred frame timeline's deadline timestamp. + int64_t preferredDeadlineTimestamp() const; - // Index into the frameTimelines that represents the platform's preferred frame timeline. - size_t preferredFrameTimelineIndex = std::numeric_limits::max(); + // Gets the preferred frame timeline's expected vsync timestamp. + int64_t preferredExpectedPresentationTime() const; +}; + +struct ParcelableVsyncEventData : public Parcelable { + VsyncEventData vsync; status_t readFromParcel(const Parcel*) override; status_t writeToParcel(Parcel*) const override; diff --git a/libs/gui/tests/DisplayEventStructLayout_test.cpp b/libs/gui/tests/DisplayEventStructLayout_test.cpp index bcd39dbbf4..da88463d63 100644 --- a/libs/gui/tests/DisplayEventStructLayout_test.cpp +++ b/libs/gui/tests/DisplayEventStructLayout_test.cpp @@ -20,9 +20,10 @@ namespace android::test { #define CHECK_OFFSET(type, member, expected_offset) \ - static_assert((offsetof(type, member) == (expected_offset)), "") + static_assert((offsetof(type, member) == (expected_offset))) TEST(DisplayEventStructLayoutTest, TestEventAlignment) { + static_assert(std::is_pod::value); CHECK_OFFSET(DisplayEventReceiver::Event, vsync, 24); CHECK_OFFSET(DisplayEventReceiver::Event, hotplug, 24); CHECK_OFFSET(DisplayEventReceiver::Event, modeChange, 24); @@ -32,10 +33,29 @@ TEST(DisplayEventStructLayoutTest, TestEventAlignment) { CHECK_OFFSET(DisplayEventReceiver::Event::Header, timestamp, 16); CHECK_OFFSET(DisplayEventReceiver::Event::VSync, count, 0); - CHECK_OFFSET(DisplayEventReceiver::Event::VSync, expectedVSyncTimestamp, 8); - CHECK_OFFSET(DisplayEventReceiver::Event::VSync, deadlineTimestamp, 16); - CHECK_OFFSET(DisplayEventReceiver::Event::VSync, frameInterval, 24); - CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncId, 32); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameInterval, 8); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.preferredFrameTimelineIndex, 16); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines, 24); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].vsyncId, 24); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, vsyncData.frameTimelines[0].deadlineTimestamp, + 32); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, + vsyncData.frameTimelines[0].expectedPresentationTime, 40); + // Also test the offsets of the last frame timeline. A loop is not used because the non-const + // index cannot be used in static_assert. + const int lastFrameTimelineOffset = /* Start of array */ 24 + + (VsyncEventData::kFrameTimelinesLength - 1) * /* Size of FrameTimeline */ 24; + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1].vsyncId, + lastFrameTimelineOffset); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1] + .deadlineTimestamp, + lastFrameTimelineOffset + 8); + CHECK_OFFSET(DisplayEventReceiver::Event::VSync, + vsyncData.frameTimelines[VsyncEventData::kFrameTimelinesLength - 1] + .expectedPresentationTime, + lastFrameTimelineOffset + 16); CHECK_OFFSET(DisplayEventReceiver::Event::Hotplug, connected, 0); diff --git a/libs/gui/tests/VsyncEventData_test.cpp b/libs/gui/tests/VsyncEventData_test.cpp index a670d42fe3..f114522951 100644 --- a/libs/gui/tests/VsyncEventData_test.cpp +++ b/libs/gui/tests/VsyncEventData_test.cpp @@ -22,54 +22,37 @@ namespace android { +using gui::ParcelableVsyncEventData; using gui::VsyncEventData; using FrameTimeline = gui::VsyncEventData::FrameTimeline; namespace test { -TEST(VsyncEventData, Parcelling) { - VsyncEventData data; - data.id = 123; - data.deadlineTimestamp = 456; - data.frameInterval = 789; - data.preferredFrameTimelineIndex = 1; - FrameTimeline timeline0 = FrameTimeline(1, 2, 3); - FrameTimeline timeline1 = FrameTimeline(4, 5, 6); - data.frameTimelines[0] = timeline0; - data.frameTimelines[1] = timeline1; +TEST(ParcelableVsyncEventData, Parcelling) { + ParcelableVsyncEventData data; + data.vsync.frameInterval = 789; + data.vsync.preferredFrameTimelineIndex = 1; + FrameTimeline timeline0 = FrameTimeline{1, 2, 3}; + FrameTimeline timeline1 = FrameTimeline{4, 5, 6}; + data.vsync.frameTimelines[0] = timeline0; + data.vsync.frameTimelines[1] = timeline1; Parcel p; data.writeToParcel(&p); p.setDataPosition(0); - VsyncEventData data2; + ParcelableVsyncEventData data2; data2.readFromParcel(&p); - ASSERT_EQ(data.id, data2.id); - ASSERT_EQ(data.deadlineTimestamp, data2.deadlineTimestamp); - ASSERT_EQ(data.frameInterval, data2.frameInterval); - ASSERT_EQ(data.preferredFrameTimelineIndex, data2.preferredFrameTimelineIndex); - for (int i = 0; i < data.frameTimelines.size(); i++) { - ASSERT_EQ(data.frameTimelines[i].id, data2.frameTimelines[i].id); - ASSERT_EQ(data.frameTimelines[i].deadlineTimestamp, - data2.frameTimelines[i].deadlineTimestamp); - ASSERT_EQ(data.frameTimelines[i].expectedPresentTime, - data2.frameTimelines[i].expectedPresentTime); + ASSERT_EQ(data.vsync.frameInterval, data2.vsync.frameInterval); + ASSERT_EQ(data.vsync.preferredFrameTimelineIndex, data2.vsync.preferredFrameTimelineIndex); + for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + ASSERT_EQ(data.vsync.frameTimelines[i].vsyncId, data2.vsync.frameTimelines[i].vsyncId); + ASSERT_EQ(data.vsync.frameTimelines[i].deadlineTimestamp, + data2.vsync.frameTimelines[i].deadlineTimestamp); + ASSERT_EQ(data.vsync.frameTimelines[i].expectedPresentationTime, + data2.vsync.frameTimelines[i].expectedPresentationTime); } } -TEST(FrameTimeline, Parcelling) { - FrameTimeline timeline = FrameTimeline(1, 2, 3); - - Parcel p; - timeline.writeToParcel(&p); - p.setDataPosition(0); - - FrameTimeline timeline2; - timeline2.readFromParcel(&p); - ASSERT_EQ(timeline.id, timeline2.id); - ASSERT_EQ(timeline.deadlineTimestamp, timeline2.deadlineTimestamp); - ASSERT_EQ(timeline.expectedPresentTime, timeline2.expectedPresentTime); -} - } // namespace test } // namespace android diff --git a/libs/nativedisplay/AChoreographer.cpp b/libs/nativedisplay/AChoreographer.cpp index b182a4a6f1..3ce381b122 100644 --- a/libs/nativedisplay/AChoreographer.cpp +++ b/libs/nativedisplay/AChoreographer.cpp @@ -103,9 +103,7 @@ class Choreographer; struct ChoreographerFrameCallbackDataImpl { int64_t frameTimeNanos{0}; - std::array frameTimelines; - - size_t preferredFrameTimelineIndex; + VsyncEventData vsyncEventData; const Choreographer* choreographer; }; @@ -450,8 +448,7 @@ bool Choreographer::inCallback() const { ChoreographerFrameCallbackDataImpl Choreographer::createFrameCallbackData(nsecs_t timestamp) const { return {.frameTimeNanos = timestamp, - .preferredFrameTimelineIndex = mLastVsyncEventData.preferredFrameTimelineIndex, - .frameTimelines = mLastVsyncEventData.frameTimelines, + .vsyncEventData = mLastVsyncEventData, .choreographer = this}; } @@ -634,7 +631,7 @@ size_t AChoreographerFrameCallbackData_getFrameTimelinesLength( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - return frameCallbackData->frameTimelines.size(); + return VsyncEventData::kFrameTimelinesLength; } size_t AChoreographerFrameCallbackData_getPreferredFrameTimelineIndex( const AChoreographerFrameCallbackData* data) { @@ -642,7 +639,7 @@ size_t AChoreographerFrameCallbackData_getPreferredFrameTimelineIndex( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - return frameCallbackData->preferredFrameTimelineIndex; + return frameCallbackData->vsyncEventData.preferredFrameTimelineIndex; } AVsyncId AChoreographerFrameCallbackData_getFrameTimelineVsyncId( const AChoreographerFrameCallbackData* data, size_t index) { @@ -650,8 +647,8 @@ AVsyncId AChoreographerFrameCallbackData_getFrameTimelineVsyncId( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= frameCallbackData->frameTimelines.size(), "Index out of bounds"); - return frameCallbackData->frameTimelines[index].id; + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + return frameCallbackData->vsyncEventData.frameTimelines[index].vsyncId; } int64_t AChoreographerFrameCallbackData_getFrameTimelineExpectedPresentTimeNanos( const AChoreographerFrameCallbackData* data, size_t index) { @@ -659,8 +656,8 @@ int64_t AChoreographerFrameCallbackData_getFrameTimelineExpectedPresentTimeNanos AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= frameCallbackData->frameTimelines.size(), "Index out of bounds"); - return frameCallbackData->frameTimelines[index].expectedPresentTime; + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + return frameCallbackData->vsyncEventData.frameTimelines[index].expectedPresentationTime; } int64_t AChoreographerFrameCallbackData_getFrameTimelineDeadlineNanos( const AChoreographerFrameCallbackData* data, size_t index) { @@ -668,8 +665,8 @@ int64_t AChoreographerFrameCallbackData_getFrameTimelineDeadlineNanos( AChoreographerFrameCallbackData_to_ChoreographerFrameCallbackDataImpl(data); LOG_ALWAYS_FATAL_IF(!frameCallbackData->choreographer->inCallback(), "Data is only valid in callback"); - LOG_ALWAYS_FATAL_IF(index >= frameCallbackData->frameTimelines.size(), "Index out of bounds"); - return frameCallbackData->frameTimelines[index].deadlineTimestamp; + LOG_ALWAYS_FATAL_IF(index >= VsyncEventData::kFrameTimelinesLength, "Index out of bounds"); + return frameCallbackData->vsyncEventData.frameTimelines[index].deadlineTimestamp; } AChoreographer* AChoreographer_create() { diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp index 2d0da4643d..5ba8a1b449 100644 --- a/services/surfaceflinger/Scheduler/EventThread.cpp +++ b/services/surfaceflinger/Scheduler/EventThread.cpp @@ -87,9 +87,10 @@ std::string toString(const DisplayEventReceiver::Event& event) { to_string(event.header.displayId).c_str(), event.hotplug.connected ? "connected" : "disconnected"); case DisplayEventReceiver::DISPLAY_EVENT_VSYNC: - return StringPrintf("VSync{displayId=%s, count=%u, expectedVSyncTimestamp=%" PRId64 "}", + return StringPrintf("VSync{displayId=%s, count=%u, expectedPresentationTime=%" PRId64 + "}", to_string(event.header.displayId).c_str(), event.vsync.count, - event.vsync.expectedVSyncTimestamp); + event.vsync.vsyncData.preferredExpectedPresentationTime()); case DisplayEventReceiver::DISPLAY_EVENT_MODE_CHANGE: return StringPrintf("ModeChanged{displayId=%s, modeId=%u}", to_string(event.header.displayId).c_str(), event.modeChange.modeId); @@ -107,13 +108,19 @@ DisplayEventReceiver::Event makeHotplug(PhysicalDisplayId displayId, nsecs_t tim } DisplayEventReceiver::Event makeVSync(PhysicalDisplayId displayId, nsecs_t timestamp, - uint32_t count, nsecs_t expectedVSyncTimestamp, + uint32_t count, nsecs_t expectedPresentationTime, nsecs_t deadlineTimestamp) { DisplayEventReceiver::Event event; event.header = {DisplayEventReceiver::DISPLAY_EVENT_VSYNC, displayId, timestamp}; event.vsync.count = count; - event.vsync.expectedVSyncTimestamp = expectedVSyncTimestamp; - event.vsync.deadlineTimestamp = deadlineTimestamp; + event.vsync.vsyncData.preferredFrameTimelineIndex = 0; + // Temporarily store the current vsync information in frameTimelines[0], marked as + // platform-preferred. When the event is dispatched later, the frame interval at that time is + // used with this information to generate multiple frame timeline choices. + event.vsync.vsyncData.frameTimelines[0] = {.vsyncId = FrameTimelineInfo::INVALID_VSYNC_ID, + .deadlineTimestamp = deadlineTimestamp, + .expectedPresentationTime = + expectedPresentationTime}; return event; } @@ -186,9 +193,10 @@ binder::Status EventThreadConnection::requestNextVsync() { return binder::Status::ok(); } -binder::Status EventThreadConnection::getLatestVsyncEventData(VsyncEventData* outVsyncEventData) { +binder::Status EventThreadConnection::getLatestVsyncEventData( + ParcelableVsyncEventData* outVsyncEventData) { ATRACE_CALL(); - *outVsyncEventData = mEventThread->getLatestVsyncEventData(this); + outVsyncEventData->vsync = mEventThread->getLatestVsyncEventData(this); return binder::Status::ok(); } @@ -338,10 +346,16 @@ void EventThread::requestNextVsync(const sp& connection) VsyncEventData EventThread::getLatestVsyncEventData( const sp& connection) const { - nsecs_t frameInterval = mGetVsyncPeriodFunction(connection->mOwnerUid); VsyncEventData vsyncEventData; + nsecs_t frameInterval = mGetVsyncPeriodFunction(connection->mOwnerUid); vsyncEventData.frameInterval = frameInterval; - generateFrameTimeline(vsyncEventData, frameInterval, systemTime(SYSTEM_TIME_MONOTONIC)); + VSyncSource::VSyncData vsyncData; + { + std::lock_guard lock(mMutex); + vsyncData = mVSyncSource->getLatestVSyncData(); + } + generateFrameTimeline(vsyncEventData, frameInterval, systemTime(SYSTEM_TIME_MONOTONIC), + vsyncData.expectedPresentationTime, vsyncData.deadlineTimestamp); return vsyncEventData; } @@ -370,7 +384,7 @@ void EventThread::onVSyncEvent(nsecs_t timestamp, VSyncSource::VSyncData vsyncDa LOG_FATAL_IF(!mVSyncState); mPendingEvents.push_back(makeVSync(mVSyncState->displayId, timestamp, ++mVSyncState->count, - vsyncData.expectedVSyncTimestamp, + vsyncData.expectedPresentationTime, vsyncData.deadlineTimestamp)); mCondition.notify_all(); } @@ -518,7 +532,8 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event, const sp& connection) const { const auto throttleVsync = [&] { return mThrottleVsyncCallback && - mThrottleVsyncCallback(event.vsync.expectedVSyncTimestamp, connection->mOwnerUid); + mThrottleVsyncCallback(event.vsync.vsyncData.preferredExpectedPresentationTime(), + connection->mOwnerUid); }; switch (event.header.type) { @@ -568,79 +583,49 @@ bool EventThread::shouldConsumeEvent(const DisplayEventReceiver::Event& event, } int64_t EventThread::generateToken(nsecs_t timestamp, nsecs_t deadlineTimestamp, - nsecs_t expectedVSyncTimestamp) const { + nsecs_t expectedPresentationTime) const { if (mTokenManager != nullptr) { return mTokenManager->generateTokenForPredictions( - {timestamp, deadlineTimestamp, expectedVSyncTimestamp}); + {timestamp, deadlineTimestamp, expectedPresentationTime}); } return FrameTimelineInfo::INVALID_VSYNC_ID; } -void EventThread::generateFrameTimeline( - nsecs_t frameInterval, nsecs_t timestamp, nsecs_t preferredExpectedVSyncTimestamp, - nsecs_t preferredDeadlineTimestamp, - std::function setPreferredFrameTimelineIndex, - std::function - setFrameTimeline) const { +void EventThread::generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs_t frameInterval, + nsecs_t timestamp, + nsecs_t preferredExpectedPresentationTime, + nsecs_t preferredDeadlineTimestamp) const { // Add 1 to ensure the preferredFrameTimelineIndex entry (when multiplier == 0) is included. for (int64_t multiplier = -VsyncEventData::kFrameTimelinesLength + 1, currentIndex = 0; currentIndex < VsyncEventData::kFrameTimelinesLength; multiplier++) { - nsecs_t deadline = preferredDeadlineTimestamp + multiplier * frameInterval; + nsecs_t deadlineTimestamp = preferredDeadlineTimestamp + multiplier * frameInterval; // Valid possible frame timelines must have future values. - if (deadline > timestamp) { + if (deadlineTimestamp > timestamp) { if (multiplier == 0) { - setPreferredFrameTimelineIndex(currentIndex); + outVsyncEventData.preferredFrameTimelineIndex = currentIndex; } - nsecs_t expectedVSyncTimestamp = - preferredExpectedVSyncTimestamp + multiplier * frameInterval; - setFrameTimeline(currentIndex, - generateToken(timestamp, deadline, expectedVSyncTimestamp), - expectedVSyncTimestamp, deadline); + nsecs_t expectedPresentationTime = + preferredExpectedPresentationTime + multiplier * frameInterval; + outVsyncEventData.frameTimelines[currentIndex] = + {.vsyncId = + generateToken(timestamp, deadlineTimestamp, expectedPresentationTime), + .deadlineTimestamp = deadlineTimestamp, + .expectedPresentationTime = expectedPresentationTime}; currentIndex++; } } } -void EventThread::generateFrameTimeline(DisplayEventReceiver::Event& event) const { - generateFrameTimeline( - event.vsync.frameInterval, event.header.timestamp, event.vsync.expectedVSyncTimestamp, - event.vsync.deadlineTimestamp, - [&](int index) { event.vsync.preferredFrameTimelineIndex = index; }, - [&](int64_t index, int64_t vsyncId, nsecs_t expectedVSyncTimestamp, - nsecs_t deadlineTimestamp) { - event.vsync.frameTimelines[index] = {.vsyncId = vsyncId, - .deadlineTimestamp = deadlineTimestamp, - .expectedVSyncTimestamp = - expectedVSyncTimestamp}; - }); -} - -void EventThread::generateFrameTimeline(VsyncEventData& out, const nsecs_t frameInterval, - const nsecs_t timestamp) const { - VSyncSource::VSyncData vsyncData; - { - std::lock_guard lock(mMutex); - vsyncData = mVSyncSource->getLatestVSyncData(); - } - generateFrameTimeline( - frameInterval, timestamp, vsyncData.expectedVSyncTimestamp, vsyncData.deadlineTimestamp, - [&](int index) { out.preferredFrameTimelineIndex = index; }, - [&](int64_t index, int64_t vsyncId, nsecs_t expectedVSyncTimestamp, - nsecs_t deadlineTimestamp) { - out.frameTimelines[index] = - VsyncEventData::FrameTimeline(vsyncId, deadlineTimestamp, - expectedVSyncTimestamp); - }); -} - void EventThread::dispatchEvent(const DisplayEventReceiver::Event& event, const DisplayEventConsumers& consumers) { for (const auto& consumer : consumers) { DisplayEventReceiver::Event copy = event; if (event.header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { - copy.vsync.frameInterval = mGetVsyncPeriodFunction(consumer->mOwnerUid); - generateFrameTimeline(copy); + const int64_t frameInterval = mGetVsyncPeriodFunction(consumer->mOwnerUid); + copy.vsync.vsyncData.frameInterval = frameInterval; + generateFrameTimeline(copy.vsync.vsyncData, frameInterval, copy.header.timestamp, + event.vsync.vsyncData.preferredExpectedPresentationTime(), + event.vsync.vsyncData.preferredDeadlineTimestamp()); } switch (consumer->postEvent(copy)) { case NO_ERROR: diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h index a858169e65..c406478c17 100644 --- a/services/surfaceflinger/Scheduler/EventThread.h +++ b/services/surfaceflinger/Scheduler/EventThread.h @@ -45,6 +45,7 @@ namespace frametimeline { class TokenManager; } // namespace frametimeline +using gui::ParcelableVsyncEventData; using gui::VsyncEventData; // --------------------------------------------------------------------------- @@ -66,7 +67,7 @@ class VSyncSource { public: class VSyncData { public: - nsecs_t expectedVSyncTimestamp; + nsecs_t expectedPresentationTime; nsecs_t deadlineTimestamp; }; @@ -99,7 +100,7 @@ public: binder::Status stealReceiveChannel(gui::BitTube* outChannel) override; binder::Status setVsyncRate(int rate) override; binder::Status requestNextVsync() override; // asynchronous - binder::Status getLatestVsyncEventData(VsyncEventData* outVsyncEventData) override; + binder::Status getLatestVsyncEventData(ParcelableVsyncEventData* outVsyncEventData) override; // Called in response to requestNextVsync. const ResyncCallback resyncCallback; @@ -217,17 +218,10 @@ private: void onVSyncEvent(nsecs_t timestamp, VSyncSource::VSyncData vsyncData) override; int64_t generateToken(nsecs_t timestamp, nsecs_t deadlineTimestamp, - nsecs_t expectedVSyncTimestamp) const; - void generateFrameTimeline(DisplayEventReceiver::Event& event) const; - void generateFrameTimeline(VsyncEventData& out, const nsecs_t frameInterval, - const nsecs_t timestamp) const; - void generateFrameTimeline( - nsecs_t frameInterval, nsecs_t timestamp, nsecs_t preferredExpectedVSyncTimestamp, - nsecs_t preferredDeadlineTimestamp, - std::function setPreferredFrameTimelineIndex, - std::function - setFrameTimeline) const; + nsecs_t expectedPresentationTime) const; + void generateFrameTimeline(VsyncEventData& outVsyncEventData, nsecs_t frameInterval, + nsecs_t timestamp, nsecs_t preferredExpectedPresentationTime, + nsecs_t preferredDeadlineTimestamp) const; const std::unique_ptr mVSyncSource GUARDED_BY(mMutex); frametimeline::TokenManager* const mTokenManager; diff --git a/services/surfaceflinger/Scheduler/MessageQueue.cpp b/services/surfaceflinger/Scheduler/MessageQueue.cpp index a020e2c834..712cd5bdf3 100644 --- a/services/surfaceflinger/Scheduler/MessageQueue.cpp +++ b/services/surfaceflinger/Scheduler/MessageQueue.cpp @@ -191,7 +191,8 @@ void MessageQueue::injectorCallback() { for (int i = 0; i < n; i++) { if (buffer[i].header.type == DisplayEventReceiver::DISPLAY_EVENT_VSYNC) { auto& vsync = buffer[i].vsync; - mHandler->dispatchFrame(vsync.vsyncId, vsync.expectedVSyncTimestamp); + mHandler->dispatchFrame(vsync.vsyncData.preferredVsyncId(), + vsync.vsyncData.preferredExpectedPresentationTime()); break; } } diff --git a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp index 01adbc8962..0e54664f77 100644 --- a/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp +++ b/services/surfaceflinger/tests/DisplayEventReceiver_test.cpp @@ -28,23 +28,24 @@ public: TEST_F(DisplayEventReceiverTest, getLatestVsyncEventData) { const nsecs_t now = systemTime(); - VsyncEventData vsyncEventData; - EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.getLatestVsyncEventData(&vsyncEventData)); + ParcelableVsyncEventData parcelableVsyncEventData; + EXPECT_EQ(NO_ERROR, mDisplayEventReceiver.getLatestVsyncEventData(&parcelableVsyncEventData)); + const VsyncEventData& vsyncEventData = parcelableVsyncEventData.vsync; EXPECT_NE(std::numeric_limits::max(), vsyncEventData.preferredFrameTimelineIndex); EXPECT_GT(vsyncEventData.frameTimelines[0].deadlineTimestamp, now) << "Deadline timestamp should be greater than frame time"; - for (size_t i = 0; i < vsyncEventData.frameTimelines.size(); i++) { - EXPECT_NE(FrameTimelineInfo::INVALID_VSYNC_ID, vsyncEventData.frameTimelines[i].id); - EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { + EXPECT_NE(FrameTimelineInfo::INVALID_VSYNC_ID, vsyncEventData.frameTimelines[i].vsyncId); + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentationTime, vsyncEventData.frameTimelines[i].deadlineTimestamp) << "Expected vsync timestamp should be greater than deadline"; if (i > 0) { EXPECT_GT(vsyncEventData.frameTimelines[i].deadlineTimestamp, vsyncEventData.frameTimelines[i - 1].deadlineTimestamp) << "Deadline timestamp out of order for frame timeline " << i; - EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, - vsyncEventData.frameTimelines[i - 1].expectedPresentTime) + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentationTime, + vsyncEventData.frameTimelines[i - 1].expectedPresentationTime) << "Expected vsync timestamp out of order for frame timeline " << i; } } diff --git a/services/surfaceflinger/tests/LayerCallback_test.cpp b/services/surfaceflinger/tests/LayerCallback_test.cpp index 5c16feeda8..8a2305b365 100644 --- a/services/surfaceflinger/tests/LayerCallback_test.cpp +++ b/services/surfaceflinger/tests/LayerCallback_test.cpp @@ -141,11 +141,12 @@ public: continue; } - vsync = {event.vsync.vsyncId, event.vsync.expectedVSyncTimestamp}; + vsync = {event.vsync.vsyncData.preferredVsyncId(), + event.vsync.vsyncData.preferredExpectedPresentationTime()}; } EXPECT_GE(vsync.vsyncId, 1); - EXPECT_GT(event.vsync.expectedVSyncTimestamp, systemTime()); + EXPECT_GT(vsync.expectedPresentTime, systemTime()); return vsync; } diff --git a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp index 0b6b475222..ec27edac6e 100644 --- a/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp +++ b/services/surfaceflinger/tests/unittests/DispSyncSourceTest.cpp @@ -229,7 +229,7 @@ TEST_F(DispSyncSourceTest, waitForCallbacks) { ASSERT_TRUE(callbackData.has_value()); const auto [when, vsyncData] = callbackData.value(); EXPECT_EQ(when, - vsyncData.expectedVSyncTimestamp - mWorkDuration.count() - + vsyncData.expectedPresentationTime - mWorkDuration.count() - mReadyDuration.count()); } } @@ -261,7 +261,7 @@ TEST_F(DispSyncSourceTest, waitForCallbacksWithDurationChange) { ASSERT_TRUE(callbackData.has_value()); const auto [when, vsyncData] = callbackData.value(); EXPECT_EQ(when, - vsyncData.expectedVSyncTimestamp - mWorkDuration.count() - + vsyncData.expectedPresentationTime - mWorkDuration.count() - mReadyDuration.count()); } @@ -283,7 +283,7 @@ TEST_F(DispSyncSourceTest, waitForCallbacksWithDurationChange) { const auto callbackData = mVSyncEventCallRecorder.waitForCall(); ASSERT_TRUE(callbackData.has_value()); const auto [when, vsyncData] = callbackData.value(); - EXPECT_EQ(when, vsyncData.expectedVSyncTimestamp - newDuration.count()); + EXPECT_EQ(when, vsyncData.expectedPresentationTime - newDuration.count()); } EXPECT_CALL(*mVSyncDispatch, cancel(_)).Times(1); @@ -307,9 +307,9 @@ TEST_F(DispSyncSourceTest, getLatestVsyncData) { const auto vsyncData = mDispSyncSource->getLatestVSyncData(); ASSERT_GT(vsyncData.deadlineTimestamp, now); - ASSERT_GT(vsyncData.expectedVSyncTimestamp, vsyncData.deadlineTimestamp); + ASSERT_GT(vsyncData.expectedPresentationTime, vsyncData.deadlineTimestamp); EXPECT_EQ(vsyncData.deadlineTimestamp, - vsyncData.expectedVSyncTimestamp - vsyncInternalDuration); + vsyncData.expectedPresentationTime - vsyncInternalDuration); } } // namespace diff --git a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp index cc0a40f74b..14d8f987b0 100644 --- a/services/surfaceflinger/tests/unittests/EventThreadTest.cpp +++ b/services/surfaceflinger/tests/unittests/EventThreadTest.cpp @@ -99,7 +99,7 @@ protected: nsecs_t expectedTimestamp, unsigned expectedCount); void expectVsyncEventReceivedByConnection(nsecs_t expectedTimestamp, unsigned expectedCount); void expectVsyncEventFrameTimelinesCorrect(nsecs_t expectedTimestamp, - nsecs_t preferredDeadline); + VSyncSource::VSyncData preferredVsyncData); void expectHotplugEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, bool expectedConnected); void expectConfigChangedEventReceivedByConnection(PhysicalDisplayId expectedDisplayId, @@ -252,40 +252,42 @@ void EventThreadTest::expectVsyncEventReceivedByConnection(nsecs_t expectedTimes expectedCount); } -void EventThreadTest::expectVsyncEventFrameTimelinesCorrect(nsecs_t expectedTimestamp, - nsecs_t preferredDeadline) { +void EventThreadTest::expectVsyncEventFrameTimelinesCorrect( + nsecs_t expectedTimestamp, VSyncSource::VSyncData preferredVsyncData) { auto args = mConnectionEventCallRecorder.waitForCall(); ASSERT_TRUE(args.has_value()) << " did not receive an event for timestamp " << expectedTimestamp; const auto& event = std::get<0>(args.value()); for (int i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { - auto prediction = - mTokenManager->getPredictionsForToken(event.vsync.frameTimelines[i].vsyncId); + auto prediction = mTokenManager->getPredictionsForToken( + event.vsync.vsyncData.frameTimelines[i].vsyncId); EXPECT_TRUE(prediction.has_value()); - EXPECT_EQ(prediction.value().endTime, event.vsync.frameTimelines[i].deadlineTimestamp) + EXPECT_EQ(prediction.value().endTime, + event.vsync.vsyncData.frameTimelines[i].deadlineTimestamp) << "Deadline timestamp does not match cached value"; EXPECT_EQ(prediction.value().presentTime, - event.vsync.frameTimelines[i].expectedVSyncTimestamp) - << "Expected vsync timestamp does not match cached value"; + event.vsync.vsyncData.frameTimelines[i].expectedPresentationTime) + << "Expected vsync.vsyncData timestamp does not match cached value"; if (i > 0) { - EXPECT_GT(event.vsync.frameTimelines[i].deadlineTimestamp, - event.vsync.frameTimelines[i - 1].deadlineTimestamp) + EXPECT_GT(event.vsync.vsyncData.frameTimelines[i].deadlineTimestamp, + event.vsync.vsyncData.frameTimelines[i - 1].deadlineTimestamp) << "Deadline timestamp out of order for frame timeline " << i; - EXPECT_GT(event.vsync.frameTimelines[i].expectedVSyncTimestamp, - event.vsync.frameTimelines[i - 1].expectedVSyncTimestamp) - << "Expected vsync timestamp out of order for frame timeline " << i; + EXPECT_GT(event.vsync.vsyncData.frameTimelines[i].expectedPresentationTime, + event.vsync.vsyncData.frameTimelines[i - 1].expectedPresentationTime) + << "Expected vsync.vsyncData timestamp out of order for frame timeline " << i; } // Vsync ID order lines up with registration into test token manager. - EXPECT_EQ(i, event.vsync.frameTimelines[i].vsyncId) + EXPECT_EQ(i, event.vsync.vsyncData.frameTimelines[i].vsyncId) << "Vsync ID incorrect for frame timeline " << i; - if (i == event.vsync.preferredFrameTimelineIndex) { - EXPECT_EQ(event.vsync.frameTimelines[i].deadlineTimestamp, preferredDeadline) + if (i == event.vsync.vsyncData.preferredFrameTimelineIndex) { + EXPECT_EQ(event.vsync.vsyncData.frameTimelines[i].deadlineTimestamp, + preferredVsyncData.deadlineTimestamp) << "Preferred deadline timestamp incorrect" << i; - EXPECT_EQ(event.vsync.frameTimelines[i].expectedVSyncTimestamp, - event.vsync.expectedVSyncTimestamp) - << "Preferred expected vsync timestamp incorrect" << i; + EXPECT_EQ(event.vsync.vsyncData.frameTimelines[i].expectedPresentationTime, + preferredVsyncData.expectedPresentationTime) + << "Preferred expected vsync.vsyncData timestamp incorrect" << i; } } } @@ -397,16 +399,17 @@ TEST_F(EventThreadTest, requestNextVsyncEventFrameTimelinesCorrect) { // Use the received callback to signal a vsync event. // The interceptor should receive the event, as well as the connection. - mCallback->onVSyncEvent(123, {456, 789}); + VSyncSource::VSyncData vsyncData = {456, 789}; + mCallback->onVSyncEvent(123, vsyncData); expectInterceptCallReceived(123); - expectVsyncEventFrameTimelinesCorrect(123, 789); + expectVsyncEventFrameTimelinesCorrect(123, vsyncData); } TEST_F(EventThreadTest, getLatestVsyncEventData) { const nsecs_t now = systemTime(); const nsecs_t preferredDeadline = now + 10000000; - const nsecs_t preferredExpectedVSyncTimestamp = now + 20000000; - const VSyncSource::VSyncData preferredData = {preferredExpectedVSyncTimestamp, + const nsecs_t preferredExpectedPresentationTime = now + 20000000; + const VSyncSource::VSyncData preferredData = {preferredExpectedPresentationTime, preferredDeadline}; EXPECT_CALL(*mVSyncSource, getLatestVSyncData()).WillOnce(Return(preferredData)); @@ -415,14 +418,14 @@ TEST_F(EventThreadTest, getLatestVsyncEventData) { << "Deadline timestamp should be greater than frame time"; for (size_t i = 0; i < VsyncEventData::kFrameTimelinesLength; i++) { auto prediction = - mTokenManager->getPredictionsForToken(vsyncEventData.frameTimelines[i].id); + mTokenManager->getPredictionsForToken(vsyncEventData.frameTimelines[i].vsyncId); EXPECT_TRUE(prediction.has_value()); EXPECT_EQ(prediction.value().endTime, vsyncEventData.frameTimelines[i].deadlineTimestamp) << "Deadline timestamp does not match cached value"; EXPECT_EQ(prediction.value().presentTime, - vsyncEventData.frameTimelines[i].expectedPresentTime) + vsyncEventData.frameTimelines[i].expectedPresentationTime) << "Expected vsync timestamp does not match cached value"; - EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentationTime, vsyncEventData.frameTimelines[i].deadlineTimestamp) << "Expected vsync timestamp should be greater than deadline"; @@ -430,19 +433,19 @@ TEST_F(EventThreadTest, getLatestVsyncEventData) { EXPECT_GT(vsyncEventData.frameTimelines[i].deadlineTimestamp, vsyncEventData.frameTimelines[i - 1].deadlineTimestamp) << "Deadline timestamp out of order for frame timeline " << i; - EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentTime, - vsyncEventData.frameTimelines[i - 1].expectedPresentTime) + EXPECT_GT(vsyncEventData.frameTimelines[i].expectedPresentationTime, + vsyncEventData.frameTimelines[i - 1].expectedPresentationTime) << "Expected vsync timestamp out of order for frame timeline " << i; } // Vsync ID order lines up with registration into test token manager. - EXPECT_EQ(i, vsyncEventData.frameTimelines[i].id) + EXPECT_EQ(i, vsyncEventData.frameTimelines[i].vsyncId) << "Vsync ID incorrect for frame timeline " << i; if (i == vsyncEventData.preferredFrameTimelineIndex) { EXPECT_EQ(vsyncEventData.frameTimelines[i].deadlineTimestamp, preferredDeadline) << "Preferred deadline timestamp incorrect" << i; - EXPECT_EQ(vsyncEventData.frameTimelines[i].expectedPresentTime, - preferredExpectedVSyncTimestamp) + EXPECT_EQ(vsyncEventData.frameTimelines[i].expectedPresentationTime, + preferredExpectedPresentationTime) << "Preferred expected vsync timestamp incorrect" << i; } } -- cgit v1.2.3-59-g8ed1b