Skip to content

Fix busy-wait that stalled video writing under CI load - #11

Merged
acj merged 2 commits into
mainfrom
claude/failed-ci-diagnosis-jfvxzo
Jun 15, 2026
Merged

Fix busy-wait that stalled video writing under CI load#11
acj merged 2 commits into
mainfrom
claude/failed-ci-diagnosis-jfvxzo

Conversation

@acj

@acj acj commented Jun 15, 2026

Copy link
Copy Markdown
Owner

This fixes a race condition that stalled CI and could have caused on-device delays, too. When the writer input's buffer filled (isReadyForMoreMediaData returns), the block kept spinning on the media queue instead of returning, which then starved the encoder threads. The updated code better conforms to the pattern that Apple recommends.

@acj
acj force-pushed the claude/failed-ci-diagnosis-jfvxzo branch 2 times, most recently from 3fc6308 to 99ff8fc Compare June 15, 2026 00:45
@acj
acj marked this pull request as draft June 15, 2026 00:51
@acj
acj force-pushed the claude/failed-ci-diagnosis-jfvxzo branch from 99ff8fc to 77847fd Compare June 15, 2026 10:40
The requestMediaDataWhenReady block had two problems that could leave the
build hanging until the test timeout:

1. It wrapped the append loop in an outer while-loop over the remaining
   assets, so when the input's buffer filled (isReadyForMoreMediaData
   became false) it busy-spun on the media queue, starving the encoder.

2. Finalization (markAsFinished/finishWriting) happened only inside the
   'is ready' loop. If appending the final frame filled the input's
   buffer, the loop exited before finalizing and the writer waited on a
   readiness callback that never needed to come — an intermittent stall
   that timed out the multi-frame test.

Follow AVFoundation's documented pattern: append while the input is ready,
return when it is not so the block is re-invoked, and finalize once the
queue is empty regardless of readiness (markAsFinished does not require a
ready input). State that must survive re-invocation lives outside the block.
@acj
acj force-pushed the claude/failed-ci-diagnosis-jfvxzo branch from 77847fd to b38b176 Compare June 15, 2026 10:46
The first AVAssetWriter/H.264 use on the CI simulator pays a large,
variable cold-start cost (30s+ observed), which landed inside the first
video test's timed expectation and could trip its timeout. Pay that cost
once in setUp, outside any XCTest expectation, so each test only measures
steady-state latency.
@acj
acj force-pushed the claude/failed-ci-diagnosis-jfvxzo branch from b38b176 to 066c6e1 Compare June 15, 2026 10:50
@acj
acj marked this pull request as ready for review June 15, 2026 11:07
@acj
acj merged commit e4a1e5b into main Jun 15, 2026
2 checks passed
@acj
acj deleted the claude/failed-ci-diagnosis-jfvxzo branch June 15, 2026 11:07
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.

2 participants