Skip to content
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

Add simple temporal smoothing #498

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Add simple temporal smoothing #498

merged 1 commit into from
Dec 19, 2023

Conversation

royshil
Copy link
Collaborator

@royshil royshil commented Dec 16, 2023

To handle some of the flickering that often happens with some models

@royshil royshil requested a review from umireon December 16, 2023 04:20
@royshil royshil added this to the 1.1.8 milestone Dec 16, 2023
@royshil royshil self-assigned this Dec 16, 2023
@royshil
Copy link
Collaborator Author

royshil commented Dec 18, 2023

@umireon What do you think?

@umireon
Copy link
Member

umireon commented Dec 18, 2023

I will try this change this week

@royshil
Copy link
Collaborator Author

royshil commented Dec 18, 2023

I will try this change this week

@umireon perfect!
i was hoping to release v1.1.8 very soon

@umireon
Copy link
Member

umireon commented Dec 18, 2023

I understand that this implements a low-pass filter for matt.

@royshil
Copy link
Collaborator Author

royshil commented Dec 18, 2023

I understand that this implements a low-pass filter for matt.

Sort of, yes, but temporal. Really all it does is linearly blend the last mask with the current one.
But since the last mask is also a blend it's an "infinite impulse response".

@umireon
Copy link
Member

umireon commented Dec 18, 2023

I'm doing z-transformation. Please wait.

@umireon
Copy link
Member

umireon commented Dec 18, 2023

I think the inverse of the current smooth factor value corresponds to the cut-off frequency and thus I suppose the value of the factor of addWeighted should be inverted on the configuration screen and it will work more straightforwardly.

@royshil
Copy link
Collaborator Author

royshil commented Dec 18, 2023

I think the inverse of the current smooth factor value corresponds to the cut-off frequency and thus I suppose the value of the factor of addWeighted should be inverted on the configuration screen and it will work more straightforwardly.

the way i have it is that 1.0 corresponds to "no temporal filter" everything is taken from the current mask
and 0.0 is the exact opposite.
does that make sense?

@umireon
Copy link
Member

umireon commented Dec 18, 2023

the way i have it is that 1.0 corresponds to "no temporal filter" everything is taken from the current mask
and 0.0 is the exact opposite.
does that make sense?

It makes sense. Thank you!
What I want to say in the previous comment is actually that the effect of the smooth factor on the cut-off frequency is exponential and users may want to configure the factor with the logarithmic scale because it would have an intuitive behavior.

In addition, the addWeighted operation can be done on GPU shader and it would be more efficient than OpenCV's function.

@royshil
Copy link
Collaborator Author

royshil commented Dec 18, 2023

i think this cv operation on the CPU is very quick
the problem with moving it to the GPU/shader is that i need to keep/save the blended image for the next frame in CPU memory... so it's not easily done (i could move the rendered blended mask... but that saves very little in time)

@royshil
Copy link
Collaborator Author

royshil commented Dec 19, 2023

@umireon can you approve so i can merge it in?

Copy link
Member

@umireon umireon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@royshil royshil merged commit f7cef6d into main Dec 19, 2023
12 checks passed
@royshil royshil deleted the roy.add_temporal_smoothing branch December 19, 2023 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants