Skip to content

Use the correct stream bitrate for strict min/max rates - #223

Merged
ekisu merged 3 commits into
masterfrom
fix/strict-filesize-bitrate
Sep 12, 2026
Merged

Use the correct stream bitrate for strict min/max rates#223
ekisu merged 3 commits into
masterfrom
fix/strict-filesize-bitrate

Conversation

@ekisu

@ekisu ekisu commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Summary

Use video_bitrate for video min/max constraints and audio_bitrate for audio-only constraints. A bitrate refactor left this block referencing an undefined bitrate, emitting minrate=nilk / maxrate=nilk.

Related to the strict-filesize reports in #48; this does not close the issue's separate speed/synchronization report.
Tracked in #217.

Verification

  • Baseline reproduces malformed nilk flags for both AVC and MP3. Depending on encoder/version, these produce parse errors or leave the requested bounds unapplied.
  • Two real-encode regression tests fail their expected-rate assertions on baseline and pass with the fix.
  • Fixed AVC output uses 800k for b, minrate, and maxrate; a generated two-second PCM fixture encoded to MP3 uses 128k for all three.
  • Both encodes complete successfully and produce nonempty output; temporary files/processes are cleaned up.
  • Parent independently ran python -m unittest -v tests.testcases.test_strict_filesize: 2 passed (~3s).
  • Build and CRLF-aware whitespace checks passed.

This corrects the flags; it is not a guarantee that every codec meets an exact output-byte limit.

Current harness-based verification (supersedes historical test notes)

Rebased onto master 72d9d11, which includes #226 and #227. Tests load the normal built script and drive real mpv.

Real AVC and audio-only MP3 encodes now use shared process/log/range helpers. Tests inspect output media and numeric b/minrate/maxrate arguments. Pinned Nix: 16 tests, 15 passed, 1 existing audio skip.

Latest rebase verification

Rebased onto master 72d9d11 after #227 merged. Normal build and the complete pinned-Nix suite pass, with two expected skips (audio fixture and unavailable optional libaom-av1 encoder).

@ekisu
ekisu force-pushed the fix/strict-filesize-bitrate branch from 3d408cd to 77b4d16 Compare September 12, 2026 00:53
@ekisu
ekisu changed the base branch from master to test/mpv-integration-harness September 12, 2026 00:54
@ekisu
ekisu force-pushed the fix/strict-filesize-bitrate branch from 77b4d16 to 127dfba Compare September 12, 2026 00:56
Base automatically changed from test/mpv-integration-harness to master September 12, 2026 01:03
calculate_bitrate returns video_bitrate/audio_bitrate, but the strict_filesize_constraint block still interpolated an undefined bitrate variable, producing minrate=nilk/maxrate=nilk which ffmpeg cannot parse. Select the bitrate of the constrained stream (video for video formats, audio for audio-only).
@ekisu
ekisu force-pushed the fix/strict-filesize-bitrate branch from 127dfba to c9a7386 Compare September 12, 2026 01:16
@ekisu

ekisu commented Sep 12, 2026

Copy link
Copy Markdown
Owner Author

Approved.

@ekisu
ekisu marked this pull request as ready for review September 12, 2026 19:37
@ekisu
ekisu merged commit e7a8b9c into master Sep 12, 2026
2 checks passed
@ekisu
ekisu deleted the fix/strict-filesize-bitrate branch September 12, 2026 19:37
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.

1 participant