Skip to content

Let generate_binary_search take frame numbers in any order - #947

Merged
caglarpir merged 1 commit into
mapillary:mainfrom
caglarpir:select-frames-in-any-order
Oct 30, 2026
Merged

caglarpir merged 1 commit into
mapillary:mainfrom
caglarpir:select-frames-in-any-order

Conversation

@caglarpir

Copy link
Copy Markdown
Contributor

FFMPEG.generate_binary_search assumed its frame numbers were sorted, and silently missed frames when they were not: [3, 1] selected only frame 1 (if(lt(n\,1)\,eq(n\,3)\,eq(n\,1))), and [10, 2, 7] missed frame 10.

This makes it take any iterable of frame numbers, in any order and with repeats:

  • generate_binary_search sorts and de-duplicates them once (sorted(set(...))).
  • The recursion moves unchanged into a private _binary_search, which assumes sorted, distinct frame numbers.
  • extract_specified_frames, the only caller, already sorted its set, and now passes the set directly.

The expression for sorted, distinct frame numbers is unchanged byte for byte, so the same frames are extracted as before (checked on 240 random sets of 0 to 28,147 frames, given as a sorted list and as a set).

Tests:

  • A doctest with unsorted, repeated input.
  • A unit test that evaluates the expression as the select filter does, for unsorted and repeated input (including a range and a 22-frame list), and checks that it selects exactly the right frames and equals the expression for the sorted frames. All 6 cases fail on main.

generate_binary_search assumed its frame numbers were sorted, and
silently missed frames when they were not: [3, 1] selected only frame
1, and [10, 2, 7] missed frame 10. It now takes any iterable of frame
numbers, in any order and with repeats, and sorts and de-duplicates
them once. The recursion, which still assumes sorted, distinct frame
numbers, moves into _binary_search.

extract_specified_frames, the only caller, already sorted its set, and
now passes the set directly. The expression for sorted, distinct frame
numbers is unchanged byte for byte, so the same frames are extracted.
@meta-cla meta-cla Bot added the cla signed label Oct 30, 2026
@caglarpir
caglarpir merged commit 6b0b273 into mapillary:main Oct 30, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant