Implement VisibilityThresholdModifier #173

ipavlidakis · 2023-10-04T09:45:43Z

🎯 Goal

Reduce CPU usage demands during a call with more than 5 participants.

📝 Summary

When in a call with more than 5 participants we use a GridView to show and arrange video tracks. This view leverages the onAppear & onDisappear hooks to disable tracks that are not on screen. Unfortunately, as we are using LazyVStack to wrap our video tracks, iOS preloads views that aren't on screen, resulting in rendering video tracks that aren't really on user's viewport.

🛠 Implementation

To resolve this problem, we introduce a ViewModifier that internally uses a GeometryReader to compute if it's in its parent's viewport. If not the View is considered hidden and we propagate a visibility change that stops renderings for the View's video track.

🎨 Showcase

Add relevant screenshots and/or videos/gifs to easily see what this PR changes, if applicable.

Before	After

Note

The smaller CPU Usage and Energy impact is because the app was unresponsive and render requests were dropping (as can be seen from the FPS counter).

🧪 Manual Testing Notes

Explain how this change can be tested manually, if applicable.

☑️ Contributor Checklist

I have signed the Stream CLA (required)
This change follows zero ⚠️ policy (required)
This change should receive manual QA
Changelog is updated with client-facing changes
New code is covered by unit tests
Comparison screenshots added for visual changes
Affected documentation updated (Docusaurus, tutorial, CMS)

🎁 Meme

martinmitrevski

According to the screenshots, the device would heat more with the new approach, only the FPS is better, right?

martinmitrevski · 2023-10-04T12:05:18Z

Sources/StreamVideoSwiftUI/CallView/CallView.swift

