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

let ByteTrack to maintain track ID per instance #1528

Merged
merged 9 commits into from
Oct 17, 2024

Conversation

grzegorz-roboflow
Copy link
Contributor

Description

BaseTrack implementation includes _count class var, similarly STrack implementation includes _external_count class var. This results in undesirable behavior when running more than one tracker in the same process. In inference we added a block wrapping ByteTrack, if more than 1 video is processed, objects across videos will receive non-deterministic track IDs.

In this change class vars are removed and track ID assignment is handled by upper layer (ByteTrack)

Type of change

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

How has this change been tested, please provide a testcase or example of how you tested the change?

Added unit test to cover this scenario.

Any specific deployment considerations

N/A

Docs

N/A

@grzegorz-roboflow grzegorz-roboflow changed the title Make BaseTrack._count instance variable let ByteTrack to maintain track ID per instance Sep 19, 2024
@LinasKo
Copy link
Collaborator

LinasKo commented Oct 15, 2024

I finally found time to look at this. There's still a few things to think through - I'll take this one over.


When two BaseTrack object share ID assignment logic, it avoids ID clashes when more than one tracker is applied. You can have ByteTrack and a separate ConfTrack (PR outstanding), and the assigned IDs will be different.

The question is - do we need that? All use cases that I'm aware of have used one tracker only.

The requirements for untangling the IDs come from inference where N videos are processed simultaneously and N ByteTrack objects are used at the same time. In that case, their state should not be shared. Explicitly requesting a unique ID provider seems like a possible solution.

Also, this PR does exactly what it claims, but it only solves half of the greater problem we have - dependence between multiple ByteTrack instances. Kalman Filter data (error, weights - not xyah+Δxyah) is currently shared between all STrack objects. It should be shared at ByteTrack level instead*


* I'm a bit skeptical whether shared_kalman makes sense, even when the original authors have it in their codebase. Yes, every object in a single scene may have a similar motion and observation error. But does it make more sense than giving each one a Kalman, or having a Kalman per-class?

@grzegorz-roboflow
Copy link
Contributor Author

grzegorz-roboflow commented Oct 17, 2024

In inference we have batch of images - more than 1 video can be processed simultaneously, I'd imagine each stream requires separate tracker and this is how we handle it in inference, by creating separate instance of ByteTrack per stream. We need tracker_id to be deterministic (i.e. we don't want one stream to interfere with tracker_id assigned in another stream). Also there can be another situation - when processing batch of videos, each pass of inference will create new instance of ByteTrack - this instance should produce tracker_id consistently, that is it does not matter what other instances of ByteTrack we had in the past within scope of the process, assigned tracker_id values should be always the same (i.e. same as if we were only processing 1 video in isolation).

* External IDs were counted as the numbers grow very large very fast
* Reworked ID counting as a class, as state is necessary, and only STracks know if an external ID is required after the update (see conditions everywhere before self.external_id_counter.new_id() is called).
@LinasKo
Copy link
Collaborator

LinasKo commented Oct 17, 2024

I slept on it, and was slightly wrong: there's no ambiguity - we'll never support multiple trackers at once applied to the same Detections object. Your proposal, save for the issue described, was correct.


Testing unfortunately revealed a regression in the external ID system. It's used to trim down the visible number of track IDs - see video below.

Since only individual STracks know if an external ID is needed, they need to control when the external ID is set - ByteTrack does not know that. Yet, there needs to be state above to hold this info. Long story short - we now have an IdCounter class.

Video before my change:

out_massive_IDs_optim.mp4

Video after my change:

out_optim.mp4

Thanks @grzegorz-roboflow! I can finally merge it. It was also my first time engaging with the tracker code. Looking at it, let's say a tiny bit of remodeling coming. 🔥🪓

@LinasKo LinasKo merged commit 251b952 into develop Oct 17, 2024
10 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