Fix Chromium performance hit in calls due to blur filter #4678

danxuliu · 2020-11-26T19:33:15Z

The CSS blur filter used in the VideoBackground component causes a severe performance hit in Chromium (the performance hit is higher the larger the image, and the larger the radius (which happens with larger images, the perfect storm :-P )).

To solve that, and given that Chromium has support for OffscreenCanvas, now the blurred background is generated instead using an OffscreenCanvas in a worker. Generating the blurred image in a normal canvas already improves the performance a lot, but it is still somewhat slow, so it needs to be done in a worker to not block the UI thread during the generation. Due to the slowness a cache for blurred backgrounds was added too.

Pending:

Do not fetch the source image again if the blurred image needs to be generated again
Probably cache blurred images so they are reused if previously calculated for the same source image and blur radius
Debounce and ensure that a new blurred image is not generated while a previous one is still being generated
Use an offscreen canvas when available
- Although blurring the canvas is way faster in Chromium it still lags a bit, so it might be better to offload the rendering to an offscreen canvas
Ensure that no memory leaks were added
Check that nothing broke in older browsers

Known issues:

The blurred background is cached locally in the component instance, and preventing the generation of other blurred backgrounds while a previous one is still being generated is also done per component instance. Due to this switching quickly and several times between the speaker mode and the grid view causes the generation of the last backgrounds to be delayed until the previous ones are ready. However that should not be something common to do, it is an annoyance rather than a big issue and the UI is still responsive while doing that, so I would not invest time on fixing that (or at least not at this moment).
Grid blur is not set for LocalVideo component, so it always uses its own self-calculated blur radius. It seems that there is something else needed besides propagating the value from LocalVideo to VideoBackground and setting :video-background-blur="videoBackgroundBlur" in the LocalVideo components of Grid.vue (probably just guarding against calculating the blur when there are no rows or columns, as videoWidth and videoHeight may be NaN or Infinity).

Future?:

Use the blurred canvas approach in other browsers besides Chromium?
- Offscreen canvas may not be availabe in Firefox (there is a setting to enable or disable it), so a normal canvas would be needed
- Nevertheless it would be good to do some benchmarks to verify if canvas or CSS filter is better in Firefox
- Safari does not support canvas filters, so the CSS filter has to be used in that case (or instead of a blurred image the average color of the image returned by https://www.npmjs.com/package/color.js could be used, but if there are no performance issues with the blur in Safari I would rather keep the CSS filter in that case)

How to test

Open Talk in Chromium
Start a call with another user (not a guest)
In a private window, join the call with the other user
Disable camera in both participants so the avatar is shown
Maximize the window (and the bigger the screen, the better)
Close and open the sidebar

Result with this pull request

The sidebar closes and opens immediately

Result without this pull request

The UI is unresponsive (even if audio and video work as expected) for several seconds while the sidebar is closing and opening

danxuliu · 2020-11-26T19:33:55Z

/backport to stable20

nickvergessen · 2020-11-26T19:46:10Z

src/components/CallView/shared/VideoBackground.vue

+			this.blur = this.$store.getters.getBlurRadius(width, height)
+		},
+
+		generateBlurredBackgroundImage() {


Or just a plain background?

I think that if we can use a blurred background we should use it, but of course if the performance hit is still bad even with an OffscreenCanvas then the plain background could be the only solution.

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

The CSS blur filter in the video background causes Chromium's UI to be very slow. The performance hit is larger when the element to blur is larger, and also when the blur radius is larger, so a promoted speaker with a blurred image seems to be the worst case (even worse than a grid with several rows and columns). However, even if the CSS blur filter is slow, applying the blur filter on a canvas is much more performant (maybe the blur filter itself has the same performance in both cases, but the UI uses it several times in a row when rendering the element, but that is just a guess). Due to this now a blurred background image is generated from the original background image whenever the blur size changes, and that blurred image is the one used in the image element without any CSS filter. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Until now a new source image was loaded every time that the blurred image had to be generated. Now the source image is loaded only when the URL of the background image changes and that "cached" image is reused to generate any blurred image. The source image is loaded using a watcher instead of a computed property because the "complete" read-only property of HTMLImageElement is not reactive. Due to this it is not possible to set "blurredBackgroundImageSource" as a computed property and then check its "complete" attribute in "generatedBackgroundBlur" to know if the image is loaded or not. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

The cache id needs to be generated from the background image URL rather than the image itself (as checksuming the image would require drawing it to a canvas, which although still faster than drawing a blurred image would defeat the use of a cache). For simplicity the images are cached locally in the VideoBackground element rather than in a central store. Due to this the images will be regenerated again if the component is created again (for example, when changing between speaker mode or grid view), but minimizes the posibility of using a wrong cached blurred image if the image returned by the URL changed. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

As the "blur" operation returns a promise it needs to be ensured that the elements will not be used after the component was destroyed. Similarly the background should be restored after getting the "raw" size of the image element to prevent an empty background to be shown while the new background is being generated (although in practice this will not happen due to "blur" still generating the image in the UI thread, but it will be needed once moved to a worker) Signed-off-by: Daniel Calviño Sánchez <[email protected]>

If several blurred backgrounds are requested while a previous one is being generated only the last requested background will be generated once the previous one finished, as the intermediate ones would be no longer needed. Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Until now the generation of the blurred backgrounds was done in a normal canvas, so it happened in the main thread and made the UI unresponsive while the image was being generated. Now, to solve that, the blurred background is generated in an OffscreenCanvas in a worker (if supported by the browser). Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu · 2020-12-06T01:29:03Z

I would still need to check that no memory leaks were introduced and that older browsers still work as expected (they should, but better ensure), but other than that I think that this is ready for review.

Also feel free to merge too if you do not see any potential memory leak and older browsers work :-)

