BufferedCapture: never let a bad PTS kill the pipeline at segment rollover - #973
Draft
Cybis320 wants to merge 2 commits into
Draft
BufferedCapture: never let a bad PTS kill the pipeline at segment rollover#973Cybis320 wants to merge 2 commits into
Cybis320 wants to merge 2 commits into
Conversation
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.
Symptom
Roughly 30s after a pipeline start that follows a camera-settings or protocol change, capture dies with what looks like a disconnect:
The camera is fine. The console (not the RMS log) shows what actually happened:
Cause
start_next_fragmentis only reachable at a segment rollover, and<sink_1>is the second fragment, so the earliest this can fire is oneraw_video_duration(30s) in. The 30s is the segment length, not a timeout.moveSegmentis connected toformat-location-fulland is splitmuxsink's only source of a filename — nolocationis set on the element. When the callback returns NULL, splitmuxsink hands its internal filesink an empty location, the filesink fails to open, and that error goes to the bus as a pipeline-level failure. The appsink then stops emitting and RMS reports it as a disconnect.The callback returned NULL because it raised. It derived the segment name straight from the first sample's PTS with no sanity check:
A camera restarting its RTP timebase (which is exactly what a settings or protocol change does) can emit a wild PTS for the first seconds, giving an out-of-range epoch that
utcfromtimestamprejects.getFramealready guards this at the same file, line 590 —moveSegmentdid not. It also had notry/except, so any exception was fatal to the pipeline.This is self-clearing: RMS reconnects, PTS state is reset, and the rest of the night is fine. It costs the first ~30s of video and one pipeline restart.
Fix
MAX_EXPECTED_PTS_NSpromoted to a module constant, shared with the existinggetFramechecksegmentTimestamp()validates the PTS against that bound, and additionally rejects a derived time more than a day from now (which also catchesstart_timestampnever having been established, since it initializes to0)moveSegmentcatches anything raised while deriving the time<STATIONID>_UNKNOWNTIME_<fragment>_video.mkv— a name that makes no claim rather than a wrong one — and logged atwarning, so it appears in the RMS log instead of only on stderrlast_segment_savetimeis removed: it existed only for the old wall-clock fallback and is now write-only.The callback deliberately still returns
Noneif the video directory itself cannot be created. That errors the pipeline, which is correct — nothing survives an unwritable data dir, and it is better than dropping clips somewhere that is never cleaned up.Cleanup
UNKNOWNTIMEclips age out exactly like normal ones.DeleteOldObservations.getRawItems(..., in_video_dir=True)collects day directories, not filenames, and deletes them wholesale. Verified against the real cleanup code: a directory holding one normal and oneUNKNOWNTIMEclip is removed completely, no leftovers.The only consumer that parses these names is
FrameInterfacewhen a clip is opened by hand in SkyFit2. It reports that the start time cannot be read from the file name, which is the honest outcome — and the name still has four_-separated fields, so the existingValueErrorhandler catches it rather than anIndexErrorescaping.Testing
moveSegmentexercised against stubs for every failure mode. All return a usable path except the unwritable-directory case:NZ005F_20260822_171411_752730_video.mkvPTS = CLOCK_TIME_NONEUNKNOWNTIMEUNKNOWNTIMEPTS = 0UNKNOWNTIMEUNKNOWNTIMEget_buffer()raisesUNKNOWNTIMEstart_timestampnever setUNKNOWNTIMElast_running_time_nsknownNone(pipeline errors, by design)🤖 Generated with Claude Code