Skip to content

Conversation

@SkalskiP
Copy link
Collaborator

Description

The updated process_video function implements a three-stage threaded pipeline consisting of a reader, a main-thread processor, and a writer, while using bounded queues to strictly control memory consumption during video processing.

When the user-supplied callback raises an exception, the function logs the error along with the affected frame index and, by default, re-raises it after performing a clean shutdown of all components.

With the optional skip_on_error=True parameter, problematic frames are silently discarded instead of interrupting the entire job, which is especially useful for noisy or partially corrupted videos.

The reader thread runs as non-daemon so that cv2.VideoCapture.release() is reliably called even during early termination, preventing file handle or resource leaks.

Shutdown is managed through a combination of a stop_event, non-blocking queue operations with small timeouts, and a light sleep-based backoff that avoids both deadlocks and excessive CPU usage.

The writer thread is kept as a daemon because disk writes can usually be safely interrupted, while both threads receive sensible join timeouts with logging warnings if they fail to complete in time.

Type of Change

🐛 Bug fix (non-breaking change which fixes an issue)

Motivation and Context

This change fixes a critical bug in the process_video function where an unhandled exception thrown inside the user-provided callback would cause the entire processing pipeline to deadlock indefinitely.

The main thread would exit its processing loop but then block forever on reader_worker.join() while the reader thread (a daemon) was stuck trying to put() into a full frame_read_queue.

Testing

  • I have tested this code locally
  • I have added unit tests that prove my fix is effective or that my feature works
  • All new and existing tests pass

@codecov
Copy link

codecov bot commented Jan 23, 2026

Codecov Report

❌ Patch coverage is 1.61290% with 61 lines in your changes missing coverage. Please review.
✅ Project coverage is 51%. Comparing base (b85b41d) to head (7c8ce48).
⚠️ Report is 14 commits behind head on develop.

❌ Your patch check has failed because the patch coverage (2%) is below the target coverage (95%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (51%) is below the target coverage (95%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@           Coverage Diff           @@
##           develop   #2102   +/-   ##
=======================================
- Coverage       52%     51%   -0%     
=======================================
  Files           61      61           
  Lines         7076    7117   +41     
=======================================
  Hits          3657    3657           
- Misses        3419    3460   +41     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@SkalskiP SkalskiP changed the title initial solution fix(process_video): deadlock on callback exception + improve shutdown robustness Jan 23, 2026
@Borda Borda added the bug Something isn't working label Jan 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working has conflicts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants