Skip to content
Merged
30 changes: 20 additions & 10 deletions .github/workflows/windows-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ permissions:
jobs:
build-vs2022:
name: Windows Build VS2022 (${{ matrix.config }}-${{ matrix.library_type }})
runs-on: windows-latest
# Pin to the windows-2022 image: windows-latest now resolves to an image that
# ships Visual Studio 2026 (v18) only, where the "Visual Studio 17 2022"
# generator cannot find a VS instance. VS 2026 is covered by the build-vs2026 job.
runs-on: windows-2022

strategy:
matrix:
Expand Down Expand Up @@ -311,16 +314,20 @@ jobs:

# Check CMake version
cmake --version

mkdir -p "build/${{ matrix.config }}-${{ matrix.library_type }}"

# Use New-Item -Force (idempotent): in PowerShell "mkdir -p" maps to New-Item,
# which errors when the directory already exists -- e.g. after a build-cache hit.
New-Item -ItemType Directory -Force -Path "build/${{ matrix.config }}-${{ matrix.library_type }}" | Out-Null
cd "build/${{ matrix.config }}-${{ matrix.library_type }}"

# Use Visual Studio 18 2026 generator with v144 toolset
# Use the Visual Studio 18 2026 generator with its default toolset.
# Forcing -T v144 makes MSBuild fail to resolve VCTargetsPath on the
# windows-2025-vs2026 image, so let CMake pick the toolset that ships with VS 2026.
Write-Host "Attempting to use Visual Studio 18 2026 generator..."
cmake ../.. -G "Visual Studio 18 2026" -A x64 -T v144 -DCCAP_BUILD_TESTS=ON $SHARED_FLAG
cmake ../.. -G "Visual Studio 18 2026" -A x64 -DCCAP_BUILD_TESTS=ON $SHARED_FLAG
if ($LASTEXITCODE -ne 0) {
Write-Host "Visual Studio 18 2026 generator not available, trying alternative with v144 toolset..."
cmake ../.. -A x64 -T v144 -DCCAP_BUILD_TESTS=ON $SHARED_FLAG
Write-Host "Visual Studio 18 2026 generator not available, trying the default generator..."
cmake ../.. -A x64 -DCCAP_BUILD_TESTS=ON $SHARED_FLAG
}

- name: Build
Expand Down Expand Up @@ -391,8 +398,10 @@ jobs:
}
EOF

# Find Visual Studio 2026 compiler
VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio/2026" -name "vcvars64.bat" 2>/dev/null | head -n1)
# Find the Visual Studio compiler. The VS 2026 install lives under a version
# folder ("...\Microsoft Visual Studio\18\..."), not a "2026" year folder, so
# search the whole VS directory rather than hard-coding the name.
VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio" -name "vcvars64.bat" 2>/dev/null | head -n1)
Comment on lines +401 to +404

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Pin vcvars64.bat discovery to VS 2026 to avoid false-positive link tests.

At Line 404, find ... | head -n1 may pick a non-2026 installation (for example VS2022) when multiple VS versions are present, so this “VS2026” link test can silently validate the wrong toolchain.

Suggested fix
-        VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio" -name "vcvars64.bat" 2>/dev/null | head -n1)
+        # Prefer VS 2026 (major version 18) explicitly.
+        VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio/18" -name "vcvars64.bat" 2>/dev/null | head -n1)
+        # Fallback only if VS 2026 path is unavailable.
+        if [ -z "$VCVARS_PATH" ]; then
+          VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio" -name "vcvars64.bat" 2>/dev/null | head -n1)
+        fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Find the Visual Studio compiler. The VS 2026 install lives under a version
# folder ("...\Microsoft Visual Studio\18\..."), not a "2026" year folder, so
# search the whole VS directory rather than hard-coding the name.
VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio" -name "vcvars64.bat" 2>/dev/null | head -n1)
# Find the Visual Studio compiler. The VS 2026 install lives under a version
# folder ("...\Microsoft Visual Studio\18\..."), not a "2026" year folder, so
# search the whole VS directory rather than hard-coding the name.
# Prefer VS 2026 (major version 18) explicitly.
VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio/18" -name "vcvars64.bat" 2>/dev/null | head -n1)
# Fallback only if VS 2026 path is unavailable.
if [ -z "$VCVARS_PATH" ]; then
VCVARS_PATH=$(find "/c/Program Files/Microsoft Visual Studio" -name "vcvars64.bat" 2>/dev/null | head -n1)
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/windows-build.yml around lines 401 - 404, The vcvars64.bat
lookup in the Windows build workflow is too broad and can resolve a non-2026
Visual Studio install, causing the VS2026 link test to use the wrong toolchain.
Update the discovery logic in the workflow step that sets VCVARS_PATH so it
explicitly targets the VS 2026 installation under the known Microsoft Visual
Studio 18 path instead of scanning all versions with find | head -n1, and keep
the rest of the link-test setup using that resolved path.


