Skip to content

Unify lost track buffer semantics#420

Open
cdeil wants to merge 1 commit into
roboflow:developfrom
cdeil:unify-lost-track-buffers
Open

Unify lost track buffer semantics#420
cdeil wants to merge 1 commit into
roboflow:developfrom
cdeil:unify-lost-track-buffers

Conversation

@cdeil
Copy link
Copy Markdown
Contributor

@cdeil cdeil commented May 15, 2026

What does this PR do?

Unify lost_track_buffer semantics across SORT, ByteTrack, OC-SORT, and BoT-SORT, and prevent positive buffers from collapsing to zero frames at low frame rates.

Previously every tracker scaled the user-facing buffer with:

int(frame_rate / 30.0 * lost_track_buffer)

That floors fractional results. For example, frame_rate=10 and lost_track_buffer=1 produced maximum_frames_without_update == 0, so a confirmed track could be deleted on the first missed detection even though the caller requested a one-frame buffer.

There was also a tracker inconsistency at the exact expiry boundary. SORT, ByteTrack, and BoT-SORT removed tracks when time_since_update == maximum_frames_without_update, while OC-SORT kept them until the value was exceeded. This PR makes the public meaning consistent: lost_track_buffer=N keeps a confirmed track alive for exactly N scaled missed frames and removes it on the next miss.

This matters for low-FPS and mobile streams, where small but valid buffers are common and premature deletion can cause avoidable ID switches after a single detector miss.

Type of Change

This is a bug fix with a small breaking change for invalid configuration: all trackers now raise ValueError when lost_track_buffer < 1 or frame_rate <= 0 / non-finite. Those values previously initialized but produced nonsensical lifecycle behavior, including immediate pruning.

Not sure if this is the semantics you want. E.g. requiring lost_track_buffer > 0 could also be reasonable. And for maximum_frames_without_update consistently putting < instead of <= would also be reasonable. But overall I'm +1 to make the trackers fully consistent here since it's already difficult to understand precisely what the parameters mean for casual/new users. Maybe a small docs addition/illustration would help.

Testing

  • I have tested this change locally
  • I have added/updated tests for this change

Red/green TDD:

  • Added test_track_survives_exact_lost_buffer_boundary, which failed for SORT, ByteTrack, and BoT-SORT because they pruned at the exact buffer boundary instead of after it.
  • Added test_low_frame_rate_lost_buffer_rounds_up_to_one_frame, which failed for all four trackers because frame_rate=10, lost_track_buffer=1 scaled to 0.
  • Added test_lost_buffer_configuration_rejects_non_positive_values, which failed for all four trackers because invalid buffer settings were accepted.
  • Applied the shared scaling/validation fix and reran the new regression tests, full pytest suite, and Ruff checks successfully.
  • Updated the affected integration metric baselines where the corrected boundary semantics keep tracks alive for one additional missed frame.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Unifies the lost_track_buffer semantics across SORT, ByteTrack, OC-SORT, and BoT-SORT. Previously, int(frame_rate / 30.0 * lost_track_buffer) floored positive low-FPS buffers to zero, and OC-SORT used a different boundary check than the other trackers. A shared helper now validates inputs, rounds up positive buffers to at least one frame, and all get_alive_tracklets helpers use <= so lost_track_buffer=N survives exactly N missed frames.

Changes:

  • Add BaseTracker._compute_maximum_frames_without_update that validates lost_track_buffer >= 0 and finite positive frame_rate, and rounds positive buffers up via ceil (with explicit zero preserved).
  • Change boundary check in SORT/ByteTrack/BoT-SORT _get_alive_tracklets from < to <= to match OC-SORT's inclusive semantics; update docstrings, CHANGELOG, and existing botsort boundary test.
  • Add parametrized regression tests for exact-boundary survival, low-FPS rounding, explicit zero buffer, and invalid configurations; refresh affected SportsMOT/DanceTrack baselines.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/trackers/core/base.py New _compute_maximum_frames_without_update helper with validation and ceil-based scaling
src/trackers/core/sort/tracker.py Use shared helper; docstring updates for buffer/frame_rate semantics
src/trackers/core/sort/utils.py Boundary check changed to <=; docstring tweak
src/trackers/core/bytetrack/tracker.py Use shared helper; docstring updates
src/trackers/core/bytetrack/utils.py Boundary check changed to <=; docstring tweak
src/trackers/core/ocsort/tracker.py Use shared helper; docstring updates
src/trackers/core/botsort/tracker.py Use shared helper; docstring/Notes updates
src/trackers/core/botsort/utils.py Boundary check changed to <=; docstring rewrite
tests/core/test_trackers.py New boundary, low-FPS, zero-buffer, and invalid-config parametrized tests
tests/core/test_botsort_utils.py Update existing boundary test to reflect inclusive semantics
tests/data/tracker_expected_sportsmot.json Refreshed BoT-SORT MOTA/IDSW baselines
tests/data/tracker_expected_dancetrack.json Refreshed ByteTrack and BoT-SORT metric baselines
CHANGELOG.md Added breaking-changes and fixed entries describing the new semantics

@AlexBodner
Copy link
Copy Markdown
Collaborator

@cdeil thank you for your PR!
This is something we should consider for future version, as consistency along trackers is one of the most important things, but each tracker also does its own things and handles this details differently in the original implementations, which leads to differences in metrics. That's why we opted for having the < and <= as of now. Though if we do not degrade performance and with other hyperparameters we can achieve the same results, while unifying interface interpretation this is a change that we will apply

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.

3 participants