Repository navigation
Skip a pending video handover once the egress is ending - #1465
Merged
pksgit merged 2 commits intoOct 21, 2026
Merged
Conversation
When a track is still waiting for its first frame as the egress ends, the
filler is the active selector pad, so its EOS goes straight through to
video_encoder_queue. A handover taken by the track's gate after that puts
the track behind the EOS: with the 1.24 input-selector the next sticky
event fails and the egress errors, and an ALLOCATION query from the
handover can be left in the queue after its sender frees it, which the
queue later dequeues ("refcounting problem?").
The video bin now sets an EOS function that marks it as ending before the
filler is sent EOS. Once ending, a track's gate takes no pending handover
and drops frames from tracks that are not selected.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🔍 Devin Review: 1 flag
Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
Contributor
|
AV-sync stats summary: view in run #39508689133 |
milos-lk
reviewed
Oct 20, 2026
Comment on lines
+1380
to
+1381
| drop := pts < b.lastPTS || (b.selectedPad != videoTestSrcName && b.selectedPad != name) || | ||
| (b.ending && b.selectedPad != name) |
Contributor
There was a problem hiding this comment.
nit: this condition becomes hard to read now - can we have a named function that would have some explanatory name?
milos-lk
approved these changes
Oct 20, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pksgit
deleted the
pradeepsharma/cs-2150-skip-pending-switch-after-eos
branch
October 21, 2026 00:38
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.
When a track is pending (subscribed, or back after going inactive, and still waiting for its first frame), #1418 keeps the filler as the selector's active pad until that frame reaches the track's gate. If the egress ends inside that window,
SendEOSpushes EOS into the filler, and because the filler is the active pad,input-selectorforwards it intovideo_encoder_queuestraight away. The track's first frame then takes the selector inside its gate, after that EOS. The handlers that switch pads stop once the pipeline is ending; the gate doesn't check.Two failures follow from a handover that lands behind that EOS:
h264parse, has to be forwarded through a selector whose output has already had EOS. With the 1.24input-selectorthat forward fails, the frame push returnsGST_FLOW_ERROR, and the egress fails withInternal data stream error. The 1.28 selector sends the new pad's stream-start first, which clears the EOS, so it doesn't hit this one.video_encoder_queue. The handover sends RECONFIGURE upstream and the track's converter answers with an ALLOCATION query. When the gate drops the frame it switched on (its PTS is behind the last one sent), that query can reach the queue before any STREAM_START. If it arrives while the EOS is still draining,gst_queuekeeps it after the sender gives up on EOS and frees it, and a later STREAM_START restarts the queue onto the freed pointer:Unexpected item ... dequeued from queue video_encoder_queue (refcounting problem?), followed by a stream error. This queue behaviour is the same in GStreamer 1.24 and 1.28.Change
VideoBinsets an EOS function,markEnding, on its bin.Bin.sendEOSruns it before pushing EOS into the filler, and it setsendingunderb.mu. Whileendingis set, a track's gate doesn't take a pending handover and drops frames from any track that isn't selected, so an unselected pad doesn't feed a selector that is finishing.The gate already takes
b.muon every frame, so this adds no lock. Because the EOS function and the gate both holdb.mu, a handover either completes before the filler is sent EOS, in which case the filler is no longer active and the selector holds its EOS as it did before #1418, or it is not taken.SendEOSEvidence
A deployment running a build with #1418 logged
Unexpected item ... dequeued from queue video_encoder_queue (refcounting problem?)five times, all after that build went out. Builds without #1418 running at the same time logged none. In every case:dropping old packet,correcting PTS, and in one casepacket PTS too far in the past compared to the pipeline3s before the warning), which is how a track goes inactive and comes back pending;video_encoder_queue; two completed.None of them logged a
not-negotiatederror, astreaming stoppedfrom a track's appsrc, or a push error, so the other way an EOS can reach the selector ahead of a switch (see Follow-ups) does not fit. Mute, unmute and subscription changes are logged at debug level, so the order of events is inferred from these lines rather than seen directly.Testing
TestSelectorHandover_EncoderQueueSurvivesEndingruns every step against the production gates, a realinput-selectorand the productionvideo_encoder_queue: the filler is on screen, a track becomes pending, the egress starts ending, the filler's EOS reaches the encoder queue and is held there by a probe (standing in for the encoder draining it), a late track frame arrives, the test sends the track's ALLOCATION query (in a pipeline it comes from the track's converter after the switch) and frees it once answered, then later track frames arrive. It checks that the track never takes the selector, that none of its frames leave the selector, that no query is queued behind the EOS, and that the encoder queue keeps accepting data. Without this change it fails every run on both 1.24 and 1.28 core elements, and most runs logUnexpected item ... dequeued from queue video_encoder_queue (refcounting problem?)andexit because we have no item in the queue, after which every push returns an error. It also fails if a handover after ending is taken only on a frame the gate passes. With this change it passes.go test ./pkg/...with thedeadlocktag and golangci-lint, as run bybuild/test/Dockerfile, pass, including the existingTestSelectorHandover_*.Internal data stream error, both on GStreamer 1.24.13 and on 1.28.7 with Ubuntu's 1.24.2 core elements. With it, 5 of 5 complete on each, and nothing reachesvideo_encoder_queueafter its EOS. The commit before fix: take the video selector handover inside the track's own gate #1418 also completes.file-media,file-trackandedgepass. The other legs weren't run locally. CI builds amd64.Follow-ups
not-negotiated,basesrcpushes EOS right after posting the error. The track is still the active pad, so the selector forwards that EOS intovideo_encoder_queue. The pipeline handles the error by removing the track, andOnTrackRemovedswitches the selector to the filler behind that EOS, where the filler's ALLOCATION query can hit the samegst_queuebehaviour.markEndingdoesn't run on this path because the egress isn't ending. Options: an event probe on each track's selector pad that switches to the filler when the selected track's EOS arrives while the pipeline is running, or a probe that drops that EOS. The logs above show no sign of this path, so it is left to a separate change.video_encoder_queuewhile it holds an EOS would cover any remaining path into thatgst_queuebehaviour.markEndingwould keep them.🤖 Generated with Claude Code