if [ -n "$VCVARS_PATH" ]; then
echo "Using Visual Studio 2026 environment from: $VCVARS_PATH"
Expand Down Expand Up @@ -658,7 +667,8 @@ jobs:
# Test building with file playback disabled (VS2022 Release only)
build-vs2022-no-file-playback:
name: Windows VS2022 Release - No File Playback
runs-on: windows-latest
# See build-vs2022: pin to windows-2022 for the Visual Studio 2022 toolchain.
runs-on: windows-2022

steps:
- name: Checkout repository
Expand Down
24 changes: 10 additions & 14 deletions src/ccap_imp_apple.mm
Original file line number Diff line number Diff line change
Expand Up @@ -256,21 +256,17 @@ - (BOOL)open {
AVAuthorizationStatus authStatus = [AVCaptureDevice authorizationStatusForMediaType:AVMediaTypeVideo];
if (authStatus == AVAuthorizationStatusNotDetermined) {
dispatch_semaphore_t sema = dispatch_semaphore_create(0);
void (^requestAccess)(void) = ^(void) {
[AVCaptureDevice requestAccessForMediaType:AVMediaTypeVideo completionHandler:^(BOOL granted) {
CCAP_NSLOG_I(@"ccap: Camera access %@", granted ? @"granted" : @"denied");
dispatch_semaphore_signal(sema);
}];
};

// Permission must be requested on the main thread
if (![NSThread isMainThread]) {
dispatch_async(dispatch_get_main_queue(), ^{ requestAccess(); });
} else {
requestAccess();
}

CCAP_NSLOG_I(@"ccap: Waiting for camera access permission...");
// Request authorization on the calling thread. requestAccessForMediaType: may be
// called from any thread and delivers its completion on an internal queue, so we
// do NOT bounce the request onto the main queue: that deadlocks whenever no run
// loop is servicing the main queue (e.g. a ccap::Provider opened from a worker
// thread in a process without a CFRunLoop, such as a Node.js/Electron addon).
[AVCaptureDevice requestAccessForMediaType:AVMediaTypeVideo
completionHandler:^(BOOL granted) {
CCAP_NSLOG_I(@"ccap: Camera access %@", granted ? @"granted" : @"denied");
dispatch_semaphore_signal(sema);
}];
dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
authStatus = [AVCaptureDevice authorizationStatusForMediaType:AVMediaTypeVideo];
}
Expand Down
12 changes: 9 additions & 3 deletions tests/test_file_playback.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -716,11 +716,17 @@ TEST_F(FilePlaybackTest, GetCurrentTimeProgression) {
EXPECT_GT(time2, time1) << "CurrentTime should increase as frames are grabbed";

double frameRate = provider.get(ccap::PropertyName::FrameRate);
ASSERT_GT(frameRate, 0.0) << "FrameRate must be positive; otherwise the bound below is vacuous (inf/NaN)";
double expectedTimeDelta = 5.0 / frameRate;

// Allow some tolerance for timing variations
EXPECT_NEAR(time2 - time1, expectedTimeDelta, expectedTimeDelta * 0.5)
<< "Time progression should roughly match frame rate";
// CurrentTime reports the wall-clock playback position, not a frame counter, so
// grabbing buffered frames faster or slower than real-time (as happens on shared CI
// runners) makes (time2 - time1) deviate from 5 / frameRate in both directions. The
// reliable invariant is forward progress, asserted above; keep only a generous upper
// bound here to catch gross regressions without flaking on timing. The deterministic
// frame-count progression is covered by GetCurrentFrameIndexProgression below.
EXPECT_LT(time2 - time1, expectedTimeDelta * 5.0)
<< "Time progression should stay within a sane multiple of the frame-rate span";

provider.stop();
provider.close();
Expand Down
Loading