Skip to content

Preserve buffered recording data during finalization - #58

Draft
steveseguin wants to merge 2 commits into
mainfrom
fix/recording-drain-queued-media
Draft

steveseguin wants to merge 2 commits into
mainfrom
fix/recording-drain-queued-media

Conversation

@steveseguin

@steveseguin steveseguin commented Oct 4, 2026 •

Copy link
Copy Markdown
Owner

Problem

GLibWebRTCHandler.finalize_recordings pushes EOS from each recording queue's source pad. This bypasses media still buffered inside the queue. A sink can finish successfully, and the handler can log "Recording container finalization complete", before that buffered tail reaches the file. The subsequent NULL transition discards it.

Change

Send EOS into each queue's sink pad instead, so the queue processes previously buffered media before forwarding EOS. Preserve the existing branch selection, duplicate-queue protection, failure handling, and bus-wait timeout.

Add six focused regressions in the existing recording test module for sink-pad dispatch on both tracks, duplicate queues, rejection, exceptions, missing pads, and recording that has not started.

Verification

  • Reproduced locally with native GStreamer 1.26.2 using only synthetic bytes in an appsrc → queue → filesink pipeline. A queue threshold holds five 3-byte buffers to make the backlog deterministic: the old path accepts EOS but writes 0/15 bytes; the fixed path writes 15/15 bytes.
  • Repeated that native test by executing the actual baseline and modified finalize_recordings methods (AST-extracted only to avoid unavailable module-startup dependencies). One track writes [0] before / [15] after; two tracks write [0, 0] before / [15, 15] after. Both versions report EOS completion. The thin native bus adapter exercises EOS success, not ERROR parsing.
  • Six added focused tests pass on Python 3.12 and 3.13 using the actual method with GStreamer stand-ins. Five fail against the baseline method.
  • Six existing recording-pair discovery tests pass. Repository Python compilation, Python 3.9 grammar for changed files, and git diff --check pass.
  • Attempted the complete unittest suite locally: 74 reported tests, with 14 import errors due to unavailable runtime dependencies including websockets; native Gst Python typelibs are also unavailable. This is not a complete-suite pass.
  • Independent review reproduced the native before/after result and reviewed the fix.
  • Initial compatibility CI passed all 156 tests on Bullseye/Python 3.9/GStreamer 1.18, Bookworm/3.11/1.22, and Trixie/3.13/1.26. A test-class relocation avoids an unnecessary merge conflict with Fix VP9 and AV1 file recording startup #57; the production fix is unchanged. The final-head rerun also passes all 156 tests in each of those three environments at head 2198f18285196c43d8a5af6826c0db896b3d2d91.
  • Both Fix VP9 and AV1 file recording startup #57 and this patch merge cleanly in a local three-way merge; all seven combined focused regression tests pass using AST extraction.

The reproduction establishes buffered-data retention, not RTP depayloading, real mux/container integrity, physical-device behavior, or end-to-end interoperability. No devices, real media, external signaling/services, merge, or deployment were used. No strict wall-clock shutdown bound is claimed: the existing timeout bounds bus waiting.

Reviewed against main 4f94ca2dcfced9f67eb72acf4102518cd3ef6674. This is independent of the VP9/AV1 startup fix in #57.

Reference: GStreamer queue EOS behavior and GstPad.send_event.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants