feat(publish): enforce EncoderConfig.frameRate for video#1656
Merged
Conversation
frameRate was accepted but ignored: the encoder always used the captured track's rate for the bitrate calc and VideoEncoderConfig, and the encode loop had no frame dropping. Now when config.frameRate is set: - #runConfig prefers it over the captured rate, feeding both the bitrate calculation and the encoder/catalog framerate. - The encode loop paces to the target by dropping frames whose timestamp falls within the minimum interval since the last encoded frame. A half interval of slack keeps the cadence stable against capture jitter. Dropped frames are not closed here: the frame is a shared Signal consumed by both the hd and sd encoders, and the owner closes each frame when the next arrives. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EncoderConfig.frameRatewas accepted but never enforced for video. The encoder always used the captured track's frame rate for the bitrate calculation and theVideoEncoderConfig, and the encode loop had no frame dropping. This wires the config value through and actually paces the output to hit the target rate.Changes
#runConfignow prefersconfig.frameRateover the captured track's rate (user.frameRate ?? settings.frameRate ?? 30). That single value already feeds the bitrate calculation (framerateFactor), theVideoEncoderConfig.framerate, and the catalogframerate/jitter.1 / frameRate) since the last encoded frame. A half-interval of slack keeps the cadence stable against capture-timestamp jitter (e.g. 60→30fps cleanly keeps every other frame).frameRatereplaces the old// TODO actually enforce this.Reviewer notes
close()d here. The frame is a sharedSignalconsumed by both thehdandsdencoders, and the owner injs/publish/src/video/index.tscloses each frame when the next one arrives. Closing inside one encoder would break the other and double-free.frameRateis read non-reactively viathis.config.peek()per frame, matching the existingkeyframeIntervalhandling.Test plan
config.frameRateset below the capture rate; confirm output framerate matches the target and catalog reports it.frameRate; confirm behavior is unchanged (captured rate used, no dropping).(Written by Claude)