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

Notifications should be prioritised #145

Open
zvirdaniel opened this issue Aug 14, 2022 · 3 comments
Open

Notifications should be prioritised #145

zvirdaniel opened this issue Aug 14, 2022 · 3 comments
Labels
enhancement New feature or request must-have

Comments

@zvirdaniel
Copy link

zvirdaniel commented Aug 14, 2022

Hi!

First of all, thanks for your work on this project, it's exactly what I was looking for and it's the reason I ended up with Synology NAS instead of dedicated NVR or some Raspberry setup. Config took some time, but it's working as expected.

However, I believe notificating the user should take priority before saving the image. Saving images takes time. In my case (slow drives and 4k camera) it takes up to 500ms. I suggest to send notification(s) before saving images.

obrazek

@djdd87
Copy link
Owner

djdd87 commented Aug 17, 2022

Maybe... I think I wanted to do this but due to the way some of the notifications work the image had to be saved to a file first. I'll look into it again at some point.

@djdd87 djdd87 added the enhancement New feature or request label Aug 17, 2022
@zvirdaniel
Copy link
Author

zvirdaniel commented Aug 17, 2022

I see. I suggest processing notificators which don't requires this first, then save the file, then process the rest.

Would you be open to doing it this way? I have zero experience with .NET but I am experienced with some java development, so if you'd agree with doing it this way, I'd try to take a look.

@djdd87
Copy link
Owner

djdd87 commented Aug 17, 2022

Yes, I'm definitely up for removing delays where possible. It looks like it's only Email that requires a FilePath, whereas everywhere else I return a FileStream from the saved image using File.OpenRead from GetReadOnlyStream on the ProcessedImage class.

It'd probably be doable by changing ProcessImage to take the SKBitmap object, then kicking off an async task to save the image so it's saved, e.g. Task<string> FilePath. Then Email can just use FilePath.Result.

The rest of the usages of GetReadOnlyStream could then return a new stream from SKBitmap.Encode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request must-have
Projects
None yet
Development

No branches or pull requests

2 participants