PVince81 · 2020-12-07T08:48:29Z

src/components/CallView/shared/VideoBackground.vue

+				width = height * sourceAspectRatio
+			}
+
+			const cacheId = this.backgroundImageUrl + '-' + width + '-' + height + '-' + this.backgroundBlur


it is likely that someone can overflow the cache by resizing the window gradually

maybe need to use stepping for different width/height (like using divisions of 1024 or something)

in general we probably only ever need to catch 2 or 3 dimensions: the one in grid view, the one in stripe view and the one in speaker view

You would really need to resize the window very slowly and for a while to overflow the cache. But it would be indeed possible.

Therefore I will limit the cache of each VideoBackground to the last ten calculated dimensions or so (as whether the sidebars are open or not also affect the size of the background and, thus, the blur).

PVince81 · 2020-12-07T08:51:08Z

Feels like this brings us further away from "keep it simple" for "just the blurred background", not sure if it's a good idea to go with such complex solution, at least one that we need to maintain ourselves. Is there no library out there that we could use that already solved this ?

PVince81 · 2020-12-07T09:00:14Z

Should the cache be separate from the VideoBackground view ?
When switching speakers, grid or pages in the stripe view the VideoBackground component will likely get destroyed and so will the cache. This means that upon next appearance the blur will need to be re-created again.

This back then I chose to cache avatars outside the component, see #4284

nickvergessen · 2020-12-08T12:45:43Z

Feels like this brings us further away from "keep it simple" for "just the blurred background", not sure if it's a good idea to go with such complex solution, at least one that we need to maintain ourselves. Is there no library out there that we could use that already solved this ?

as much as I like the blur, I'm totally in on that. All the troubles and CPU problems sound like not worth it and as stated before, I tend to say let's just use a plain background then.

nickvergessen · 2020-12-09T10:23:24Z

/backport to stable20.1

danxuliu · 2020-12-16T20:48:29Z

Is there no library out there that we could use that already solved this ?

To be honest I did not check, I just implemented it. But if there is a suitable library let's use it, sure :-) Suggestions welcome :-)

Should the cache be separate from the VideoBackground view ?

I thought about it, but I kept it in the VideoBackground so it was reloaded whenever the avatar might be reloaded (for example, when changing between grid view and speaker mode). This was to prevent a mismatch between the avatar and its background. However... a user changing her avatar during a call should be quite a corner case, so it might be better to cache the backgrounds globally so they are reused when switching views.

