Skip to content

refactor: carry moq-video decode timestamps as moq_net::Timestamp#2146

Merged
kixelated merged 1 commit into
devfrom
claude/moq-video-deferred-work-a32311
Jul 11, 2026
Merged

refactor: carry moq-video decode timestamps as moq_net::Timestamp#2146
kixelated merged 1 commit into
devfrom
claude/moq-video-deferred-work-a32311

Conversation

@kixelated

@kixelated kixelated commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

The moq-video decode path carried presentation timestamps as a bare u64 of microseconds, the one spot in the media pipeline not already using moq_net::Timestamp — the container frame is container::Frame.timestamp: Timestamp and encode::Producer::publish takes a Timestamp too. This threads Timestamp through decode so it is symmetric with encode and the container.

Note: this PR was originally about surfacing per-output decoder PTS through the backends. That work landed independently on dev via #2145 (which added the NVDEC backend and the zero-copy NVDEC → NVENC path). Rebased onto #2145, the only remaining, non-redundant improvement is the timestamp typing, so the PR is now scoped to that.

What changes

u64 microseconds → moq_net::Timestamp across the decode surface:

  • decode::Frame.timestamp, decode::Decoder::decode, and the pub(crate) Backend::decode / Decoded.
  • All four backends: openh264 / VideoToolbox / Media Foundation echo the input timestamp (unchanged behaviour, just retyped); NVDEC continues to thread it through the cuvid parser (Timestamp::as_micros() in, Timestamp::from_micros() out).

Deletes three lossy conversions that only existed to bridge the u64 gap:

  • decode::Consumer::read now passes mux_frame.timestamp straight through (was as_micros().try_into().map_err(TimeOverflow)).
  • moq-transcode's rung serving drops an as_micros() on the way in and a from_micros() on the way out; the decode→encode timestamps now flow as Timestamp end to end.

libmoq's C ABI is unchanged (moq_video_frame.timestamp_us: u64); the adapter converts via frame.timestamp.as_micros().

Public API change (moq-video)

  • decode::Frame.timestamp_us: u64decode::Frame.timestamp: moq_net::Timestamp
  • decode::Decoder::decode(payload, timestamp_us: u64, keyframe)decode(payload, timestamp: moq_net::Timestamp, keyframe)

Breaking reshapes of pub items, but moq-video/moq-transcode are unreleased (dev-only, 0.0.x) with no published contract, so this lands on dev. No wire-protocol or C-ABI change.

Test plan

  • macOS: cargo build/test -p moq-video; decode round-trip tests pass (VideoToolbox H.264 + H.265, openh264), now also asserting the timestamp round-trips through the codec and returns strictly increasing.
  • macOS: moq-transcode builds; clippy clean (moq-video --all-targets, moq-transcode --lib); rustdoc -D warnings clean; cargo fmt applied.
  • Windows: MediaFoundation backend cross-compiles via cargo zigbuild --target x86_64-pc-windows-gnu.
  • Not verified locally (mechanical edits, left to CI): the NVDEC backend (Linux + CUDA) and libmoq — both are un-buildable on macOS on this branch independent of this change (a pre-existing dev feature/target quirk), so CI is the check for them.

🤖 Generated with Claude Code

(Written by Claude Opus 4.8)

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @kixelated, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@kixelated kixelated enabled auto-merge (squash) July 10, 2026 19:22
@kixelated kixelated disabled auto-merge July 10, 2026 19:24
@kixelated kixelated force-pushed the claude/moq-video-deferred-work-a32311 branch from 384a1d3 to c477852 Compare July 10, 2026 19:40
@kixelated kixelated changed the title feat: moq-video surface per-output decoder PTS through decode backends refactor: carry moq-video decode timestamps as moq_net::Timestamp Jul 10, 2026
@kixelated kixelated enabled auto-merge (squash) July 10, 2026 19:40
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Caution

Review failed

An error occurred during the review process. Please try again later.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch claude/moq-video-deferred-work-a32311

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kixelated kixelated force-pushed the claude/moq-video-deferred-work-a32311 branch from c477852 to 5833d9e Compare July 10, 2026 19:51
@kixelated kixelated disabled auto-merge July 11, 2026 01:53
The decode path carried presentation timestamps as a bare `u64` of
microseconds, the one spot in the media pipeline not already using
`moq_net::Timestamp` (the container frame is `timestamp: Timestamp` and
`encode::Producer::publish` takes one too). Thread `Timestamp` through
decode instead: `decode::Frame.timestamp`, `Decoder::decode`, the
`pub(crate)` `Backend::decode` / `Decoded`, and all four backends
(openh264 / videotoolbox / mediafoundation echo the input timestamp,
nvdec threads it through the cuvid parser as before).

This makes decode symmetric with encode and the container, and deletes
three lossy `as_micros()` / `from_micros()` conversions: one in
`decode::Consumer::read` and two in `moq-transcode`'s rung serving.
libmoq's C ABI stays `moq_video_frame.timestamp_us: u64`; the adapter
converts via `Timestamp::as_micros()`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@kixelated kixelated force-pushed the claude/moq-video-deferred-work-a32311 branch from 5833d9e to d31e830 Compare July 11, 2026 02:08
@kixelated kixelated enabled auto-merge (squash) July 11, 2026 02:09
@kixelated kixelated merged commit c6908c4 into dev Jul 11, 2026
3 checks passed
@kixelated kixelated deleted the claude/moq-video-deferred-work-a32311 branch July 11, 2026 02:29
kixelated added a commit that referenced this pull request Jul 11, 2026
…refactor

Rebase fallout from #2146: decode::Frame::resize preserves `timestamp` (the
field `timestamp_us` no longer exists), and the shared feed passes container
timestamps through as-is instead of converting to microseconds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

1 participant