Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 95619432 authored by Adithya Srinivasan's avatar Adithya Srinivasan
Browse files

Return janktype properly for dropped frames 

In SurfaceFrame::getJankType(), a dropped frame won't have any present
time set and will return std::nullopt currently. This behavior can
result in a memory leak in releasePendingBuffer. Whenever SurfaceFrames
are created, they are added to a deque 'mPendingJankClassification'.
This deque is popped from the front as and when the frames are
presented. However, if a dropped frame is in the front of the deque, the
deque keeps growing in size to infinity. Since getJankType() reports a
std::nullopt, it assumes that the frame hasn't been presented yet and
keeps waiting for it.

Bug: 179701581
Test: libsurfaceflinger_unittest
Change-Id: I8ad1ed07222d39e4f98e4f7f3178db9fe52ea712
parent e06403fa

services/surfaceflinger/FrameTimeline/FrameTimeline.cpp

+5 −0
Original line number Diff line number Diff line
@@ -327,7 +327,12 @@ void SurfaceFrame::setRenderRate(Fps renderRate) { 


std::optional<int32_t> SurfaceFrame::getJankType() const {

    std::scoped_lock lock(mMutex);

    if (mPresentState == PresentState::Dropped) {

        // Return no jank if it's a dropped frame since we cannot attribute a jank to a it.

        return JankType::None;

    }

    if (mActuals.presentTime == 0) {

        // Frame hasn't been presented yet.

        return std::nullopt;

    }

    return mJankType;

services/surfaceflinger/tests/unittests/TransactionSurfaceFrameTest.cpp

+38 −0
Original line number Diff line number Diff line
@@ -328,6 +328,41 @@ public: 
        EXPECT_EQ(PresentState::Presented, bufferlessSurfaceFrame2->getPresentState());

        EXPECT_EQ(12, bufferlessSurfaceFrame2->getActuals().endTime);

    }



    void PendingSurfaceFramesRemovedAfterClassification() {

        sp<BufferStateLayer> layer = createBufferStateLayer();



        sp<Fence> fence1(new Fence());

        auto acquireFence1 = fenceFactory.createFenceTimeForTest(fence1);

        sp<GraphicBuffer> buffer1{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};

        layer->setBuffer(buffer1, fence1, 10, 20, false, mClientCache, 1, std::nullopt,

                         {/*vsyncId*/ 1, /*inputEventId*/ 0});

        ASSERT_NE(nullptr, layer->mCurrentState.bufferSurfaceFrameTX);

        const auto droppedSurfaceFrame = layer->mCurrentState.bufferSurfaceFrameTX;



        sp<Fence> fence2(new Fence());

        auto acquireFence2 = fenceFactory.createFenceTimeForTest(fence2);

        sp<GraphicBuffer> buffer2{new GraphicBuffer(1, 1, HAL_PIXEL_FORMAT_RGBA_8888, 1, 0)};

        layer->setBuffer(buffer2, fence2, 10, 20, false, mClientCache, 1, std::nullopt,

                         {/*vsyncId*/ 1, /*inputEventId*/ 0});

        acquireFence2->signalForTest(12);



        ASSERT_NE(nullptr, layer->mCurrentState.bufferSurfaceFrameTX);

        auto& presentedSurfaceFrame = layer->mCurrentState.bufferSurfaceFrameTX;



        commitTransaction(layer.get());

        bool computeVisisbleRegions;

        layer->updateTexImage(computeVisisbleRegions, 15, 0);



        // Both the droppedSurfaceFrame and presentedSurfaceFrame should be in

        // pendingJankClassifications.

        EXPECT_EQ(2u, layer->mPendingJankClassifications.size());

        presentedSurfaceFrame->onPresent(20, JankType::None, Fps::fromPeriodNsecs(11),

                                         /*displayDeadlineDelta*/ 0, /*displayPresentDelta*/ 0);

        layer->releasePendingBuffer(25);



        EXPECT_EQ(0u, layer->mPendingJankClassifications.size());

    }

};



TEST_F(TransactionSurfaceFrameTest, PresentedBufferlessSurfaceFrame) {
@@ -364,4 +399,7 @@ TEST_F(TransactionSurfaceFrameTest, 
    MergePendingStates_BufferlessSurfaceFramesWithOverlappingToken();

}



TEST_F(TransactionSurfaceFrameTest, PendingSurfaceFramesRemovedAfterClassification) {

    PendingSurfaceFramesRemovedAfterClassification();

}

} // namespace android
 
 No newline at end of file