New api draft - rebased - #1610
Draft
Sakoram wants to merge 16 commits into
Draft
Conversation
added 16 commits
June 10, 2026 14:13
Import the additive unified session API subsystem from new_api_draft: - lib/src/new_api/ (mt_session*, video tx/rx, buffer, event, common) - include/mtl_session_api.h public header - app/sample/new_api/ samples - meson wiring (subdir new_api, header install, sample executables) Video ST20 only; audio/anc/st22 vtables are extern-declared but unused. Verified: zero pipeline-symbol usage; builds clean against main with no source changes (libmtl.so + 4 new samples link OK).
Re-apply the new_api_draft migration of the st20p RxTxApp paths on top of main, routing frame get/put/create/destroy through mtl_session_* instead of st20p_*. Preserve main's per-frame pkts modulo-gated dbg logging and the app_rx_st20p_result wire-stats reconciliation block (ported to mtl_session_io_stats_get). app_base.h st20p tx/rx structs now hold mtl_session_t* session. Deferred: ST20P_TX_FLAG_DROP_WHEN_LATE has no new-API flag yet (feature gap).
Add tests/unit/new_api/ harness + suite (St20NewApiRxTest) mirroring the old-API pipeline suite (St20PipelineRxTest) so both the pipeline and the unified-session RX paths have parallel coverage of the same producer/consumer contract. Both suites pass (12/12); full UnitTest 268/268. Characterization tests of existing imported code (Gate-2 failing-first exempt). New-API adaptations vs pipeline: busy atomic -> removed (none in new_api); corrupted counter -> incomplete-delivery assertion; stats overlay -> abstract stats_get vs passthrough io_stats.
- tx: emit warn() when st20p drop_when_late is set, so the unsupported unified-session option is an observable no-op instead of a silent one (reviewer MAJOR). - rx: hash s->st20p_frame_size in sha_check to match the TX side, instead of buf->data_size, so partial-recv / multi-plane frames stay symmetric (reviewer MINOR).
Add MTL_SESSION_FLAG_DROP_WHEN_LATE to the new unified session API for st20 video TX. Raw st20_api.h has no ST20_TX_FLAG_DROP_WHEN_LATE, so the late-drop is enforced in video_tx_get_next_frame: when the flag and USER_PACING are set, a READY frame whose TAI timestamp is more than one frame period in the past is recycled to FREE instead of transmitted, a buffers_dropped stat is bumped, and an MTL_EVENT_FRAME_LATE event is posted. Drops per call are capped at MT_VIDEO_TX_DROP_MAX_BATCH, matching the pipeline's ST_TX_DROP_MAX_BATCH, so a backlog cannot stall the datapath thread. The per-frame TAI is already stored in st20_frames[].tv_meta by tx_apply_buffer_metadata, so no new plumbing is needed. Add a new-API TX unit harness/test pair mirroring the existing RX harness, and an old-API st20 pipeline TX harness/test mirror so the two TX paths are pinned by parallel suites (lifecycle, user_meta passthrough, derive no-convert, stats surfaces, non-blocking get, framebuff count, and the three drop-when-late cases). Both suites pass under UnitTest.
Expand the new-API (unified session) st20 video RX unit suite from the
6 stats-only cases to cover every mapped RX feature, and mirror the
feature cases on the old st20 pipeline RX suite so the two stay parallel.
Test-only change; no lib edits.
New St20NewApiRxTest cases (characterization of existing production code
in lib/src/new_api/mt_session_video_rx.c, driven via the harness #include
of the production .c — no NIC/EAL device):
- StatusCompleteVsReconstructed: COMPLETE and RECONSTRUCTED both collapse
onto MTL_FRAME_STATUS_COMPLETE; CORRUPTED -> INCOMPLETE + flag.
- ConvertTransportToApp: derive is zero-copy (no converter); transport!=app
runs the (stubbed) converter and the buffer carries the app fmt/size.
- UserMetaPassthrough: frame_trans user_meta aliases into buf->user_meta.
- VideoMetaFieldsFilled: pkts_total/pkts_recv[]/second_field/dimensions.
- RtpTimestampAndTfmt: rtp_timestamp/tfmt/timestamp forwarded from meta.
- FramebuffCntClampedToTwo: drives the real mtl_video_rx_session_init clamp
via a stubbed st20_rx_create that captures ops.framebuff_cnt and fails.
- BlockGetTimeoutSemantics: timeout_ms=0 on an empty ready_ring is a bounded
poll returning -ETIMEDOUT, never a condvar block.
- BuffersProcessedSiteCrossMode: pins divergence #4 — library-owned bumps
buffers_processed at buffer_get, user-owned-post at notify_frame_ready.
- AutoDetectPostsFormatEvent: video_rx_notify_detected posts
MTL_EVENT_FORMAT_DETECTED with the detected geometry.
- ExtFrameQueryWrapperBinds: mem_register + buffer_post + the query_ext_frame
wrapper bind a posted user buffer to a transport ext_frame slot.
Harness (st20_rx_harness.{c,h}) gains: caller-supplied-meta injection, a
recorded video_convert_frame stub, an st20_rx_create capture stub for the
clamp, an event ring + poll wrapper, user-owned-post setup, and mem_register/
buffer_post/query_ext_frame wrappers.
Mirrored on St20PipelineRxTest (where a real old-pipeline analog exists):
StatusReconstructedPassthrough (raw status, no collapse), RtpTimestampAndTfmt,
VideoMetaFieldsFilled (pkts_total/second_field; pipeline copies no pkts_recv),
and UserMetaPassthrough (value-copied into the framebuf user_meta buffer). The
pipeline harness now injects caller-supplied meta and allocates per-framebuf
user_meta scratch.
Deferred to Phase 4 (integration), noted in the test file:
- TimingParserPostsReport: MTL_EVENT_TIMING_REPORT is posted nowhere in
lib/src/new_api; the timing-parser->event bridge is not yet wired, so
there is no unit-tier production path to exercise.
All 300 UnitTest cases pass (26 across the two RX suites); clang-format-14
and build.sh clean.
BLOCKER 1: tx_frame_is_late treated an unstamped slot (timestamp==0, tfmt==0==TAI) as infinitely late and silently dropped it. Gate the late check on timestamp != 0 so only frames the user actually stamped with TAI are eligible. BLOCKER 2: the drop path recycled the slot and posted only FRAME_LATE, so in MTL_BUFFER_USER_OWNED mode the external buffer (s->user_buf_ctx[i]) was never returned and leaked. Mirror video_tx_notify_frame_done: retrieve and clear user_buf_ctx[i] and post MTL_EVENT_BUFFER_DONE with user_ctx in addition to FRAME_LATE. Remove the unreachable MT_VIDEO_TX_DROP_MAX_BATCH cap (the for-loop is already bounded by the framebuffer count; tripping it would have transmitted a late frame) and the redundant frame_late.epoch_skipped=0.
…HEN_LATE The flag now exists in the unified session API, so replace the placeholder warn() no-op (added when the flag was absent) with the real flag mapping, making drop-when-late reachable end-to-end through RxTxApp.
- mtl-build skill: add the Unit Tests build+run section (exact binary path build_unit/tests/unit/UnitTest), fix Format guidance to prefer formatting only touched files over the whole-tree format-coding.sh churn, and add the broken-jobserver + stale-.ninja_deps phantom-crash traps to Common Build Errors. - copilot-instructions: orchestrator should independently re-run build + full unit suite after a Develop handoff; reported crashes are often stale-build artifacts, not regressions.
Three unit-provable crash/correctness fixes in the new unified session API: - B5: size the buffer wrapper pool from the actual low-level frame count (forced >= 2), not the raw num_buffers. With num_buffers 0/1 the hot-path `buffers[idx % buffer_count]` divided by zero or aliased frames. Create now always calls mtl_session_init_buffers(), sized via mtl_session_video_frame_count(). - B6: video_tx_slice_ready() returned a fake success (0) for an unimplemented mode; return -ENOTSUP to match the RX twin and the dispatcher convention. - RX-ext: external-frame completion stored the library's internal user_meta as the user ctx; store the app's ext-frame opaque (meta->opaque) instead. Adds four RED-before/GREEN-after unit tests pinning each fix (BufferPoolSizeFollowsFrameCount rx+tx, SliceReadyUnsupported, ExtFrameCompletionCarriesAppOpaque). Full unit suite 309/309.
Redesign the event system to honor the producer/consumer model: - Producer (tasklet via notify_* callbacks) now appends WITHOUT allocating or blocking: replace the malloc-per-post pointer ring with a value-backed rte_ring_create_elem(sizeof(mtl_event_t), RING_F_SC_DEQ). On a full ring it drops and bumps events_dropped (atomic), never waits. This removes the per-event mt_rte_zmalloc/mt_rte_free from tasklet context. - Consumer (app thread) now BLOCKS on the eventfd via poll() honoring timeout_ms instead of a usleep(100) busy-wait. timeout==0 -> -ETIMEDOUT, stop() -> -EAGAIN (distinct, P7/P27), with the sub-ms wait rounded up to avoid a poll(...,0) spin. - stop() now signals the eventfd (mtl_session_events_signal, async-signal-safe non-blocking write) so a consumer already blocked in poll() wakes promptly and returns -EAGAIN, instead of stalling for the full timeout. - Implement get_event_fd on both video vtables; drop the now-dead pointer-mode fallback dequeue in the dispatcher. Scope: event mechanism only. The BUFFER_DONE-per-frame flood, user-owned conversion, and tasklet re-enqueue malloc are addressed in later Phase 2 steps. Unit: 320/320 (11 new event/stop tests incl. a discriminating blocked-poll stop-wakeup concurrency test). Reviewer: APPROVE.
In library-owned TX mode a completed transmission returns the frame slot to the FREE pool implicitly and the app reuses it via the next buffer_get(); it does not consume a per-frame completion event. video_tx_notify_frame_done was nonetheless posting MTL_EVENT_BUFFER_DONE on every transmitted frame, flooding the 64-entry value-backed event ring (nothing drains it at frame rate) and dropping real events. Gate the BUFFER_DONE post on s->ownership == MTL_BUFFER_USER_OWNED, mirroring the existing drop_when_late path. Stats updates and the TX_FRAME_FREE store stay unconditional; FRAME_LATE and RX are untouched. BUFFER_DONE.ctx (user_ctx) is only meaningful for user-owned external buffers anyway. Unit: 322/322. New tests: LibraryOwnedFrameDonePostsNoEvent (fails without fix), UserOwnedFrameDonePostsBufferDone (distinct data/ctx sentinels assert ctx round-trip). Reviewer: APPROVE.
…ser-buf ring (Phase 2.3, B3+B4)
User-owned (zero-copy) TX previously did, inside the pacing tasklet
(video_tx_get_next_frame):
- a full-frame video_convert_frame for non-derive (format-mismatch) buffers,
- per-buffer mt_rte_zmalloc/free for the user-buf ring entries (B3),
- a re-enqueue malloc when no slot was free.
Both are data-plane violations (heavy work + allocation in the tasklet).
B3: make user_buf_ring value-backed via rte_ring_create_elem(sizeof(entry));
enqueue/dequeue by value, no per-buffer malloc/free. The ring is now touched
only by the app thread (buffer_post / uinit).
B4: move slot binding + conversion onto the app thread (buffer_post):
- video_tx_claim_free_slot: CAS FREE->APP_OWNED.
- video_tx_fill_slot: derive(st20_tx_set_ext_frame) or non-derive
(video_convert_frame into the slot framebuffer), store user_ctx, then
RELEASE-publish READY; on hard error release the slot to FREE.
- buffer_post drains the backlog claim-then-pop (a slot is reserved before
the head is dequeued, so the backlog is never reordered = strict FIFO at
the bind stage); a new buffer goes to the tail when a backlog exists.
- get_next_frame is now a pure READY->TRANSMITTING scan: no convert, no
enqueue/dequeue, no malloc.
Safety: app thread claims a slot exclusively (APP_OWNED) before
st20_tx_set_ext_frame (which only touches that slot, refcnt==0) and before
RELEASE-publishing READY; the tasklet ACQUIRE-claims READY->TRANSMITTING, so
the converted framebuffer and user_ctx are fully visible.
Completion contract: buffer_post returning 0 yields exactly one async
BUFFER_DONE; a negative return yields none. Backlog entries (already accepted
with a 0 return) get a BUFFER_DONE on hard bind error so the app is not stranded.
Unit: 325/325 (new UserOwnedConvertHappensOnPostNotTasklet,
UserOwnedValueRingRoundTrip, UserOwnedPostBackpressureDefersAndBinds - the last
defers 3 entries, frees one slot, and asserts the OLDEST binds first; fails
against the prior head-rotation logic). Reviewer: APPROVE.
Known follow-up (Phase 4 lifecycle): no buffer_flush yet, so a backlog is
drained only by subsequent buffer_post; documented in the buffer_post contract.
Bring the new_API docs into the repo and align them with the current post-Phase-2 implementation: - CURRENT_STATE.md: rewritten to lib/src/new_api/ paths, value-backed event ring + eventfd poll, frame-state model, implemented buffer_post/ mem_register, completion contract, and the -ENOTSUP/TODO list. - GRACEFUL_SHUTDOWN.md: guard-centric teardown. destroy() is the single safe primitive (validated generation-tagged handle + refcount drain + eventfd wake); stop() is optional for SIGINT quiesce and pause/resume. - List-of-changes.md: fixed dangling links, annotated auto-start divergence. - samples/: reference the real app/sample/new_api samples; slice/ST22 diagrams marked not-yet-implemented. Docs describe the target guard design; the guard itself is not yet wired into mt_session.c.
Collaborator
Author
|
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
1 participant
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.
just a draft of how API could look like.
Next step: port fully RxTxApp, integration tests and plugins to run full test suite
notes: