Suggestion: Decouple Threading Logic for a More Flexible and Robust Design

[mpryahin]

First off, thanks for creating this library! It's a great abstraction for a very common problem.

I've been looking through the implementation and had some thoughts on the threading model that I wanted to share for discussion.

Current Design and Potential Issues

The library currently manages its own threading via the start_async method. While this is convenient, I've noticed a couple of potential issues with this approach:

1. Race condition. In threaded mode, the _background_capture method writes to self._latest_frame at the same time the main application thread might be reading it via get_latest_frame. (example) Since this access isn't synchronized with a lock, it could lead to race conditions like torn frames, especially under heavy load.

2. Slightly confusing base class logic: In VideoCaptureBase, the _read_frame_for_thread method contains a potentially confusing call to self.read(), which in turn calls other methods that check the thread's status. It seems to create a tangled execution path that could be hard to debug. The subclasses appear to handle this correctly by using a _read_direct method, but the base implementation itself is a bit confusing.

Proposal: A Synchronous Core with External Concurrency

The real strength of this library is how it handles each individual source type's settings, abstracts away their configurations using the factory method pattern, and implements the reading logic from each of the supported sources.

What if the library's core classes were simplified to be purely synchronous, leaving the concurrency management to the application developer?

My suggestion is to remove the internal threading logic (start_async, _background_capture, etc.) and provide a simple, blocking read() method as the primary way to get a frame.

Benefits of this Approach:

• The developer gains full control over how to run the reader: in a simple loop, wrap it in a threading.Thread for a producer-consumer pattern, or use multiprocessing.Process to get around the GIL. The library no longer imposes a specific concurrency model.
• This design eliminates the race condition by its very nature and simplifies the library's internal state, making it easier to maintain and reason about.
• The public API becomes more focused and predictable. A simple connect(), disconnect(), and read() is very intuitive.

Suggested API and Usage:

The core VideoCapture classes would just have a clean, synchronous API:

class VideoCapture:
    def connect(self):        # Establish a connection
    def disconnect(self):     # Release the resource
    def read(self):           # Synchronously reads a single frame
    def config(self):         # Returns the source configuration 

The application developer would then be responsible for building their own producer logic, giving them full control over the architecture:

def frame_producer(source, frame_queue, stop_event):
    try:
        source.connect()
        while not stop_event.is_set():
            success, frame = source.read()
            if not success:
                break
            frame_queue.put(frame)
    finally:
        source.disconnect()

This frame_producer can then be run directly in the main application thread, or given to a threading.Thread or multiprocessing.Process depending on the application's specific needs.

This change would turn the FrameSource classes into predictable building blocks that can be used in a much wider variety of application architectures.

Would love to hear your thoughts on this idea!

[olkham]

Thanks for the feedback and suggestion, I agree it would be more simple in every way.
The main motivation was when reading from some of my IP cameras the connection needs to be kept alive by requesting frames. I had issues in the past where slow processing later in the pipeline e.g. during inference, would cause the stream connection to die. So I had to thread it to keep alive.
I think frame_producer would neatly solve that.

One of the features I was thinking about adding is a callback to the FrameSource, so that when a frame was acquired it would fire off say an inference request on it. But again this would probably be better off living in the frame_producer

[brmarkus]

"It depends" - there might always be use-cases or special conditions requiring different strategies.

[mpryahin]

That's great to hear, and thanks for the context about the IP cameras. A slow processing pipeline causing the stream to die makes total sense why you built the threading in.

I think the frame_producer approach, implemented by the library's consumers, would solve that exact issue perfectly. The producer thread's only job would be to call read() continuously to keep the connection alive, while the main thread can pull from the queue and process frames at its own pace without ever blocking the source. It neatly decouples the keep-alive from the processing.

We can add recipes/examples for that.

This will make the framework much simpler, as it won't need to decide for the consumer how they should implement their processing flow.

I'm happy to move this forward. My idea is to start by refactoring the VideoCaptureBase interface, and once we agree on that, I can create new implementations for the existing sources.

After that, I can add a few examples/recipes showing how to use the new synchronous design in different scenarios: single-threaded, with a producer thread, multiprocessing.

How does that sound as a plan?

[olkham]

@mpryahin sounds good - I had a play with the frame_producer approach using the existing synchronous method before refactoring out all the internal threading stuff. I created a branch called external_thread with an example in tests/test_simple_external_threading.py - there's no appreciable performance difference between approaches which is good :)