feat(encoder): add preserveFramePictType to allow forced keyframes - #315
Merged
Merged
Conversation
The high-level Encoder clears `frame.pictType` on every frame in `prepareFrameForEncoding`, so a caller cannot force a keyframe on demand: setting `frame.pictType = AV_PICTURE_TYPE_I` (with `forced-idr`) — the standard FFmpeg way to request an IDR — is erased before `avcodec_send_frame`, and the encoder never emits the forced keyframe. For live streaming that is not cosmetic: a decoder produces nothing before its first keyframe, and on a lossy or late-joining connection the client needs to be able to request a fresh IDR (loss recovery, a new viewer, a resized encode). Through the high-level API there was no way to honour that request. Clearing pictType is still correct for transcoding, where decoded input frames carry frame-type hints that should not drive the output GOP. So this adds an opt-in `preserveFramePictType` option rather than changing the default: when set, the frame's pictType is passed through untouched. Existing callers are unaffected. Verified against libx264 with a long GOP: by default a forced keyframe request on a mid-stream frame produces no keyframe (only the initial IDR); with `preserveFramePictType: true` the forced IDR appears. Both cases are covered by tests. Closes seydx#314
Pradhumn115
added a commit
to Pradhumn115/beamdesk
that referenced
this pull request
Jul 28, 2026
Found a real bug in node-av while building the H.264 capture — the high-level Encoder clears frame.pictType, making forced keyframes impossible — and contributed the fix upstream: an opt-in preserveFramePictType option, verified against libx264 with tests. - issue: seydx/node-av#314 - PR: seydx/node-av#315 Replaces the pre-submission draft with a record of what was filed, and links it to the existing explanation in agent/src/capture/h264.ts of why we use the low-level sendFrame() path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011D5fBGzCVCbegUMeeS91pF
Owner
|
Thanks! |
Contributor
Author
|
Thanks a lot for the quick review and merge! 🙏 node-av has been great to build a live H.264 streamer on — the low-level |
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.
Fixes #314.
Problem
The high-level
Encoderclearsframe.pictTypeon every frame inprepareFrameForEncoding:That makes it impossible to force a keyframe on demand. Setting
frame.pictType = AV_PICTURE_TYPE_I(withforced-idron encoders that support it) is the standard FFmpeg way to request an IDR for a specific frame, but it is erased beforeavcodec_send_frame, so the encoder never emits the forced keyframe.For live streaming this matters: a decoder produces nothing before its first keyframe, and on a lossy or late-joining connection the client needs to request a fresh IDR (packet-loss recovery, a new viewer, a resized encode). Through the high-level API there was no way to honour that.
Change
Clearing
pictTypeis still correct for transcoding, where decoded input frames carry frame-type hints that should not drive the output GOP. So this is an opt-in option rather than a default change:EncoderOptions.preserveFramePictType?: boolean(defaultfalse).false(default), behaviour is unchanged.true, the frame'spictTypeis passed through, soAV_PICTURE_TYPE_Iforces an IDR.Verification
Tested against libx264 with a long GOP (so the only keyframes are the initial IDR and any forced one), encoding 10 frames and requesting a forced keyframe on frame 5:
preserveFramePictType: trueBoth cases are covered by new tests in
test/encoder.test.ts. Confirmed non-vacuous: reverting the guard makes the "forces a keyframe" test fail while the default-behaviour test still passes.Happy to adjust — e.g. a dedicated
encoder.requestKeyframe()-style API instead of a flag — if you'd prefer a different shape.