@@ -159,18 +159,11 @@ public struct CallView<Factory: ViewFactory>: View {
        viewFactory.makeVideoParticipantsView(
            viewModel: viewModel,
            availableSize: size,
-            onViewRendering: handleViewRendering(_:participant:),


Is it possible to do this without breaking changes to the public API?

I remove this one as it's not being used anywhere. We can definitely go ahead without this change but makes our API confusing as we are asking for something what we won't be using.

Other things to keep in mind when looking on the screenshots (as you may missed the note below them):

The before screenshot shows lower consumption because the app at that point was unresponsive. Nothing was updating on the screen and that was causing consumption to drop.

The after screenshot was captured after 55minutes in a call while the before it was on minute 18.

ok, you should sell it better then 😄

Sources/StreamVideoSwiftUI/CallView/ParticipantsGridView.swift

Sources/StreamVideoSwiftUI/ViewFactory.swift

ipavlidakis · 2023-10-04T12:17:54Z

According to the screenshots, the device would heat more with the new approach, only the FPS is better, right?

The energy impact isn't really representative for the thermal state. The device feels cooler with the latest changes. That doesn't mean that processing video isn't an energy impactful operation. The difference is that it takes more time to jump the thermal state levels.

Sources/StreamVideo/WebRTC/StreamVideoCaptureHandler.swift

martinmitrevski · 2023-10-05T08:06:48Z

Sources/StreamVideoSwiftUI/CallView/ParticipantsGridView.swift

-            } else {
-                VStack {
-                    participantsContent
+        GeometryReader { geometryProxy in


Seems like we might have an extra geometry reader? The availableSize value should also come from a geometry reader. Ideally, we should use only one.

availableSize is indeed coming from a GeometryReader in the CallView. We can't really use it here as we don't need only the size but the origin too.

We can potentially use only one GeometryReader but that will require a change on the API that will allow us to pass availableRect/availableViewport: CGRect instead of availableSize: CGSize.

I would say it's better to break the API at this point, since we are still in beta. Geometry readers can be expensive, best to use them only when needed. Wdyt?

Changing this one I think will incur many changes. How do you feel about having it in another PR?

Maybe another PR towards this PR, to see how much it is? Since if we do breaking changes, we should do them very soon.

A PR to this branch sounds good. Here it is #176

Sources/StreamVideoSwiftUI/CallView/VisibilityThresholdModifier.swift

StreamVideoTests/Utils/SerialActor_Tests.swift

docusaurus/docs/iOS/03-guides/10-view-slots.mdx

StreamVideoTests/Utils/SerialActor_Tests.swift

martinmitrevski · 2023-10-05T08:17:00Z

Sources/StreamVideo/WebRTC/StreamVideoCaptureHandler.swift

-        Task { [weak self] in
-            guard let self else { return }
+        Task { [serialActor] in
+            await serialActor.enqueue { [weak self] in


Not 100% sure why enqueuing async tasks helps performance, can you please elaborate one more time? 🙏

The idea is that Tasks not running on a specific actor will run as part of the GlobalActor. The previous days I remember reading that the GlobalActor is concurrent. That meant that the Tasks created here (outside of an Actor) will run as part of the GlobalActor. As they will run concurrently we may doing processing for more that one frame at the same time which can be consuming.

However, looking again now I couldn't find what I was reading and i ended up asking ChatGPT. The answer is that the GlobalActor is serial. So I will remove this one.

To control whenever TrackViews will be taking up rendering tasks

ipavlidakis · 2023-10-06T13:32:37Z

...ghtLayout_Tests/test_spotlight_participantsConnectionQuality_snapshot.default-light-good.png

This one is higher as now we are using the frame from global and in the empty space is where we have the controls.

github-actions · 2023-10-06T15:34:34Z

	1 Warning
⚠️	Big PR

Generated by 🚫 Danger

sonarqubecloud · 2023-10-06T16:28:07Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

71.9% Coverage
0.0% Duplication

martinmitrevski

lgtm! ✅

ipavlidakis requested a review from a team as a code owner October 4, 2023 09:45

ipavlidakis self-assigned this Oct 4, 2023

ipavlidakis added the enhancement New feature or request label Oct 4, 2023

ipavlidakis force-pushed the improvements/visibility-observation-for-video-track-views branch from cf92b10 to 4d8c789 Compare October 4, 2023 12:09

martinmitrevski reviewed Oct 4, 2023

View reviewed changes

ipavlidakis force-pushed the improvements/visibility-observation-for-video-track-views branch 2 times, most recently from b6c17b3 to 49dce2a Compare October 4, 2023 15:36

martinmitrevski reviewed Oct 5, 2023

View reviewed changes

ipavlidakis added 5 commits October 6, 2023 15:00

Implement VisibilityThresholdModifier

5f06d72

To control whenever TrackViews will be taking up rendering tasks

Implement SerialActor and enqueue video capturing tasks

2aeb12d

Add visibilityObservation on spotlightLayout

bb622d1

Update docs

3bd5ea8

Address feedback

f221811

ipavlidakis force-pushed the improvements/visibility-observation-for-video-track-views branch from 3e63270 to 68f4c85 Compare October 6, 2023 12:01

Fix snapshot tests

a423ee4

ipavlidakis force-pushed the improvements/visibility-observation-for-video-track-views branch from 04371c5 to a423ee4 Compare October 6, 2023 13:30

ipavlidakis commented Oct 6, 2023

View reviewed changes

[Improvements]Change available size to available frame (#176)

ab38e14

martinmitrevski approved these changes Oct 6, 2023

View reviewed changes

ipavlidakis merged commit 95a0a5b into main Oct 9, 2023
14 of 16 checks passed

ipavlidakis deleted the improvements/visibility-observation-for-video-track-views branch October 9, 2023 08:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement VisibilityThresholdModifier #173

Implement VisibilityThresholdModifier #173

ipavlidakis commented Oct 4, 2023 •

edited

Loading

martinmitrevski left a comment

martinmitrevski Oct 4, 2023

ipavlidakis Oct 4, 2023

ipavlidakis Oct 4, 2023

martinmitrevski Oct 4, 2023

ipavlidakis commented Oct 4, 2023

martinmitrevski Oct 5, 2023

ipavlidakis Oct 5, 2023

martinmitrevski Oct 5, 2023

ipavlidakis Oct 5, 2023

martinmitrevski Oct 5, 2023

ipavlidakis Oct 6, 2023

martinmitrevski Oct 5, 2023

ipavlidakis Oct 5, 2023

ipavlidakis Oct 6, 2023

github-actions bot commented Oct 6, 2023

sonarqubecloud bot commented Oct 6, 2023

martinmitrevski left a comment

Implement VisibilityThresholdModifier #173

Implement VisibilityThresholdModifier #173

Conversation

ipavlidakis commented Oct 4, 2023 • edited Loading

🎯 Goal

📝 Summary

🛠 Implementation

🎨 Showcase

Note

🧪 Manual Testing Notes

☑️ Contributor Checklist

🎁 Meme

martinmitrevski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ipavlidakis commented Oct 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 6, 2023

sonarqubecloud bot commented Oct 6, 2023

martinmitrevski left a comment

Choose a reason for hiding this comment

ipavlidakis commented Oct 4, 2023 •

edited

Loading