From 399dc3ce8b68551c0aa70f9dbf1f71d507089cd0 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 21 Sep 2026 16:27:10 +0200 Subject: [PATCH 1/3] fix(moq-audio,moq-video): build the capture feature against the unified Clock #3813 replaced `moq_mux::Clock::micros() -> u64` with `Clock::now() -> Timestamp`, and updated every caller except the two behind the `capture` feature. `cargo check -p moq-audio --features capture` and the same for moq-video fail on `main` as a result, and nothing in CI builds that feature, so it went unnoticed. The video producer wanted a `Timestamp` anyway and was converting into one, so it now takes `clock.now()` directly and loses a fallible conversion with it. The audio capture path feeds a converter that counts in microseconds, so it reads `now().as_micros()`, which is what `micros()` computed before. `moq_net::Timestamp` in the video producer is left needed only by its tests, so its import narrows to `cfg(test)`. Co-Authored-By: Claude Opus 5 (1M context) --- rs/moq-audio/src/encode/capture.rs | 2 +- rs/moq-video/src/encode/producer.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/moq-audio/src/encode/capture.rs b/rs/moq-audio/src/encode/capture.rs index abdd817619..56a824cbb6 100644 --- a/rs/moq-audio/src/encode/capture.rs +++ b/rs/moq-audio/src/encode/capture.rs @@ -867,7 +867,7 @@ impl Output for EncoderOutput<'_, E> { } fn now(&self) -> u64 { - self.clock.micros() + self.clock.now().as_micros() as u64 } fn write(&mut self, samples: capture::Samples, timestamp_us: u64) -> Result<(), Error> { diff --git a/rs/moq-video/src/encode/producer.rs b/rs/moq-video/src/encode/producer.rs index 5e8aaa3a00..886f8cb754 100644 --- a/rs/moq-video/src/encode/producer.rs +++ b/rs/moq-video/src/encode/producer.rs @@ -12,7 +12,7 @@ use std::time::Instant; use moq_mux::catalog::hang::CatalogExt; -#[cfg(any(feature = "capture", test))] +#[cfg(test)] use moq_net::Timestamp; use crate::Error; @@ -568,7 +568,7 @@ async fn capture_loop( // Stamp at capture, so a backend that buffers still publishes each // access unit at the time the picture was grabbed. - let frame = Frame::new(surface, Timestamp::from_micros(clock.micros())?); + let frame = Frame::new(surface, clock.now()); if force_keyframe { encoder.keyframe(); force_keyframe = false; From 9a0f90e4581a79714b0c432867f73a9890b45355 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 21 Sep 2026 16:55:08 +0200 Subject: [PATCH 2/3] fix(moq-video): pass the MediaCodec bitrate in bits per second The encoder's resolved bitrate became a bandwidth `Rate`, and the Android backend still handed it to a helper that takes a plain `u64`. The file is only compiled for Android, so no host build caught it; moq-video failed to build for any Android target. Co-Authored-By: Claude Opus 5 (1M context) --- rs/moq-video/src/encode/backend/mediacodec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/moq-video/src/encode/backend/mediacodec.rs b/rs/moq-video/src/encode/backend/mediacodec.rs index 2ddecafe2a..2ec08e54da 100644 --- a/rs/moq-video/src/encode/backend/mediacodec.rs +++ b/rs/moq-video/src/encode/backend/mediacodec.rs @@ -431,7 +431,7 @@ fn encoder_format(config: &Config, mime: &str) -> MediaFormat { format.set_i32(KEY_WIDTH, config.width as i32); format.set_i32(KEY_HEIGHT, config.height as i32); format.set_i32(KEY_COLOR_FORMAT, COLOR_FORMAT_NV12); - format.set_i32(KEY_BIT_RATE, clamp_i32(config.resolved_bitrate())); + format.set_i32(KEY_BIT_RATE, clamp_i32(config.resolved_bitrate().as_bps())); format.set_i32(KEY_BITRATE_MODE, BITRATE_MODE_CBR); format.set_i32(KEY_FRAME_RATE, config.framerate as i32); format.set_i32(KEY_PRIORITY, PRIORITY_REALTIME); From c4bff8dfd41383f334f51853a4c7cac76d44c3e4 Mon Sep 17 00:00:00 2001 From: Frando Date: Mon, 21 Sep 2026 17:19:35 +0200 Subject: [PATCH 3/3] ci: compile the capture feature and the Android backends on pull requests Both fixes before this one are for code no pull-request job compiles. The device code in moq-video and moq-audio sits behind the off-by-default `capture` feature, and moq-video's MediaCodec backend behind `target_os = "android"`. The Linux gate compiles neither, so #3813's Clock rename and the move to `bandwidth::Rate` both reached main with their callers broken. Nightly's macOS job did report the first, after the merge. `check-changed` now runs a new `capture` recipe whenever moq-video or moq-audio is selected. Selection includes dependents, so a moq-mux or moq-net diff reaches it, and `_select-test` asserts that a moq-mux diff does and a moq-relay diff does not. It lints just those two crates with `capture`, not `--all-features`, which would also build the bindgen codecs nightly already covers. android.yml runs the existing `just rs android` recipe on an ubuntu runner, outside Nix because the dev shell has no NDK and the runner image does. It triggers on moq-video's in-repo dependency graph. Checked by reverting each fix: `just rs capture` fails on `Clock::micros`, and `just rs android` fails on the `Rate` mismatch. Both pass with the fixes. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/android.yml | 81 +++++++++++++++++++++++++++++++++++ rs/justfile | 43 +++++++++++++++++-- 2 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 .github/workflows/android.yml diff --git a/.github/workflows/android.yml b/.github/workflows/android.yml new file mode 100644 index 0000000000..9b8e9b4d85 --- /dev/null +++ b/.github/workflows/android.yml @@ -0,0 +1,81 @@ +name: Android + +# Compiles moq-video for Android, which nothing else on a pull request does. +# Its MediaCodec encoder and decoder, the `Surface::HardwareBuffer` variant and +# the `frame::android` module are all `#[cfg(target_os = "android")]`, so every +# Linux job compiles them to nothing, and a moq-net change that breaks them +# (a bitrate turning into a `bandwidth::Rate`, say) lands on main unnoticed. +# +# Outside Nix, unlike check.yml: the dev shell has no NDK, and the GitHub +# ubuntu image ships one that cargo-ndk finds through `ANDROID_NDK_HOME`. The +# recipe is still `just rs android`, so a local run and this one compile the +# same thing, and rust-toolchain.toml still pins the compiler. +# +# Narrow trigger, like wasm.yml. The filter is moq-video's in-repo dependency +# graph (`cargo metadata`, normal dependencies), plus the files that decide how +# it is built. A crate added to that graph has to be added here too. + +permissions: + contents: read + +on: + pull_request: + # `closed` is here only so merging/closing a PR cancels its in-flight run + # via the concurrency group below; the job itself is skipped on close. + types: [opened, synchronize, reopened, closed] + paths: + - "rs/moq-video/**" + - "rs/moq-mux/**" + - "rs/moq-net/**" + - "rs/moq-nvenc/**" + - "rs/moq-loc/**" + - "rs/moq-msf/**" + - "rs/moq-json/**" + - "rs/moq-binary/**" + - "rs/moq-flate/**" + - "rs/moq-pattern/**" + - "rs/hang/**" + - "rs/kio/**" + - "rs/justfile" + - "justfile" + - ".cargo/config.toml" + - "Cargo.toml" + - "Cargo.lock" + - "rust-toolchain.toml" + - ".github/workflows/android.yml" + +concurrency: + group: android-${{ github.ref }} + cancel-in-progress: true + +env: + CARGO_NDK_VERSION: 4.1.2 + JUST_VERSION: 1.52.0 + +jobs: + android: + name: Android + if: github.event.action != 'closed' + # x86_64 rather than the arm runners check.yml uses: GitHub documents the + # Android SDK and NDK as preinstalled on this image. + runs-on: ubuntu-24.04 + timeout-minutes: 30 + + steps: + - name: Checkout + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + # Run from the checkout, so rustup installs the toolchain rust-toolchain.toml + # pins and adds the target to that one rather than to the image's default. + - name: Add the Android target + run: rustup target add aarch64-linux-android + + # No Rust cache: this workflow only runs on pull requests, which may not + # save one, so it would have nothing to restore. + - name: Install just and cargo-ndk + run: cargo install --locked "just@$JUST_VERSION" "cargo-ndk@$CARGO_NDK_VERSION" + + - name: Check + run: just rs android diff --git a/rs/justfile b/rs/justfile index 9d2beeb8e7..b80e8ffb20 100644 --- a/rs/justfile +++ b/rs/justfile @@ -259,6 +259,15 @@ _wants-tokio-features $PACKAGES: set -euo pipefail grep -qw 'moq-tokio' <<< "$(just rs _names "$PACKAGES")" +# True when the selection includes a crate with a `capture` feature. Selection +# pulls in dependents, so a diff to moq-mux or moq-net lands here too, which is +# how a rename in either reaches the device code that calls it. +[private] +_wants-capture $PACKAGES: + #!/usr/bin/env bash + set -euo pipefail + grep -qwE '(moq-video|moq-audio)' <<< "$(just rs _names "$PACKAGES")" + # Print the crate names behind a list of `--package` flags. [private] _names $PACKAGES: @@ -300,6 +309,15 @@ _select-test: fail "a moq-relay diff must not drive it: $(just rs _names "$relay")" fi + # The break the capture gate was added for: a moq-mux API change that missed + # its callers behind `capture`. A moq-mux diff has to reach it. + mux=$(just rs _select "rs/moq-mux/src/clock.rs") + just rs _wants-capture "$mux" \ + || fail "a moq-mux diff must drive the capture gate: $(just rs _names "$mux")" + if just rs _wants-capture "$relay"; then + fail "a moq-relay diff must not drive the capture gate: $(just rs _names "$relay")" + fi + tokio=$(just rs _select "rs/moq-tokio/src/lib.rs") just rs _wants-tokio-features "$tokio" \ || fail "a moq-tokio diff must drive its feature matrix: $(just rs _names "$tokio")" @@ -382,6 +400,13 @@ check-changed $FILES: just rs wasm fi + # moq-video's and moq-audio's device code is behind `capture`, which is off + # by default, so the pass above never compiles it. Only nightly's + # `--all-features` did, and a break there lands on main. + if [[ "$packages" == "ALL" ]] || just rs _wants-capture "$packages"; then + just rs capture + fi + # Compile moq-tokio by itself at the feature extremes and with each crypto provider. # Its default-feature build is already part of the ordinary clippy pass above. tokio-features: @@ -479,15 +504,27 @@ uring *args: macos *args: cargo check --locked -p moq-video -p moq-audio --all-targets --all-features {{ args }} +# The Linux device code: moq-video's V4L2 and X11 capture and moq-audio's cpal +# capture, all behind the off-by-default `capture` feature. `check-changed` runs +# this whenever either crate is selected. The per-crate `--features` spelling +# keeps it to those two instead of widening what their dependencies compile. +# `--all-features` would also build the bindgen codecs, which nightly covers. + +# Compile and lint the capture feature, which the default check skips. +capture *args: + cargo clippy --locked -p moq-video -p moq-audio --all-targets --features moq-video/capture,moq-audio/capture {{ args }} -- -D warnings + # Same idea as `windows`/`macos`, for Android: moq-video's MediaCodec encoder and # decoder, its `Surface::HardwareBuffer` variant, and the `frame::android` module # are all `#[cfg(target_os = "android")]`, so a host-target check compiles none of # them and every error in them stays invisible. # # Unlike `windows` and `macos` this needs no special host: the Android targets -# cross-compile from the Linux dev shell in about half a minute. It needs an NDK, -# which `cargo-ndk` finds through `ANDROID_NDK_HOME` or the standard SDK layout, -# so it is a manual gate rather than a CI one until the shell provides one. +# cross-compile from Linux in about half a minute. It needs an NDK, which +# `cargo-ndk` finds through `ANDROID_NDK_HOME` or the standard SDK layout. The +# dev shell provides neither, so android.yml runs this outside Nix on a GitHub +# runner, whose image ships an NDK, for every PR that touches moq-video or +# anything it is built from. # # arm64-v8a alone: it is the only ABI anything here ships, and the code is not # ABI-specific, so the other two would compile the same source twice more.