fix(L100-003): remove frame_rate param from ByteTrack, simplify max_time_lost#2206
fix(L100-003): remove frame_rate param from ByteTrack, simplify max_time_lost#2206Zeesejo wants to merge 1 commit intoroboflow:developfrom
frame_rate param from ByteTrack, simplify max_time_lost#2206Conversation
…ime_lost
The frame_rate parameter was only used in a single expression:
self.max_time_lost = int(frame_rate / 30.0 * lost_track_buffer)
The hardcoded 30.0 divisor made frame_rate confusing: changing it scaled
max_time_lost in a non-obvious way, and the docstring for lost_track_buffer
("Number of frames to buffer when a track is lost") was inconsistent with
the actual scaled behaviour.
Fix:
- Remove frame_rate parameter entirely.
- Set max_time_lost = lost_track_buffer directly (mirrors how
minimum_consecutive_frames is already handled).
- Update docstring to reflect the correct semantics.
Backward-compatible: users who relied on the default frame_rate=30
get identical max_time_lost values (30/30*30 == 30). Users who passed a
custom frame_rate should migrate to setting lost_track_buffer directly.
Fixes roboflow#2202
There was a problem hiding this comment.
Pull request overview
Removes the misleading frame_rate argument from supervision.tracker.byte_tracker.core.ByteTrack.__init__ and makes max_time_lost directly equal to lost_track_buffer, aligning the implementation with the documented “frames to buffer” semantics.
Changes:
- Removed
frame_ratefromByteTrack.__init__and its docstring. - Simplified
max_time_lostcomputation tolost_track_buffer.
| def __init__( | ||
| self, | ||
| track_activation_threshold: float = 0.25, | ||
| lost_track_buffer: int = 30, | ||
| minimum_matching_threshold: float = 0.8, | ||
| frame_rate: int = 30, | ||
| minimum_consecutive_frames: int = 1, | ||
| ): |
There was a problem hiding this comment.
frame_rate is removed from __init__, but there are still in-repo call sites that pass frame_rate= (e.g. src/supervision/detection/tools/smoother.py docstring example and several scripts under examples/). This makes the codebase examples/docs inconsistent and also contradicts the PR description claim that all usages omit frame_rate or use the default. Either update those references in the same PR, or keep frame_rate as a deprecated/ignored parameter that emits SupervisionWarnings guidance to use lost_track_buffer directly.
| self.frame_id = 0 | ||
| self.det_thresh = self.track_activation_threshold + 0.1 | ||
| self.max_time_lost = int(frame_rate / 30.0 * lost_track_buffer) | ||
| self.max_time_lost = lost_track_buffer | ||
| self.minimum_consecutive_frames = minimum_consecutive_frames |
There was a problem hiding this comment.
self.max_time_lost used to be explicitly converted to int(...). With self.max_time_lost = lost_track_buffer, callers who pass a non-int (e.g. track_seconds * fps where track_seconds is a float) will now silently change the type/rounding behavior. Consider normalizing/validating lost_track_buffer (e.g. cast to int and/or reject negative values) so max_time_lost remains an integer frame count as documented.
| def __init__( | ||
| self, | ||
| track_activation_threshold: float = 0.25, | ||
| lost_track_buffer: int = 30, | ||
| minimum_matching_threshold: float = 0.8, | ||
| frame_rate: int = 30, | ||
| minimum_consecutive_frames: int = 1, | ||
| ): | ||
| self.track_activation_threshold = track_activation_threshold | ||
| self.minimum_matching_threshold = minimum_matching_threshold | ||
|
|
||
| self.frame_id = 0 | ||
| self.det_thresh = self.track_activation_threshold + 0.1 | ||
| self.max_time_lost = int(frame_rate / 30.0 * lost_track_buffer) | ||
| self.max_time_lost = lost_track_buffer | ||
| self.minimum_consecutive_frames = minimum_consecutive_frames |
There was a problem hiding this comment.
This PR changes a public initializer API and alters max_time_lost semantics, but there are no tests asserting the new behavior (e.g. that lost_track_buffer controls removal timing, and/or that passing frame_rate is rejected or produces a deprecation warning). Adding a focused unit test would prevent regressions and clarify the intended contract.
Description
Fixes #2202
The
frame_rateparameter inByteTrack.__init__was only ever used in one place:This was confusing for two reasons:
30.0divisor means the parameter doesn't behave as its name implies — changingframe_ratesilently scalesmax_time_lostin a non-obvious way.lost_track_buffersays "Number of frames to buffer when a track is lost", which is inconsistent with the scaled formula.With the default
frame_rate=30, the formula resolves toint(30 / 30.0 * 30) = 30, somax_time_lostwas always equal tolost_track_bufferfor the vast majority of users.Changes
frame_rateparameter fromByteTrack.__init__.self.max_time_lost = lost_track_buffer(same pattern asminimum_consecutive_frames).frame_rateargument entry.Backward Compatibility
frame_rate=30,lost_track_buffer=30): no behaviour change —max_time_loststays30.frame_rate: they should now pass the desired frame count directly vialost_track_buffer. This is a breaking change for that subset, but the old API was semantically misleading.Type of change
frame_rateparameter)How Has This Been Tested?
frame_rate=30(the default) or omit it entirely, meaningmax_time_lostis unchanged.frame_rateparameter.Checklist