as much as I like the blur, I'm totally in on that. All the troubles and CPU problems sound like not worth it and as stated before, I tend to say let's just use a plain background then.

I would keep the blur system. It is of course more complex than just using filter: blur, but I do not think that the code is too complex, and once finished I do not think that it will need to be touched very often (or at least I hope so :-) ). But even if it does at least for me having the blurred backgrounds is worth the "trouble".

marcoambrosini

Tested and works, though sometimes when switching to the grid view and trying to select videos, nothing happens. Not sure if this is related to this pr. No errors in the console, the selected video peer id is logged correctly

danxuliu added 2. developing bug regression feature: frontend 🖌️ "Web UI" client feature: call 📹 Voice and video calls labels Nov 26, 2020

danxuliu added this to the 💚 Next Major (21) milestone Nov 26, 2020

backportbot-nextcloud bot added the backport-request label Nov 26, 2020

nickvergessen reviewed Nov 26, 2020

View reviewed changes

danxuliu added 9 commits December 6, 2020 02:18

Return only blur radius instead of full filter string in store

3d4a79a

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Extract computed property for background blur

5aee9c2

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

Refactor canvas size calculation to do it before the canvas is created

0b6e40a

Signed-off-by: Daniel Calviño Sánchez <[email protected]>

danxuliu force-pushed the fix-chromium-performance-hit-in-calls-due-to-blur-filter branch from 0714337 to 9708a54 Compare December 6, 2020 01:27

danxuliu marked this pull request as ready for review December 6, 2020 01:28

danxuliu added 3. to review and removed 2. developing labels Dec 6, 2020

danxuliu requested review from PVince81, marcoambrosini and nickvergessen December 6, 2020 01:29

PVince81 reviewed Dec 7, 2020

View reviewed changes

marcoambrosini approved these changes Dec 17, 2020

View reviewed changes

nickvergessen merged commit b5877ba into master Dec 17, 2020

nickvergessen deleted the fix-chromium-performance-hit-in-calls-due-to-blur-filter branch December 17, 2020 13:23

This was referenced Dec 17, 2020

[stable20] Fix Chromium performance hit in calls due to blur filter #4780

Merged

[stable20.1] Fix Chromium performance hit in calls due to blur filter #4781

Merged

backportbot-nextcloud bot removed the backport-request label Dec 17, 2020

This was referenced Dec 17, 2020

Improve cache handling in blur filter #4792

Closed

Fix loading worker on browsers without CSP "worker-src" directive #4822

Merged

Tachi107 mentioned this pull request Dec 21, 2020

Blur lag introduced in Talk 10.0.4 on Chromium #4827

Closed

This was referenced Jan 4, 2021

Fix for Chromium performance hit in calls due to blur filter should be applied too to Chromium-based browsers #4887

Closed

Do not load the blurred background source if not needed #4888

Merged

danxuliu mentioned this pull request Jan 21, 2021

Replace blur with average color in video backgrounds #4985

Merged

6 tasks

danxuliu mentioned this pull request Sep 13, 2022

CSS blur filter seriously impacts performance in Chrome #7896

Closed

Fix Chromium performance hit in calls due to blur filter #4678

Fix Chromium performance hit in calls due to blur filter #4678

Uh oh!

Conversation

danxuliu commented Nov 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to test

Result with this pull request

Result without this pull request

Uh oh!

danxuliu commented Nov 26, 2020

Uh oh!

nickvergessen Nov 26, 2020

Choose a reason for hiding this comment

Uh oh!

danxuliu Nov 27, 2020

Choose a reason for hiding this comment

Uh oh!

danxuliu commented Dec 6, 2020

Uh oh!

PVince81 Dec 7, 2020

Choose a reason for hiding this comment

Uh oh!

danxuliu Dec 16, 2020

Choose a reason for hiding this comment

Uh oh!

PVince81 commented Dec 7, 2020

Uh oh!

PVince81 commented Dec 7, 2020

Uh oh!

nickvergessen commented Dec 8, 2020

Uh oh!

nickvergessen commented Dec 9, 2020

Uh oh!

danxuliu commented Dec 16, 2020

Uh oh!

marcoambrosini left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danxuliu commented Nov 26, 2020 •

edited

Loading