-
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 all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,8 @@ import { | |
| CONVERSATION, | ||
| } from '../constants' | ||
|
|
||
| const LRU = require('lru-cache') | ||
|
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. normally we use import? not require?
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. The library does not support |
||
|
|
||
| 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.
This is reaching a level where i think it stops being useful/worth it
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.
It is only ~16 KiB unpacked including its single dependency, so I do not think it is so bad :-) If I had written my own with just the needed features it would have been (I hope...) smaller, but this was faster and easier 🤷
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.
well its not about the bytes or whatever, its about the complexity off all this. 3rdparty lib, caching, loading images and blurring them with JS, …