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

Fix thread synchronization bug + many more #8562

Merged
merged 12 commits into from
Aug 25, 2023

Conversation

CristiFati
Copy link

@CristiFati CristiFati commented Jun 17, 2022

The most important thing is in the 4th commit. Maybe on "normal" hardware it's not visible, but I have an older laptop (tried this from Win) and I can barely reach ~3 FPS. The problem is that the 3 threads are not synchronized correctly, and when user presses Esc on drawing window, the drawing thread is terminating and also stop reading from queues, leaving (at least one of) the other 2 threads hanging in put. Only way to stop is killing the process (Ctrl+ Break).

Now the program gracefully stops in both scenarios (tested with a movie file).

The patch also contains what I consider to be a (lame) workaround (gainarie) - which is opening the capture twice.

To be frank, I saw this as a preamble for a bigger thing: switching to multiprocessing. But besides another (worse) workaround (loading the network twice), the result was totally unexpected: ~2 FPS, so I dropped it. I created a separate PR (with no intention of merging it), just to be available to whomever would be interested: #8567.

I noticed that many files are duplicated under build/darknet/x64. Shouldn't that be cleaned up?

@CristiFati
Copy link
Author

CristiFati commented Jul 6, 2022

The test env is quite poor. Many tests fail because of setup issues totally unrelated to the changes.

@CristiFati
Copy link
Author

When (if ever) this is going to be merged, please let me know to rebase first.

@cenit cenit merged commit c87e33e into AlexeyAB:master Aug 25, 2023
20 checks passed
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