Conversation
|
Thanks for the PR Rebuilt the addon against your branch on macOS arm64, muxer-memory 12/12 and the muxer/format-context/fmp4/rtp/demuxer/pipeline suites all green. The budget logic looks right to me. maxInterleaveBytes is the part I want. Timestamp independent, bounds the thing that actually ate the container, gives the owner an error it can act on. maxDtsCorrection I'd rather take separately, mainly because of the 1s default on FMP4Stream. For continuous formats, so RTSP without AVFMT_TS_DISCONT, dtsErrorThreshold only kicks in at 3600s, so a 2s or 30s backward jump from a reconnect or an NTP step walks straight into muxFixupTs. Today that gets clamped and the stream survives with a glitch, with this PR it tears the session down. On my side that's HKSV restarting on every camera clock wobble. Can you split it? maxInterleaveBytes in one PR, maxDtsCorrection and the muxFixupTs block in a second. The FMP4 cleanup fix in doStop has to ride along with the memory one though. Budget rejects, Muxer.close() rethrows the worker error, and the old bare await bails out of doStop before the decoders are closed, so they leak and onClose never fires. That's the path I rely on to recreate a session. Only drop One gap while you're at it: there's no end to end test for the byte path through FMP4Stream. The budget tests all go against the raw Muxer, the real pipeline test goes through the DTS path. Budget rejection to onClose to a clean restart is the route I depend on, would be good to have it covered. |
|
Thanks for rebuilding and checking the broader suites. I agree that the memory bound and the decision to reject a camera clock change should be separate policies. Updated this PR in
All 321 tests in muxer-memory/muxer/format-context/fmp4-stream/rtp-stream/demuxer/pipeline passed against the locally built addon; TypeScript and focused lint/format checks also passed. The native patch is unchanged from the revision you rebuilt. The separate timestamp proposal is #354. I made it opt-in, default 0 in both Muxer and FMP4Stream, so even that proposal preserves reconnect/NTP tolerance unless the owner explicitly selects a threshold. Its 91 muxer-timestamps/muxer/fmp4-stream tests passed. The description calls out that the cleanup in this memory PR should land first. |
Fixes #351.
FMP4 output can retain gigabytes of video packets when media timestamps advance too slowly for FFmpeg's time-based interleaving limit. The affected production queue held 298,704 packets (7.84 GiB) spanning only 3.32 seconds of media time.
maxInterleaveBytesbounds that native queue independently of timestamps.FMP4Streamenables a 64 MiB budget by default; genericMuxerand low-levelFormatContextretain their existing defaults. Zero disables the budget. Packet backing buffers, side data and packet metadata are counted under the existing context lock. A conservative upper-bound counter is reconciled against the actual queue when needed, avoiding a full traversal on every write. Both synchronous and asynchronous writes consume a rejected packet and return ENOMEM; the high-level muxer propagates this even withexitOnError: false.The limit covers the interleaving queue, not codec/container-private memory or total process RSS. Timestamp correction is unchanged in this PR.
FMP4 teardown keeps releasing input/codecs if
Muxer.close()repeats an earlier worker error.onClosereceives the original pipeline error, while an externalstop()completes normally rather than rethrowing it.Verification
bee585a).onCloseexactly once, the input closes, externalstop()resolves, and a fresh session emits video successfully.Native verification uses the previously rebuilt macOS-arm64 addon against Homebrew FFmpeg 63.1.101 and the repository's matching internal headers. The native patch has not changed in this revision. Linux/static release builds and a production long-run test remain outstanding; this has not been deployed to the affected container.