feat(ffmpeg): RTMP egress can carry a second audio track, measured arriving as two distinct tracks (#141) - #331
Merged
Merged
Conversation
…riving as two distinct tracks (#141) DestSpec.SecondAudioOutLabel names a second finished mix from the destination's filter graph and maps it as a second encoded audio track. Empty for every destination that exists, so every existing command is byte-for-byte what it was. The cap it lifts was never a stated refusal in this package -- it was one `-map [aout]` and a doc comment describing "the single stereo track the platform will accept" as if the muxer and the wire were the constraint. Nobody had measured whether they are. They are not. The #141 refusal that IS stated -- `-c:a copy` on an RTMP destination, in db.AudioEncoding.copyProblems -- is untouched. Its reason is about platforms accepting multitrack audio, and nothing here measured a platform. TestTwoDistinctMixesReachAnRTMPFarEnd publishes the exact argv DestinationArgs builds, through a real FFmpeg, into internal/rtmpserver -- this product's own RTMP server, not a permissive listener -- records what arrives and reads the tones off each received track: 300 Hz on one, 5000 Hz on the other, 81 dB of band-balance apart. Tones rather than a track count, because two tracks carrying the same audio is a failure a count cannot see; a second subtest publishes one mix twice and asserts the spread is absent, so the distinctness check has been watched to fail. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…lled by The doc comment claimed the copy arm was structural and unmutatable. It is not: appending secondAudioMap to copyAudioArgs -- the obvious way somebody would extend the feature to the copy path -- puts a filter label on a command that has no filter graph, and the arm fails naming it. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
…an use Fifteen seconds of source for a three-second recording. The publisher is paced with -re and killed as soon as the recording is in hand, so the extra length costs nothing and removes the one shape this test could fail in on a slow machine: the source running out while the subscriber is still starting, which fails as a 30s timeout that reads like a defect. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
There was a problem hiding this comment.
Pull request overview
Adds opt-in support for mapping a second finished audio mix into RTMP (and other video-capable) egress commands, and introduces an end-to-end test that verifies two distinct audio tracks survive polyemesis’s RTMP server path.
Changes:
- Extend
ffmpeg.DestSpecwithSecondAudioOutLabeland map it as a second-map [...]audio stream when applicable. - Add a full wire test that publishes two distinct mixes via FFmpeg into
internal/rtmpserver, records the far end, and validates distinctness via bandpass +astats. - Update documentation/changelog and acceptance-suite commentary to clarify what is measured (wire capability) vs. what remains unproven (platform ingest acceptance).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/acceptance-multistream.sh | Clarifies that “can polyemesis send two tracks” is now measured separately from platform acceptance. |
| internal/ffmpeg/second_audio_track_test.go | New argv + wire-level test suite proving two distinct audio tracks reach an RTMP far end. |
| internal/ffmpeg/build.go | Adds SecondAudioOutLabel and conditionally appends a second audio -map to destination args. |
| docs/TESTING.md | Documents the new in-process wire measurement test and its scope/limitations. |
| CHANGELOG.md | Records the new capability and the fact it’s measured end-to-end (mechanically, not platform acceptance). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+340
to
+345
| res := testenv.ReserveTCP(t) | ||
| addr := "127.0.0.1:" + strconv.Itoa(res.Port()) | ||
| res.Release() | ||
| srv := rtmpserver.New(slog.New(slog.NewTextHandler(io.Discard, nil)), addr, | ||
| rtmpserver.ConstantTimeLookup(map[string]rtmpserver.Target{key: tg})) | ||
| if err := srv.Start(); err != nil { |
|
rainmanjam
added a commit
that referenced
this pull request
Aug 14, 2026
…egotiation that falls back quietly (#326) (#333) * feat(routing): a profile can describe two mixes, measured as two distinct tracks (#326) routing.Compile emitted one mix whose internal labels were fixed constants -- a_t0, a_mix, aout -- so concatenating two compiled graphs collided on every one of them. That is why nothing could set ffmpeg.DestSpec.SecondAudioOutLabel, which has been able to map and encode a second audio track since #331 and has had no way to be asked for one. Every label site now goes through a namespace. The empty namespace is byte-for-byte what the package emitted before, so a destination that gains a VOD track does not have its live mix rewritten, and a destination that never asks for one produces the argv it always did. CompilePair returns both mixes in one filter_complex plus the label to map for the second. A secondary that will not compile is a WARNING, not an error: an optional VOD track must never veto a working broadcast. A primary that will not compile is still an error -- there is no stream without it. TWO TAPS OF ONE INGEST TRACK NEED NO asplit. Both halves emit [0:a:N] for a track they share, and the obvious reading is that a filter pad feeds one input so a shared tap needs an explicit split. An input STREAM is not a filter pad and FFmpeg inserts the split itself -- measured on 6.0.1 (Alpine 3.18, the floor internal/ffmpeg/detect.go enforces) and 8.1.2. An asplit here would have been dead weight carried on a guess. TestAPairedGraphReachesFFmpegAsTwoDistinctMixes hands the real graph to the real binary and reads the tones back off both tracks, because the failure this ships with otherwise is TWO TRACKS THAT ARE THE SAME MIX -- which a track count sees as success. Mutating the VOD mix to carry the live content fails it at -18.1 dB on a tone that must be absent. It needs no skip: PCM in NUT with no video uses only codecs built into every FFmpeg, and the binary lookup goes through testenv.FFmpegBinary, whose skip lives inside internal/testenv and which fails rather than skips under POLYEMESIS_REQUIRE_FFMPEG. The skip census is unchanged at 94. All six tests mutation-verified; each mutation recorded in its test's doc comment with the observed failure. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX * feat(destinations): a destination can carry a second (VOD) audio mix (#326) routing.CompilePair landed with nothing able to ask for it. This is the path from a stored setting to the wire: two columns, the engine's compile step, and the one place a DestSpec is built. db.Destination gains Multitrack -- opt in to Twitch Enhanced Broadcasting -- and VODProfile, the second mix. Both default to off, and off produces byte for byte the filter graph and the argv the destination produced before the columns existed. VODProfile IS A POINTER because "no second mix" and "a second mix that happens to be the zero profile" are different things, and the zero profile fails Validate. The column stores '' rather than '{}' for the same reason and marshalVODProfile enforces the one spelling of absence at the write end: json.Marshal of a nil pointer produces `null`, which is not empty, takes the decode branch, and arrives as a nil profile by a route the reader cannot tell from a corrupt value. Had either end disagreed, every row written before this column existed would have come back carrying a second audio track that cannot compile. NOT ON THE PROVISIONAL PATH. A provisional compile already runs on a guessed layout and says so; a second guessed mix on top doubles what is approximate while the operator is being told the first one is unreliable. The VOD track returns on the first reconcile after a probe succeeds. The second label rides on routing.Result rather than only on Pair, so the engine's one description of an output carries a VOD track without a single signature changing type -- and the backup feed picks it up through the same struct, rather than silently publishing one track where the primary publishes two. THREE DRIFT GUARDS, all satisfied rather than worked around: - TestUITypesCanNameEveryDestinationField wanted a types.ts entry. - TestEveryStoredLeafIsClassified wanted all 22 new stored JSON leaves classified. Every one is sPublic: a routing profile is mix settings, not a credential. This is the #310/#324 guard and it fired correctly. - TestReadSafeViewsScrubEverySecretLeaf follows from that classification. NO CAPABILITY-MATRIX COLUMN, deliberately. Support is yes/manual/no/ unknown and Enhanced Broadcasting's honest answer is "depends whether your server has a supported GPU" -- `yes` would be false for the majority GPU-less VPS install, which is the normal case here, and the matrix answers a per-ACCOUNT question while this is a per-destination toggle. The UI toggle is gated to Twitch, worded so that not getting Enhanced Broadcasting reads as ordinary rather than as a fault. Both new db tests mutation-verified, each mutation and its observed failure recorded in the test's doc comment. A third mutation was discarded rather than counted: it failed to BUILD, which is not the named test failing. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX * feat(multitrack): negotiate at go-live, or fall back quietly (#326) multitrack could describe a negotiation and could not make one. Negotiate is the decision: ask Twitch, read the verdict, resolve the endpoint, and answer where to publish. IT RETURNS NO ERROR, deliberately. Twitch refuses any client without a supported GPU, and polyemesis is built to be installed on the operator's own server -- a rented VPS has none. On most installs the fallback IS the path, every time, for ever. An error return would make the ordinary case look broken: logged at error level, counted as a fault, retried by somebody who assumed non-nil meant something had gone wrong. The caller publishes to Target or to the destination's own URL, and both are correct. A NO-GPU ASK DOES NOT MAKE THE CALL. Twitch was measured refusing, by name, a request with no GPU information, an Intel iGPU, an unrecognised vendor and an out-of-date driver -- so the answer is known, and spending a round trip at go-live to be told it would delay every broadcast on every GPU-less install to learn nothing. The test asserts the call is not made, not merely that the outcome is a fallback: the same answer arrived at slowly is the failure a Use==false assertion cannot see. A LEAK FOUND AND CLOSED. The package doc says status.html_en_us "is scrubbed before it is shown anywhere". It was not. Config.explain returns the field verbatim, and that field is Twitch QUOTING THE REQUEST BACK -- the request that carries the stream key. Only the errors Client.Fetch builds were ever scrubbed, so the transport and decode paths were safe and the verdict path was not. Every note now leaves Negotiate through one scrubbing closure. TestNoOutcomeEverCarriesTheStreamKeyInItsNote sweeps all four paths, including a server that echoes the key back the way the live endpoint does; it FAILED before this change, which is how the gap was found. Same shape as #310 and #324. The minted-key test is the important one and it is about a failure that WORKS: publishing with the operator's own key does not fail loudly, it CONNECTS and sends a ladder the ingest never agreed to. So the assertion is that the signed v1_ prefix specifically survives -- the minted key ENDS with the operator's own, so "contains the operator key" would pass on the wrong value. Its fixture is SYNTHETIC and says so. The captured fixture ships with `authentication` emptied because a real minted key is a live credential; committing one would put a working stream key in the repository, which is the thing the leak above is about. The shape is reproduced from the measurement in IngestEndpoint.Authentication. All five tests mutation-verified against the real code paths they name -- Resolve's minted-key branch, the short circuit, Verdict's StatusError case, the scrubber, and the VOD preference -- each recorded with its observed failure. VERIFIED AGAINST FIXTURES, NOT THE LIVE API. Both fixtures were captured from the live endpoint by earlier work; nothing here re-derives them and I hold no Twitch token. The success path has never been exercised against Twitch by this change. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX * fix(multitrack): correct why html_en_us is scrubbed, and cover the minted key (#326) Two corrections to 007e915, one to a comment and one to a real gap the comment's wrong reasoning was hiding. THE COMMENT WAS WRONG ABOUT WHICH FIELD CARRIES THE KEY. 007e915 said status.html_en_us is "Twitch quoting the request back -- the request carrying the stream key", and called the scrub a fix for an observed leak. Measured against the live endpoint with a distinctive canary sent as `authentication`: html_en_us echoes client.name, not the key. A refusal for missing canvases named the broadcast software and did not contain the canary, and no refusal that could be produced quoted the key. The key that does come back is in ingest_endpoints[].authentication -- the 312-character minted key on the success path. The scrub stays, rejustified: html_en_us is attacker-influenced text from a third party that polyemesis renders to an operator, and it is built by quoting request fields back. Scrubbing text we do not control is cheap. That is defence, and it is now described as defence. A comment naming the wrong field is how the next person scrubs the wrong field. THE GAP THE WRONG REASONING HID. Chasing the right field surfaced one that matters more. destSecrets registered row.StreamKey -- the ORIGINAL -- and alerts.SecretSet.Scrub is a substring replace. The minted key ENDS WITH the original, so registering only the original masks its last segment and leaves v1_<64 hex signature>_<8 hex>_<hex manifest>_<MASK> standing in process.log, on the monitoring page's argv, and in every error the supervisor renders. A partially redacted live credential, which reads as protection to anyone glancing at the file. Same class as #310 and #324. destSecrets now takes the credentials that did not exist until go-live. Variadic rather than a row field: a minted key is a fact about one run of one process, and storing it would store a credential stale by the next broadcast. The test asserts the SIGNATURE PREFIX is gone, not that the original key is absent -- the latter passes on the broken version, which is exactly why the gap was easy to miss. A negative control pins that the gap is real, so the first test cannot pass because alerts.Redact's residual pass happened to catch it; if that control ever fails, the protection has moved and the comment needs rewriting rather than the test deleting. ALSO RECORDED: the live endpoint returned a SUCCESSFUL negotiation, with a full ladder and a minted key, for a plainly invalid stream key. Validation happens at publish, not at negotiation, so Outcome.Use is not evidence the credential works. Written on the field a caller reads. Three mutations, each with the named test confirmed running. The third asserts a PASS: dropping wireSpellings leaves the test green, confirming it does not lean on truncation expansion, which is a separate concern. Claude-Session: https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX
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.
RTMP egress could carry exactly one audio track. This lifts that to two, opt-in
per destination, and measures two distinct tracks arriving at a real far end
before claiming anything.
The restriction, and which half of it I did not lift
The
#141comment the work started from is not ininternal/ffmpeg— it is ininternal/db/destinations.go, onAudioEncoding.copyProblems, and it refuses-c:a copyon an RTMP destination because "platform ingests expect one encodedstereo track, so a copied multitrack stream would upload cleanly and be
rejected". That refusal is untouched. Its reason is a fact about platforms —
no mainstream ingest documents accepting multitrack audio, which
scripts/acceptance-multistream.shstates at length — and nothing here measureda platform.
What was in
internal/ffmpegwas the other half, and it was never a statedrefusal at all:
DestinationArgsemitted exactly one-map [aout], and the doccomment described "the single stereo track the platform will accept" as though
the muxer and the wire were the constraint. Nobody had measured whether they
are. They are not.
The change
DestSpec.SecondAudioOutLabelnames a second finished mix from the same filtergraph. When set, it is mapped as a second audio track after the first.
command is byte-for-byte what it was.
the copy path (which has no graph).
AudioOutLabel. FFmpeg refuses to mapone filter output twice (
Output with label 'aout' … was already used elsewhere, exit 234, nothing published), and "the mix twice" is not a secondtrack anyway.
put on a wire and read back.
What I measured
internal/ffmpeg.TestTwoDistinctMixesReachAnRTMPFarEnd. No credentials, noplatform, runs in CI.
5000 Hz — the multistream suite's tones, for its reasons.
DestinationArgsreturns is run against a real FFmpeg,with the relay input swapped for that file and
-readded so there is a livestream to attach to. Nothing else about the command is touched.
internal/rtmpserver— the RTMP server this productships, not an
ffmpeg -listen 1sink. It performs the handshake, addressesthe stream by key, and its subscriber side has to carry the E-RTMP
multitrack messages a second audio track arrives as.
-map 0 -c copy, so a track thatarrived is recorded whether or not the test expected it.
astats, the multistreamsuite's idiom.
Result, reproducible on FFmpeg 8.1.2 (CI pins n8.1):
How I know it is two tracks and not one duplicated
A track count cannot tell those apart: two streams, two codecs, two plausible
bitrates, and the second one worthless. So:
must have its own band above -45 dBFS and the other band at least 20 dB below
it — plus a band-balance spread of at least 30 dB between the two tracks.
The measured spread is 81 dB.
the same mix into both labels and asserts the spread is absent. It measures
46.7 / 46.7 — two real tracks (both are checked against the presence floor
first, so this is not a measurement of silence), zero apart. Without that
subtest, "the tracks differ by 30 dB" would be a sentence nobody had watched
fail.
Mutation testing
Every test here was broken against the committed tree and watched to fail by
name, then restored from a file backup with
git diff --statconfirmed clean.Each mutation is recorded in its test's doc comment. The wire test was killed
from both ends:
secondAudioMapreturning nil → both subtests fail withthe built command exited (exit status 234) without ever publishing, FFmpegnaming the now-unmapped
vodout.case *message.AudioExMultitrackininternal/rtmpserver'sisSetupreturning false, so the second track's decoder configuration isnever replayed → both subtests fail with
the subscriber never finished identifying the stream and was killed after 30s. This is what makes the testa measurement of the far end rather than of an argv.
The argv tests were killed by, respectively: returning a second map
unconditionally; returning nil; deleting the same-label guard; deleting the
DestAudioguard; and — for the copy-path arm, which first claimed to beunmutatable — appending
secondAudioMaptocopyAudioArgs, which is exactlyhow somebody would extend this feature to the copy path and puts a filter label
on a command that has no filter graph.
Two failure paths were made louder while doing this, because both first
appeared as slow, misleading failures: a publisher that exits without ever
publishing is now reported immediately with FFmpeg's own message instead of
after a 25 s handshake deadline, and a subscriber killed by its context now says
that a track whose configuration the far end did not replay looks exactly like
that.
No new
t.Skip— deliberately, including notesting.Short()guard, whichwould be a new site on the #161 ratchet for a pass nobody in this repo takes.
internal/testenv/testdata/skips.jsonis unchanged.What this does NOT establish
the
copyProblemsrefusal that rests on it stands.routing.Compileemits one mixwith fixed internal labels (
a_t0,a_mix,aout), so two compiled graphscannot simply be concatenated — they collide on every label. No DB column, no
API field, no UI. That is Implement Enhanced Broadcasting (IVS Multitrack Video): negotiated config, global-contribute ingest, and the VOD audio track #326's work; this is the capability it needs, with
the wire question answered first.
audioCodecArgsnames no stream, so both tracks takethe destination's bitrate: two tracks cost twice the audio bitrate, and there
is no way to ask for less on the second.
index. Nothing here labels one track "live" and the other "VOD", and nothing
was measured about whether a receiver could tell which is which beyond order.
video destination because the argv shape is identical, but only the RTMP/FLV
path was put on a wire.
Gates:
gofmt -l,go vet ./...,go build ./...,go test ./..., andshellcheck -S warning scripts/acceptance-multistream.sh(comment-only edit;the same 9 pre-existing warnings before and after).
https://claude.ai/code/session_01HeLrWaDmsNeeNSbHQfEofX