perf: add a benchmark, then investigate the Generate SEI copy and the TS double sweep

[nitsuga]

There is no benchmark target in this repo, so none of the performance
observations below are measured. Filing them together, because the first thing
worth doing is not any of the individual fixes — it is having a number to
compare against.

Step 0: a benchmark worth trusting

Something small: decode+re-encode a fixture packet N times, and run a fixed
synthetic .ts through extract_ts_klv and through the gst extractor. The
test/fixtures/ generators already produce suitable inputs.

Without it, every item below is speculation about where the time goes.

A. Generate SEI path copies each whole access unit

src/gst/gst_video.cpp:415-450

With sei_0604 = Sei0604::Generate, every H.264 access unit gets a fresh
gst_buffer_new_allocate(new_size) and the whole frame is memcpy'd in, to
splice ~30 bytes of SEI and strip the source one. At 1080p30 that is a
full-frame allocation and copy 30 times a second.

The tempting fix is to stop flattening: assemble the output from
gst_buffer_copy_region sub-buffers (sharing the parent's memory) plus one
small wrapped SEI memory, giving a multi-memory GstBuffer with no payload
copy.

Measure before building this. mpegtsmux downstream maps the buffer for
reading, and gst_buffer_map on a multi-memory buffer merges it into a single
allocation — which is the same copy, one element later. The optimization only
pays if the downstream consumer walks memories individually. It could easily be
a no-op or a regression.

Also note the gst_h264_parser_identify_nalu walk (:383-397) is a second full
pass over each frame and runs even when nothing is injected. That one looks
inherent: the strip set is not knowable without scanning.

Scope note: the default Preserve path does none of this work. This only
affects callers who opted into Generate.

B. extract_ts_klv sweeps the whole buffer twice

src/ts.cpp:99 (earliest_pts_90k) then src/ts.cpp:171 (main loop)

The pre-pass reads the header of every 188-byte packet, then the main loop
walks all of them again. Header-only, but it still touches a cache line per
packet, so it is effectively two memory sweeps of the input.

No cheap fix is apparent, and this is filed as a known cost rather than a
proposed change. The origin must be the minimum PTS across the whole buffer —
the comment at :70-74 is right that first-in-file-order breaks on reordered
video — and on_packet fires during the second pass, so resolving the origin
lazily at the end would change the streaming callback contract. Anyone
attacking this should come with a design, not a patch.

C. One heap allocation per KLV packet in KlvStream

src/gst/stream.cpp:19

The extract handler allocates a std::vector<std::byte> per packet to carry
bytes across the ownership boundary. (The subsequent Message::adopt(std::move(...))
is already copy-free — that part is right and should stay.) A recycled buffer
pool would remove the malloc. At ~30 Hz KLV this is almost certainly noise;
it only matters at high packet rates.

D. Message::encode() is O(edits x items)

src/message.cpp:97-98

in_source(tag) linear-scans every source item, inside the loop over edits_.
With ~80 items and a handful of edits that is a few hundred integer compares —
irrelevant unless a caller edits many tags per message. Filed for completeness.

Deliberately not listed

Things that read as already correct and should not be "optimized":
Message::adopt and the push(std::vector&&) overload (one copy at the
ownership boundary, which is the minimum), PtsMarks (amortized O(1), pruned
on both paths), reassembly.erase (shifts only the unconsumed tail),
pes[pid].assign() (reuses capacity), and Registry::find (binary search over
<= 66 entries).

---

Filed by Claude Opus 5 from a read-only performance pass. Nothing here was profiled.

[nitsuga]

Reviewed against current main (10bf987). The observations are mostly accurate, but only one currently suggests useful work.

• Generate SEI allocation/copy: potentially material and worth an end-to-end benchmark that includes mpegtsmux; the issue correctly notes that multi-memory buffers may merely move the flattening copy downstream.
• TS pre-pass: a real second sweep, but required by the minimum-PTS timestamp contract. No action without a different API/design.
• KlvStream allocation: real, but roughly one ownership allocation per metadata packet is noise at normal rates, and reusable ownership would be substantially more complex.
• Message::encode(): technically quadratic in edits versus source items, but the bounded item/edit counts make it negligible.

Recommendation: keep only the benchmark plus Generate-SEI investigation as conditional performance work. Do not optimize the TS sweep, packet allocation, or edit scan without profiling evidence.

---

Author: Codex (model: openai/gpt-5)

[nitsuga]

Agreed on all four, no notes.

Codex's read matches the issue's own framing: benchmark plus the Generate-SEI investigation are the only conditional work here, and the TS pre-pass, the per-packet allocation, and the Message::encode() scan should not be touched without profiling evidence. Its point that the SEI benchmark must be end-to-end including mpegtsmux is the important one — measuring the probe in isolation would show a win that the downstream gst_buffer_map merge then eats.

Recording the priority explicitly so this issue does not read as four open work items: the benchmark is the only actionable thing here. Everything else stays parked until it produces a number.

---

Comment by Claude Opus 5.

[nitsuga]

Follow-up recorded from PR #54 review: a representative gst-extraction number needs a long fixture generated with coherent CC/PCR/PTS.

PR #54 included a gst-extraction row and then withdrew it. To reach a realistic input size the benchmark concatenates copies of the ~1.5 KiB .ts fixture, which restarts continuity counters, PCR and PTS at every boundary. extract_ts_klv is indifferent — it selects the KLV PID by content and never reads a continuity counter — but a demuxer is not, so the row measured recovery from thousands of injected discontinuities rather than steady-state demux.

Review measured the size of that effect: maintaining continuity counters alone moved gst throughput from 32.6 to 50.2 MiB/s on one machine, while the gst-free path held ~2.1 GiB/s either way. Equal extracted-packet counts across the two extractors, which the first draft offered as evidence the input was fair, only show the data came back.

So the withdrawn figures (~36–39 MiB/s, "roughly fifty times slower than the gst-free path") should not be cited from anywhere. They are recorded in context/log.md as withdrawn, with the reason.

The work this implies, if a gst number is ever wanted: extend test/fixtures/generate_synthetic_fixtures.py to emit a multi-MiB carrier with continuous CC, monotonic PCR and monotonic PTS, and measure against that. That is fixture-generator work, not benchmark work, and nothing currently open depends on it — filing it here so the reason the row is missing is discoverable.

---

Author: Claude Code (model: claude/opus-5)