Skip to content

Fix Windows build under NOMINMAX: use std::max - #48

Open
PureChocolate wants to merge 1 commit into
rodrigomatta:mainfrom
PureChocolate:fix/windows-nominmax
Open

Fix Windows build under NOMINMAX: use std::max#48
PureChocolate wants to merge 1 commit into
rodrigomatta:mainfrom
PureChocolate:fix/windows-nominmax

Conversation

@PureChocolate

@PureChocolate PureChocolate commented Aug 13, 2026

Copy link
Copy Markdown

The WIN32 branch uses unqualified max, which only compiles when windows.h's max macro is defined. NOMINMAX builds fail on this line. The non-WIN32 branch already uses std::max, and <algorithm> is included, so this aligns both branches to work in all configurations.

Verified:

  • bare max reproduces the error; std::max compiles clean.
  • Full project builds with UCRT64 GCC + NOMINMAX + Vulkan SDK.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Windows compatibility when processing the codec context frame setting.

The WIN32 branch uses unqualified max, which only compiles when windows.h's max macro is defined. NOMINMAX builds fail on this line. The non-WIN32 branch already uses std::max, and <algorithm> is included, so this aligns both branches to work in all configurations.
@coderabbitai

coderabbitai Bot commented Aug 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1e9e0b69-cae4-4ae3-82fd-afba1d2d2b71

📥 Commits

Reviewing files that changed from the base of the PR and between 2c33261 and 44ecb56.

📒 Files selected for processing (1)
  • src/main.cpp

📝 Walkthrough

Walkthrough

The Windows-specific --codec-context-frames parsing branch now uses std::max. The existing parsing and clamping behavior remains unchanged.

Changes

Codec context frame parsing

Layer / File(s) Summary
Qualify the Windows frame clamp
src/main.cpp
The Windows branch now calls std::max, matching the non-Windows branch. Integer conversion, negative-value clamping, and conversion-failure handling remain unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Mergeability Score: ⚪ Minimal · up to 44ecb

This change fixes Windows builds using NOMINMAX without altering runtime behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the Windows build fix and the replacement of unqualified max with std::max.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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