-
Notifications
You must be signed in to change notification settings - Fork 508
Improve cache handling in blur filter #4792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
Before the cache for blurred video backgrounds was local to each VideoBackground instance. During a call different VideoBackground instances can be mounted and destroyed for the same participant (for example, when switching between speaker mode and grid view, or when a promoted speaker changes), so the cache was lost and the blurred background had to be recalculated every time that happened. Now the cache is global and shared between VideoBackground instances, so previously cached backgrounds can be used by new VideoBackground instances. The global cache is lazily initialized when needed and cleared each time a call ends. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,8 @@ import { | |
| CONVERSATION, | ||
| } from '../constants' | ||
|
|
||
| const LRU = require('lru-cache') | ||
|
|
||
| const state = { | ||
| isGrid: false, | ||
| isStripeOpen: true, | ||
|
|
@@ -35,6 +37,7 @@ const state = { | |
| selectedVideoPeerId: null, | ||
| videoBackgroundBlur: 1, | ||
| participantRaisedHands: {}, | ||
| blurredBackgroundImageCache: {}, | ||
| } | ||
|
|
||
| const getters = { | ||
|
|
@@ -54,6 +57,7 @@ const getters = { | |
| return (width * height * state.videoBackgroundBlur) / 1000 | ||
| }, | ||
| isParticipantRaisedHand: (state) => (peerId) => !!state.participantRaisedHands[peerId], | ||
| getBlurredBackgroundImageCache: (state) => (videoBackgroundId) => state.blurredBackgroundImageCache[videoBackgroundId], | ||
| } | ||
|
|
||
| const mutations = { | ||
|
|
@@ -86,6 +90,16 @@ const mutations = { | |
| clearParticipantHandRaised(state) { | ||
| state.participantRaisedHands = {} | ||
| }, | ||
| initializeBlurredBackgroundImageCache(state, videoBackgroundId) { | ||
| if (state.blurredBackgroundImageCache[videoBackgroundId]) { | ||
| return | ||
| } | ||
|
|
||
| Vue.set(state.blurredBackgroundImageCache, videoBackgroundId, new LRU({ max: 10 })) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so what happens in a call with 12 users and they speak all one sentence after another, it will have to calculate again and again?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, each participant has (or should :-P ) its own cache; 10 is the number of backgrounds for each participant (as the blurred backgrounds are different depending on the size of the video background). |
||
| }, | ||
| clearBlurredBackgroundImageCache(state) { | ||
| state.blurredBackgroundImageCache = {} | ||
| }, | ||
| } | ||
|
|
||
| const actions = { | ||
|
|
@@ -116,6 +130,8 @@ const actions = { | |
| leaveCall(context) { | ||
| // clear raised hands as they were specific to the call | ||
| context.commit('clearParticipantHandRaised') | ||
|
|
||
| context.commit('clearBlurredBackgroundImageCache') | ||
| }, | ||
|
|
||
| /** | ||
|
|
@@ -197,6 +213,10 @@ const actions = { | |
| }) | ||
| context.commit('presentationStarted', false) | ||
| }, | ||
|
|
||
| initializeBlurredBackgroundImageCache(context, videoBackgroundId) { | ||
| context.commit('initializeBlurredBackgroundImageCache', videoBackgroundId) | ||
| }, | ||
| } | ||
|
|
||
| export default { state, mutations, getters, actions } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
normally we use import? not require?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The library does not support
import, onlyrequire🤷