Skip to content

Encode the B-frame end_time test clip with explicit B-frames - #21

Merged
RikHeijdens merged 1 commit into
mainfrom
test/explicit-b-frames
Oct 5, 2026
Merged

RikHeijdens merged 1 commit into
mainfrom
test/explicit-b-frames

Conversation

@EthanRosenthal

Copy link
Copy Markdown
Contributor

Addresses review feedback on #19: test_decode_end_time_with_b_frames relied on libx264's default GOP containing B-frames.

  • TestVideoParameters gets a b_frames: Option<u32> (default None, so other tests are unchanged). It passes -bf N, and for libx264 also b-adapt=0, since -bf only caps the B-frame run and adaptive placement can still pick fewer.
  • The B-frame end_time test requests 3 B-frames.

Checked with ffprobe on the test clip (FFmpeg 7.1.1): explicit flags give 111 B / 38 P / 1 I frames, the same as today's default; -bf 0 gives 0 B. cargo test: 95 passed, 3 ignored.

test_decode_end_time_with_b_frames relied on libx264's default GOP
having B-frames. Add a b_frames option to TestVideoParameters (-bf N, plus
b-adapt=0 for libx264 so the pattern is fixed rather than adaptive) and
request 3 B-frames in that test, so it keeps exercising out-of-order
packets if encoder defaults change.
@upwind-code-us

upwind-code-us Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Upwind Upwind Code Scan - ✅ Passed

0 newly introduced vulnerabilities · 0 resolved · 1 total in this PR vs main

Total breakdown: 🟡 1 Medium

View full analysis in Upwind Console

Scan completed in 17s

Scan history (1 scan)
Commit Scanned at New Resolved Net
3d7974d < 2026-10-04 22:22 UTC 0 0 0

Last scanned: 3d7974d · 2026-10-04 22:22 UTC

@upwind-code-us

upwind-code-us Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Upwind Upwind IaC Scan - ✅ Passed

0 newly introduced misconfigurations · 0 resolved · 0 total in this PR vs main

View full analysis in Upwind Console →

Scan completed in 3s

Scan history (1 scan)
Commit Scanned at New Resolved Net
3d7974d < 2026-10-04 22:22 UTC 0 0 0

Last scanned: 3d7974d · 2026-10-04 22:22 UTC

@RikHeijdens RikHeijdens left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Thanks!

@RikHeijdens
RikHeijdens merged commit 811d625 into main Oct 5, 2026
6 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.

3 participants