Repository navigation
Conversation
…rdinator
fcvm fleet aggregate --push-url-env VAR pushes the aggregator's output to the
RTMPS URL in VAR (src/fleet/aggregate/push.rs). It writes FLV itself:
- each frame's timestamp is its scene time after the first pushed frame, so a
skipped slot is a gap, not a drift;
- an onMetaData tag names the size, so the ffmpeg copy probes nothing (with
default probing a real-time copy held frames back 0.9 s median, 4.9 s max);
- a silent AAC-LC track rides beside the video (IVS served a video-only stream
5.2 s behind scene time against 1.8 s with the track);
- every access unit carries an SEI (user data unregistered, fixed UUID) with
{"t", "e"}, the frame's scene time and epoch, for the page's overlay sync.
The push starts at a keyframe, waits for the next one after lost frames, and
restarts its ffmpeg child with backoff.
fcvm fleet coord --broadcast-listen runs that aggregator as a child
(child::supervise_with, only while a layout exists) with the ingest URL from
--broadcast-secret or --broadcast-file. coord::broadcast::layout is the whole
image of every placed tile at its published IPv6 name; none while asleep.
Defaults: 720 lines, tiles at q=std, CRF 21 capped at 4.5 Mbit/s.
FleetCommand::Aggregate is boxed (clippy large_enum_variant with the new field).
Tests: push unit tests (SEI escaping and parsing, AVCC with the SEI before the
first slice, FLV timeline, sequence headers, metadata, audio cadence, lost-frame
resync, copy arguments), an ffmpeg-tests end-to-end push to a file checked with
ffprobe, the coordinator's broadcast layout, and the gated supervision starting
and stopping a real aggregator. make test-unit over the fleet suites: 184 passed.
|
@codex review |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 50 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 49641c9c97
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s ffmpeg - FleetAggregateArgs takes the last value of a repeated option (args_override_self). The coordinator passes the broadcast's defaults then --broadcast-arg, and a repeated --crf was an ArgumentConflict that stopped the child at start, again and again. - The push's write to its ffmpeg child is in the select with cancellation and the child's exit, under a 5 s progress deadline (WRITE_STALL). A stalled ingest stops ffmpeg reading; the unbounded write held up cancellation and kept the push from starting again. Tests, each red before the change: a_broadcast_arg_overrides_a_default (ArgumentConflict), a_stalled_ingest_does_not_hold_up_cancellation (did not stop within 2 s), a_stalled_ingest_is_started_again (one start in 10 s). Fleet suites with ffmpeg-tests: 196 passed; clippy clean.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4be6773c3f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The ingest child had kill_on_drop only, which cannot run when the aggregator is killed (the supervisor stops it with SIGKILL), and an ffmpeg blocked on a stalled ingest does not exit when its stdin closes. It now gets child::arm_pdeathsig (PR_SET_PDEATHSIG with the parent rechecked after the fork), the hop the repository's per-hop teardown rule asks for. the_push_child_dies_with_its_aggregator (test_fleet_aggregate) SIGKILLs a real aggregator whose push child is a stand-in that never exits: the child outlived it before the change and dies with it after.
ejc3
left a comment
ejc3
left a comment
There was a problem hiding this comment.
NOT-A-DEFECT: Codex's review bodies on this PR carry no finding of their own; each inline finding they announced is answered with a disposition in its thread.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b86e626d31
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
the_push_child_dies_with_its_aggregator spawned fcvm with kill_on_drop only, which nextest's SIGKILL skips. The spawn now arms PR_SET_PDEATHSIG(SIGKILL) with the parent rechecked after the fork, as tests/common/mod.rs::set_test_pdeathsig does. every_fcvm_this_file_spawns_dies_with_the_test pins it: every hand-written fcvm spawn in test_fleet_aggregate.rs must arm it; it failed on the old spawn.
ejc3
left a comment
ejc3
left a comment
There was a problem hiding this comment.
NOT-A-DEFECT: Codex's review bodies carry no finding of their own; each inline finding they announced is answered in its thread (all resolved).
|
@codex review |
ejc3
left a comment
ejc3
left a comment
There was a problem hiding this comment.
NOT-A-DEFECT: CodeRabbit's comment is a review-limit notice, not a finding; Codex reviews this PR.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 105cdc4969
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A tile lent to a visitor keeps its rectangle, and its GPU draws the visitor's private camera over the whole image; the broadcast layout read it like any tile, showing a private session to every broadcast viewer. coord::broadcast::tile now gives a claimed slot no URL, so the composite draws it black. the_broadcast_never_streams_a_borrowed_tile (test_fleet_coord) failed before the change: the layout carried the private VM's frame URL.
ejc3
left a comment
ejc3
left a comment
There was a problem hiding this comment.
NOT-A-DEFECT: the latest Codex review body carries no finding of its own; its inline finding is answered in its thread.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Stacked on: feat/broadcast-mode (PR #1384)
The broadcast's source:
fcvm fleet aggregate --push-url-env VARpushes the aggregator's output to a live ingest, andfcvm fleet coord --broadcast-listenruns that aggregator over the whole wall.Contract
src/fleet/aggregate/push.rs). The aggregator writes FLV to anffmpeg -c copychild that sends it to the RTMPS URL inVAR:onMetaDatatag names the size first, so the copy probes nothing (with ffmpeg's default probing a real-time copy held frames back 0.9 s median, 4.9 s max; with these options 0 ms);6a0e3c5d-9b41-4f2e-8d77-1c52f30ab619) with{"t", "e"}, which IVS keeps through its transcode in every rendition checked.The push starts at a keyframe, waits for the next one after lost frames, and restarts its child with backoff.
src/fleet/coord/broadcast.rs):--broadcast-listenwith--broadcast-secret(Secrets Manager) or--broadcast-fileruns the aggregator as a child only while a tile is placed (child::supervise_with,only_with_layout), its layout every placed tile at its published IPv6 name. Defaults: 720 lines, tiles atq=std, CRF 21 capped at 4.5 Mbit/s. A secret it cannot read stops the coordinator at start.FleetCommand::Aggregateis boxed (clippylarge_enum_variantafter the new field).Downstream impact
Nothing changes unless
--broadcast-listenor--push-url-envis given; staging gets them from the ejc3/aws PR at the end of the stack.Evidence
push.rs: RBSP escaping, SEI build and parse (incl. 255-byte size runs), the SEI placed before the first slice, the FLV timeline (scene-time stamps, gaps, drops), sequence headers on a changed SPS, the metadata tag, the audio cadence and ordering, lost-frame resync, and the copy's arguments.the_pushed_stream_decodes_with_scene_time_stamps_and_sei(--features ffmpeg-tests): libx264 frames through the real pusher to a file, ffprobe finds every frame at its scene time, the silent audio, and one SEI per frame. It was red with-fflags nobufferalone (the copy wrote nothing) and with no metadata.the_broadcast_layout_is_the_whole_image_of_every_placed_tile(test_fleet_coord) anda_supervised_aggregator_that_needs_a_layout_stops_without_one(test_fleet_aggregate: a realfcvm fleet aggregatechild started for a layout, stopped without one, started again).make test-unit FILTER="-E 'binary(test_fleet_coord) | binary(test_fleet_aggregate) | binary(test_fleet_agent) | test(/fleet::/)'": 192 passed.make clippyclean.