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

Move inotify checks to separate thread #162

Merged
merged 2 commits into from
Sep 25, 2021

Conversation

ObsidianX
Copy link
Contributor

Querying the inotify watcher causes the camera to freeze momentarily every time it's queried since load_images() is called several times when consumers > 0. This moves it to its own thread so it doesn't interrupt the stream.

The primary fix is simply unindenting lines 299:306 pre-patch (101:108 post-patch) which will prevent the momentary freeze, but continuously querying inotify in the same loop as the camera stream noticeably reduces the framerate.

@fangfufu
Copy link
Owner

Okay I didn't quite get what you are saying, but here is my understanding:

  1. Inotify pauses the webcam the same way as sigint pauses the webcam. That causes the image to be reloaded when inotify causes pausing, which leads to the camera to freeze momentarily. So you put pausing by sigint in its own thread.
  2. You decided to put inotify stuff in a separate thread, because having it in the main loop reduces performance.

Multithreading implementation used to cause stability issue (#116), but I suppose you are not passing numpy array around. So I am okay with this.

@fangfufu fangfufu merged commit 42c7b8f into fangfufu:master Sep 25, 2021
fangfufu added a commit that referenced this pull request Sep 25, 2021
fangfufu added a commit that referenced this pull request Sep 25, 2021
@fangfufu
Copy link
Owner

Something went wrong with the merge (#163). I am investigating. If I clone your repository and run it directly, it seems to work fine.

@fangfufu
Copy link
Owner

Actually your patch is broken from ObsidianX@4619a74

I should have checked it properly. I cloned your repository - I couldn't be bothered to download the pull request and merge it locally. I didn't realise that you were contributing from a branch of your fork. I thought you were working on the master branch of your fork directly.

@ObsidianX
Copy link
Contributor Author

Actually your patch is broken from ObsidianX@4619a74

I should have checked it properly. I cloned your repository - I couldn't be bothered to download the pull request and merge it locally. I didn't realise that you were contributing from a branch of your fork. I thought you were working on the master branch of your fork directly.

Hmm, strange. I made my branch based on master, but might not have pulled recently enough. I'll rebase and retest. My apologies!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants