Closed javiber closed 2 years ago
@facundo-lezama I ran this for the yolov5 demo and it worked (with a minor quirk) I was drawing the detections of each tracker in different colors so the result was pretty clear, however, by default the user might get 2 objects with the same id and same color. This makes me wonder if we should draw the global_id instead or even replace the id with the global one. We discussed a lot about having the ids be track-dependent but what is really adding? especially considering that the counts no longer need to be inferred from the ids
The quirk is that there is a small bug that should be solved in #198 (there is some overlap between that PR and this on the validate_points change that I'm going to clean up this tomorrow)
Nice one, @javiber. That was a smart move!
It's interesting what you say about the ids. I think this change should give the users the flexibility they need to spawn multiple trackers. Let's see how it goes from here, and we may later look back and see if there's something that appears unnecessary.
The name of the PR should be changed before being merged.
Some options:
Should we mention the addition of global_id
in the PR title? I think we shouldn't because it's what we had before. The new part is being able to differentiate IDs of different Trackers.
This PR includes the new
TrackedObjectFactory
which is initialized by theTracker
and is in charge of creating newTrackedObjects
. This change makesTrackedObject.id
unique for each tracker instead of globally unique.This would allow users to infer the count of objects from the ids (since they are sequential), however, if the count is needed we should directly provide it in the Tracker. For this reason
total_object_count
andcurrent_object_count
are added to theTracker
Added a new
global_id
toTrackedObject
in case users need something to identify objects accross trackers.Also included some small changes:
self.period
from Tracker. because it's not needed, it wasn't defined on the init and it is information that the user already has since it must be passed to theTracker
in eachupdate
self.abst_to_rel
from Tracker for the same reasons as previous pointTrackedObject.is_initializing
: inside this calculated property was the logic to request a new id if the conditions were met but this depended on the Tracker accessing the property. With the change, the flag and the id are handled insideTrackedObject.hit
initialization_delay
is set to 0 in the init of theTrackedObject
(problem introduced by the previous point)Detection.age
to the init, used by theTrackedObject
for buildingpast_detections
update_coordinate_transformation
toDetection
andTrackedObject
so that the Tracker can pass the coordinate transformation without messing with the inner workings of each class.Detection
instead ofTrackedObject
TrackedObject.hit
Open questions for reviewers to chime in:
TrackedObjectFactory.create
receives a lot of parameters simply to be forwarded toTrackedObject
this brings the question if it's not better for the Tracker to directly instantiate the TrackedObject and pass the factory. I kept it this way to better implement the factory patternTrackedObject.global_id
be constructed differently? uuid? combination of a newTracker.id
and theTrackedObject.id
TrackedObject
? we only use it on the merge method to adjust thehit_counter
but I feel we can remove itage
just to be used while building the past_detections of the tracked object?Tracker
?