From 6cdd3fd7dab0149041ee8dd7273fe83888d2b1e3 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Thu, 7 Sep 2023 18:45:58 -0700 Subject: libgui: plumb setFrameRate thru BufferQueue The current implementation just assumes that the Surface and BLASTBufferQueue lives in the same process and rely on inheritance to handle setFrameRate. This doesn't work for any usecase that the Surface is Parceled to a diffrent process. Bug: 281695725 Test: atest CtsGraphicsTestCases --test-filter SetFrameRateTest* Change-Id: I4e08b92b618fa7b863ca3ef4f7b46d9f1c30c775 --- libs/gui/FrameRateUtils.cpp | 65 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 libs/gui/FrameRateUtils.cpp (limited to 'libs/gui/FrameRateUtils.cpp') diff --git a/libs/gui/FrameRateUtils.cpp b/libs/gui/FrameRateUtils.cpp new file mode 100644 index 0000000000..6993bfab45 --- /dev/null +++ b/libs/gui/FrameRateUtils.cpp @@ -0,0 +1,65 @@ +/* + * Copyright 2023 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 +#include +#include + +#include + +namespace android { +// Returns true if the frameRate is valid. +// +// @param frameRate the frame rate in Hz +// @param compatibility a ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_* +// @param changeFrameRateStrategy a ANATIVEWINDOW_CHANGE_FRAME_RATE_* +// @param functionName calling function or nullptr. Used for logging +// @param privileged whether caller has unscoped surfaceflinger access +bool ValidateFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy, + const char* inFunctionName, bool privileged) { + const char* functionName = inFunctionName != nullptr ? inFunctionName : "call"; + int floatClassification = std::fpclassify(frameRate); + if (frameRate < 0 || floatClassification == FP_INFINITE || floatClassification == FP_NAN) { + ALOGE("%s failed - invalid frame rate %f", functionName, frameRate); + return false; + } + + if (compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_DEFAULT && + compatibility != ANATIVEWINDOW_FRAME_RATE_COMPATIBILITY_FIXED_SOURCE && + (!privileged || + (compatibility != ANATIVEWINDOW_FRAME_RATE_EXACT && + compatibility != ANATIVEWINDOW_FRAME_RATE_NO_VOTE))) { + ALOGE("%s failed - invalid compatibility value %d privileged: %s", functionName, + compatibility, privileged ? "yes" : "no"); + return false; + } + + if (__builtin_available(android 31, *)) { + if (changeFrameRateStrategy != ANATIVEWINDOW_CHANGE_FRAME_RATE_ONLY_IF_SEAMLESS && + changeFrameRateStrategy != ANATIVEWINDOW_CHANGE_FRAME_RATE_ALWAYS) { + ALOGE("%s failed - invalid change frame rate strategy value %d", functionName, + changeFrameRateStrategy); + if (FLAG_BQ_SET_FRAME_RATE) { + return false; + } + } + } + + return true; +} + +} // namespace android -- cgit v1.2.3-59-g8ed1b From 107788e8ce8f855b7555680e222f563810a51793 Mon Sep 17 00:00:00 2001 From: Ady Abraham Date: Tue, 17 Oct 2023 12:31:08 -0700 Subject: libgui: use flag MACRO for BQ_SETFRAMERATE Bug: 281695725 Test: presubmit Change-Id: I644dadbfc72cd50d80c50d36da96078253bbc009 --- libs/bufferqueueconverter/Android.bp | 2 ++ libs/gui/Android.bp | 38 ++++++++++++++-------- libs/gui/BLASTBufferQueue.cpp | 6 ++-- libs/gui/BufferQueue.cpp | 3 +- libs/gui/BufferQueueProducer.cpp | 4 +-- libs/gui/FrameRateUtils.cpp | 6 ++-- libs/gui/IGraphicBufferProducer.cpp | 8 ++--- libs/gui/Surface.cpp | 4 +-- libs/gui/include/gui/BLASTBufferQueue.h | 6 ++-- libs/gui/include/gui/BufferQueue.h | 6 ++-- libs/gui/include/gui/BufferQueueProducer.h | 4 +-- libs/gui/include/gui/Flags.h | 22 ------------- libs/gui/include/gui/IConsumerListener.h | 6 ++-- libs/gui/include/gui/IGraphicBufferProducer.h | 5 +-- libs/gui/tests/Android.bp | 2 +- libs/gui/tests/BufferQueue_test.cpp | 4 +-- .../include/surfacetexture/SurfaceTexture.h | 4 +-- .../surfacetexture/SurfaceTexture.cpp | 2 +- 18 files changed, 63 insertions(+), 69 deletions(-) delete mode 100644 libs/gui/include/gui/Flags.h (limited to 'libs/gui/FrameRateUtils.cpp') diff --git a/libs/bufferqueueconverter/Android.bp b/libs/bufferqueueconverter/Android.bp index d4605ea13a..3fe71cefce 100644 --- a/libs/bufferqueueconverter/Android.bp +++ b/libs/bufferqueueconverter/Android.bp @@ -34,5 +34,7 @@ cc_library { "libbase", "liblog", ], + static_libs: ["libguiflags"], export_include_dirs: ["include"], + export_static_lib_headers: ["libguiflags"], } diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp index f17a6547f7..c84ee1f9da 100644 --- a/libs/gui/Android.bp +++ b/libs/gui/Android.bp @@ -20,6 +20,25 @@ package { default_applicable_licenses: ["frameworks_native_license"], } +aconfig_declarations { + name: "libgui_flags", + package: "com.android.graphics.libgui.flags", + srcs: ["libgui_flags.aconfig"], +} + +cc_aconfig_library { + name: "libguiflags", + host_supported: true, + vendor_available: true, + min_sdk_version: "29", + apex_available: [ + "//apex_available:platform", + "com.android.media.swcodec", + "test_com.android.media.swcodec", + ], + aconfig_declarations: "libgui_flags", +} + cc_library_headers { name: "libgui_headers", vendor_available: true, @@ -36,6 +55,8 @@ cc_library_headers { "android.hardware.graphics.bufferqueue@1.0", "android.hardware.graphics.bufferqueue@2.0", ], + static_libs: ["libguiflags"], + export_static_lib_headers: ["libguiflags"], min_sdk_version: "29", // TODO(b/218719284) can media use be constrained to libgui_bufferqueue_static? apex_available: [ @@ -192,19 +213,6 @@ cc_library_static { }, } -aconfig_declarations { - name: "libgui_flags", - package: "com.android.graphics.libgui.flags", - srcs: ["libgui_flags.aconfig"], -} - -cc_aconfig_library { - name: "libguiflags", - host_supported: true, - vendor_available: true, - aconfig_declarations: "libgui_flags", -} - filegroup { name: "libgui-sources", srcs: [ @@ -265,6 +273,9 @@ cc_defaults { "libbinder", "libGLESv2", ], + export_static_lib_headers: [ + "libguiflags", + ], } cc_library_shared { @@ -460,6 +471,7 @@ cc_library_static { static_libs: [ "libgtest", "libgmock", + "libguiflags", ], srcs: [ diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp index dd0a028865..8d0331ebb5 100644 --- a/libs/gui/BLASTBufferQueue.cpp +++ b/libs/gui/BLASTBufferQueue.cpp @@ -26,7 +26,7 @@ #include #include #include -#include + #include #include #include @@ -41,8 +41,6 @@ #include #include -#include - using namespace com::android::graphics::libgui; using namespace std::chrono_literals; @@ -144,7 +142,7 @@ void BLASTBufferItemConsumer::onSidebandStreamChanged() { } } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void BLASTBufferItemConsumer::onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) { sp bbq = mBLASTBufferQueue.promote(); diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index ab0f6d213f..b0f6e69115 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -22,7 +22,6 @@ #include #include #include -#include namespace android { @@ -99,7 +98,7 @@ void BufferQueue::ProxyConsumerListener::addAndGetFrameTimestamps( } } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void BufferQueue::ProxyConsumerListener::onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) { sp listener(mConsumerListener.promote()); diff --git a/libs/gui/BufferQueueProducer.cpp b/libs/gui/BufferQueueProducer.cpp index 67dff6dec6..19693e37cf 100644 --- a/libs/gui/BufferQueueProducer.cpp +++ b/libs/gui/BufferQueueProducer.cpp @@ -32,7 +32,7 @@ #include #include #include -#include + #include #include #include @@ -1753,7 +1753,7 @@ status_t BufferQueueProducer::setAutoPrerotation(bool autoPrerotation) { return NO_ERROR; } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) status_t BufferQueueProducer::setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) { ATRACE_CALL(); diff --git a/libs/gui/FrameRateUtils.cpp b/libs/gui/FrameRateUtils.cpp index 6993bfab45..11524e2b51 100644 --- a/libs/gui/FrameRateUtils.cpp +++ b/libs/gui/FrameRateUtils.cpp @@ -14,14 +14,16 @@ * limitations under the License. */ -#include #include #include #include #include +#include + namespace android { +using namespace com::android::graphics::libgui; // Returns true if the frameRate is valid. // // @param frameRate the frame rate in Hz @@ -53,7 +55,7 @@ bool ValidateFrameRate(float frameRate, int8_t compatibility, int8_t changeFrame changeFrameRateStrategy != ANATIVEWINDOW_CHANGE_FRAME_RATE_ALWAYS) { ALOGE("%s failed - invalid change frame rate strategy value %d", functionName, changeFrameRateStrategy); - if (FLAG_BQ_SET_FRAME_RATE) { + if (flags::bq_setframerate()) { return false; } } diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index d0c09e481d..e81c098b85 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -28,7 +28,7 @@ #include #include -#include + #include #include #include @@ -763,7 +763,7 @@ public: } return result; } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) virtual status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override { Parcel data, reply; @@ -973,7 +973,7 @@ status_t IGraphicBufferProducer::setAutoPrerotation(bool autoPrerotation) { return INVALID_OPERATION; } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) status_t IGraphicBufferProducer::setFrameRate(float /*frameRate*/, int8_t /*compatibility*/, int8_t /*changeFrameRateStrategy*/) { // No-op for IGBP other than BufferQueue. @@ -1522,7 +1522,7 @@ status_t BnGraphicBufferProducer::onTransact( reply->writeInt32(result); return NO_ERROR; } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) case SET_FRAME_RATE: { CHECK_INTERFACE(IGraphicBuffer, data, reply); float frameRate = data.readFloat(); diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index a87f05357f..07a0cfed63 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -43,7 +43,7 @@ #include #include -#include + #include #include @@ -2571,7 +2571,7 @@ void Surface::ProducerListenerProxy::onBuffersDiscarded(const std::vectorsetFrameRate(frameRate, compatibility, changeFrameRateStrategy); diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h index 02d7c4d2ac..892215ec32 100644 --- a/libs/gui/include/gui/BLASTBufferQueue.h +++ b/libs/gui/include/gui/BLASTBufferQueue.h @@ -19,7 +19,7 @@ #include #include -#include + #include #include @@ -31,6 +31,8 @@ #include #include +#include + namespace android { class BLASTBufferQueue; @@ -59,7 +61,7 @@ public: protected: void onSidebandStreamChanged() override EXCLUDES(mMutex); -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override; #endif diff --git a/libs/gui/include/gui/BufferQueue.h b/libs/gui/include/gui/BufferQueue.h index 2756277f2c..0948c4d076 100644 --- a/libs/gui/include/gui/BufferQueue.h +++ b/libs/gui/include/gui/BufferQueue.h @@ -19,11 +19,13 @@ #include #include -#include + #include #include #include +#include + namespace android { class BufferQueue { @@ -70,7 +72,7 @@ public: void addAndGetFrameTimestamps( const NewFrameEventsEntry* newTimestamps, FrameEventHistoryDelta* outDelta) override; -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override; #endif diff --git a/libs/gui/include/gui/BufferQueueProducer.h b/libs/gui/include/gui/BufferQueueProducer.h index 38805d0221..de47483dca 100644 --- a/libs/gui/include/gui/BufferQueueProducer.h +++ b/libs/gui/include/gui/BufferQueueProducer.h @@ -18,7 +18,7 @@ #define ANDROID_GUI_BUFFERQUEUEPRODUCER_H #include -#include + #include namespace android { @@ -202,7 +202,7 @@ public: // See IGraphicBufferProducer::setAutoPrerotation virtual status_t setAutoPrerotation(bool autoPrerotation); -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) // See IGraphicBufferProducer::setFrameRate status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override; diff --git a/libs/gui/include/gui/Flags.h b/libs/gui/include/gui/Flags.h deleted file mode 100644 index a2cff56e97..0000000000 --- a/libs/gui/include/gui/Flags.h +++ /dev/null @@ -1,22 +0,0 @@ -/* - * Copyright 2023 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. - */ - -#pragma once - -// TODO(281695725): replace this with build time flags, whenever they are available -#ifndef FLAG_BQ_SET_FRAME_RATE -#define FLAG_BQ_SET_FRAME_RATE false -#endif \ No newline at end of file diff --git a/libs/gui/include/gui/IConsumerListener.h b/libs/gui/include/gui/IConsumerListener.h index e183bf2668..51d3959de7 100644 --- a/libs/gui/include/gui/IConsumerListener.h +++ b/libs/gui/include/gui/IConsumerListener.h @@ -19,13 +19,13 @@ #include #include -#include - #include #include #include +#include + namespace android { class BufferItem; @@ -93,7 +93,7 @@ public: virtual void addAndGetFrameTimestamps(const NewFrameEventsEntry* /*newTimestamps*/, FrameEventHistoryDelta* /*outDelta*/) {} -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) // Notifies the consumer of a setFrameRate call from the producer side. virtual void onSetFrameRate(float /*frameRate*/, int8_t /*compatibility*/, int8_t /*changeFrameRateStrategy*/) {} diff --git a/libs/gui/include/gui/IGraphicBufferProducer.h b/libs/gui/include/gui/IGraphicBufferProducer.h index 3562906870..7639e709ca 100644 --- a/libs/gui/include/gui/IGraphicBufferProducer.h +++ b/libs/gui/include/gui/IGraphicBufferProducer.h @@ -31,7 +31,6 @@ #include #include -#include #include #include @@ -42,6 +41,8 @@ #include #include +#include + namespace android { // ---------------------------------------------------------------------------- @@ -677,7 +678,7 @@ public: // the width and height used for dequeueBuffer will be additionally swapped. virtual status_t setAutoPrerotation(bool autoPrerotation); -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) // Sets the apps intended frame rate. virtual status_t setFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy); diff --git a/libs/gui/tests/Android.bp b/libs/gui/tests/Android.bp index 38c0eed474..6dcd501404 100644 --- a/libs/gui/tests/Android.bp +++ b/libs/gui/tests/Android.bp @@ -21,7 +21,7 @@ cc_test { "-Wall", "-Werror", "-Wno-extra", - "-DFLAG_BQ_SET_FRAME_RATE=true", + "-DCOM_ANDROID_GRAPHICS_LIBGUI_FLAGS_BQ_SETFRAMERATE=true", ], srcs: [ diff --git a/libs/gui/tests/BufferQueue_test.cpp b/libs/gui/tests/BufferQueue_test.cpp index 17aa5f1350..1410c7dce0 100644 --- a/libs/gui/tests/BufferQueue_test.cpp +++ b/libs/gui/tests/BufferQueue_test.cpp @@ -1266,9 +1266,7 @@ TEST_F(BufferQueueTest, TestProducerConnectDisconnect) { } TEST_F(BufferQueueTest, TestBqSetFrameRateFlagBuildTimeIsSet) { - if (flags::bq_setframerate()) { - ASSERT_EQ(true, FLAG_BQ_SET_FRAME_RATE); - } + ASSERT_EQ(flags::bq_setframerate(), COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE)); } struct BufferItemConsumerSetFrameRateListener : public BufferItemConsumer { diff --git a/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h b/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h index 32fb3508ff..099f47dbe1 100644 --- a/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h +++ b/libs/nativedisplay/include/surfacetexture/SurfaceTexture.h @@ -19,7 +19,7 @@ #include #include #include -#include + #include #include #include @@ -352,7 +352,7 @@ protected: /** * onSetFrameRate Notifies the consumer of a setFrameRate call from the producer side. */ -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) override; #endif diff --git a/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp b/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp index c2535e0bf6..3a09204878 100644 --- a/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp +++ b/libs/nativedisplay/surfacetexture/SurfaceTexture.cpp @@ -515,7 +515,7 @@ void SurfaceTexture::FrameAvailableListenerProxy::onFrameAvailable(const BufferI } } -#if FLAG_BQ_SET_FRAME_RATE +#if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BQ_SETFRAMERATE) void SurfaceTexture::onSetFrameRate(float frameRate, int8_t compatibility, int8_t changeFrameRateStrategy) { SFT_LOGV("onSetFrameRate: %.2f", frameRate); -- cgit v1.2.3-59-g8ed1b