mpph264enc could not say constrained-baseline, so H.264 was never offered

webrtcsink's codec discovery builds its encoding chain with no output caps, so `force_profile` is
true and it inserts a capsfilter demanding `profile=constrained-baseline` — WebRTC's interoperable
floor. h264parse strips `alignment`, `stream-format` and `parsed` from a caps query but not
`profile`, so that demand reaches mpph264enc's src pad, whose template listed only
`{ baseline, main, high }`. Empty intersection, GstVideoEncoder's sink getcaps returns nothing,
and the failure surfaces upstream as videorate reporting it "could not transform NV12 ... in
anything we support". Discovery then drops H.264 with a warning nobody was reading, VP8 wins by
default, and the session dies in rtpvp8pay.

The element could always produce constrained-baseline; only its static template denied it.
Measured on an RK3566: `mpph264enc profile=baseline ! h264parse` negotiates
`profile=(string)constrained-baseline` on the parser's src pad, because baseline mode turns CABAC
and 8x8 transform off and MPP emits no FMO, ASO or redundant slices. So this widens the template
by one word rather than claiming something new.

Patches now live in a directory per upstream, applied by an `apply_patches` helper and recorded in
the MANIFEST as `patch <project>/<file>`. Two reasons: a patch aimed at the wrong tree failed the
same way a stale one does, and the MANIFEST's flat `patch <file>` line landed next to whichever
.so happened to precede it, which read as provenance for the wrong plugin.

Assisted-by: Claude:claude-opus-5[1m]

README.md

@@ -7,8 +7,8 @@ Two plugins, for two unrelated reasons. Neither is packaged anywhere we can inst
 
 | plugin | provides | why it is here |
 |---|---|---|
-| `libgstrockchipmpp.so` | `mpph264enc`, `mpph265enc`, `mppjpegenc`, `mppvp8enc`, `mppvideodec`, `mppjpegdec` | Debian ships no Rockchip encoder in any suite. Radxa's own `gstreamer1.0-rockchip1_1.14-4` does contain them, so this build is about the pin, dropping `libx11-6`, and riding along with the plugin below — see [below](#the-permission-trap-that-hid-all-of-this). |
-| `libgstrswebrtc.so`, `libgstrsrtp.so` | `webrtcsink`, `webrtcsrc`, `rsrtp*` | `gstreamer1.0-plugins-rs` does not exist in **any** Debian suite — not trixie, backports, sid or experimental. |
+| `libgstrockchipmpp.so` | `mpph264enc`, `mpph265enc`, `mppjpegenc`, `mppvp8enc`, `mppvideodec`, `mppjpegdec` | Debian ships no Rockchip encoder in any suite. Radxa's own `gstreamer1.0-rockchip1_1.14-4` does contain them, so this build is about the pin, dropping `libx11-6`, and riding along with the plugin below — see [below](#the-permission-trap-that-hid-all-of-this). Patched; see [`patches/`](patches/). |
+| `libgstrswebrtc.so`, `libgstrsrtp.so` | `webrtcsink`, `webrtcsrc`, `rsrtp*` | `gstreamer1.0-plugins-rs` does not exist in **any** Debian suite — not trixie, backports, sid or experimental. Patched; see [`patches/`](patches/). |
 
 `webrtcbin` is **not** here: it comes from `gstreamer1.0-plugins-bad` in Debian and needs no
 build.
@@ -104,8 +104,25 @@ These are binaries built from other people's source, so where that source is mat
   [the upstream repository](https://gitlab.freedesktop.org/gstreamer/gst-plugins-rs) at the tag in
   `pins.env`.
 
-Nothing here is modified — no patches, no forks. Each release's `MANIFEST` names the repository
-and the exact ref per plugin, which is both the licence answer and the reason a media bug found on
+**Both upstreams are patched, and that matters for more than tidiness.** MPL-2.0 asks that
+modifications be identifiable, so they are stated here, listed in [`patches/`](patches/) with what
+each one buys and how it ends, and recorded in every release's `MANIFEST` — a release names each
+upstream ref *and* every patch applied over it.
+
+Two patches today, both about getting hardware H.264 to a browser:
+
+- **`gst-plugins-rs`** — `webrtcsink` inserts a software `videoconvert ! videoscale` in front of
+  any encoder it does not recognise, and `mpph264enc` converts on the SoC's 2D accelerator
+  instead, so a CPU pass over every frame is added to work the hardware was going to do anyway,
+  on cores the robot's control loop shares.
+- **`gstreamer-rockchip`** — `mpph264enc`'s pad template omitted `constrained-baseline` from its
+  profile list, which is the one profile WebRTC asks for. Without this, `webrtcsink` cannot offer
+  H.264 on an RK3566 at all, and the only trace is a `videorate` complaining about NV12.
+
+[`patches/README.md`](patches/README.md) has the reasoning for each, the trade-off it accepts, and
+the route upstream that would delete it.
+
+Together the ref and the patch list are both the licence answer and the reason a media bug found on
 a robot can be traced to a specific build.
 
 This repository's own build scripts are Apache-2.0, matching the daemon.

patches/README.md

@@ -0,0 +1,82 @@
+# Patches
+
+One directory per upstream — `gst-plugins-rs/`, `gstreamer-rockchip/` — applied to that
+project's checkout by `scripts/build.sh` in filename order, and recorded in every release's
+`MANIFEST` as `patch <project>/<file>` so a binary can be traced to the exact source that
+produced it.
+
+The routing is explicit rather than glob-everything, because a patch aimed at the wrong tree fails
+the same way a stale one does, and the two want different fixes.
+
+**Carrying a patch is a cost, so each one has to say what it buys and how it ends.** A patch with no
+route upstream is a fork with extra steps: it has to be re-cut at every version bump, and the
+binary stops being something anyone else can reproduce from a public ref alone.
+
+`build.sh` runs `git apply --check` first, so a patch that no longer applies fails the build naming
+itself, rather than silently producing a plugin missing the change it was carried for.
+
+---
+
+## `gst-plugins-rs/0001-webrtcsink-no-converter-for-mpph264enc.patch`
+
+**What it changes.** `make_converter_for_video_caps` in `net/webrtc/src/webrtcsink/imp.rs` builds
+the chain `webrtcsink` inserts in front of an encoder it selected. It special-cases hardware it
+knows — NVMM, D3D11, CUDA, GL, VA, and on `main` also `v4l2h264enc` — and falls back to software
+`videoconvert ! videoscale` for anything else. This adds an arm for `mpph264enc` that inserts
+nothing.
+
+**Why.** Rockchip's MPP encoder takes NV12, I420, YUY2 and more directly, and converts on the SoC's
+2D accelerator rather than the CPU. A software convert-and-scale pass in front of it costs a full
+CPU traversal of every frame on four Cortex-A55s — which is exactly what the hardware encoder is
+there to avoid, and which shares those cores with `robotd`'s 50 Hz control loop.
+
+**Why not just keep pre-encoding.** Because the robot did, and it costs more than it looks.
+Handing `webrtcsink` finished H.264 means it cannot reach the encoder, so two things it normally
+does silently do not happen: congestion control cannot adapt the bitrate to the link, and a peer's
+PLI cannot produce a keyframe — a viewer that loses one stays broken until the next periodic GOP.
+Letting `webrtcsink` own the encoder fixes both, and this patch is what makes that affordable here.
+
+**The trade-off it makes.** Without `videoscale` the bin cannot resize, so the negotiated
+resolution has to be one the source already produces. True on this robot, which pins its caps
+upstream of its tee — and the honest reason this may need discussion before upstream takes it, since
+a general fix would want RGA-backed scaling rather than none. `mpph264enc` has `width` and `height`
+properties that scale on the VPU, but nothing in `webrtcsink` sets them.
+
+**How it ends.** Upstream. The `v4l2h264enc` arm on `main` is the same shape for another hardware
+encoder, so the precedent exists; if it is taken, this file is deleted at the next version bump.
+Until then it is re-cut per bump, which `--check` will demand rather than let slide.
+
+---
+
+## `gstreamer-rockchip/0001-mpph264enc-advertise-constrained-baseline.patch`
+
+**What it changes.** One word in `mpph264enc`'s src pad template: its profile list was
+`{ baseline, main, high }` and is now `{ constrained-baseline, baseline, main, high }`.
+
+**Why.** Without it `webrtcsink` cannot offer H.264 on this SoC at all, and says so only at
+`GST_DEBUG=*:WARNING`. Its codec discovery pass builds the encoding chain with no output caps, so
+`force_profile` is true and it inserts a capsfilter demanding
+`video/x-h264, stream-format=avc, profile=constrained-baseline` — WebRTC's interoperable floor.
+`h264parse` strips `alignment`, `stream-format` and `parsed` from a caps query but **not
+`profile`**, so that demand reaches the encoder's src pad, whose template could not satisfy it.
+The intersection is empty, `GstVideoEncoder`'s sink getcaps returns nothing, and the failure
+surfaces far upstream as `videorate` reporting it "could not transform NV12 … in anything we
+support". Discovery then drops H.264 with a warning, VP8 is negotiated instead, and the session
+dies in `rtpvp8pay`. Nothing in the error names the profile.
+
+**Why it is true and not a convenient claim.** `mpph264enc profile=baseline` sets `h264:cabac_en`
+and the 8x8-transform flag off, and MPP emits no FMO, ASO or redundant slices — so the SPS it
+writes really does carry `profile_idc=66` with `constraint_set1_flag`. Measured on an RK3566
+rather than reasoned about: `videotestsrc ! mpph264enc profile=baseline ! h264parse` negotiates
+`profile=(string)constrained-baseline` on the parser's src pad. The element could always produce
+this; only its template denied it. A pad template is a capability set, not current state — the
+same template already advertises all three other profiles regardless of which one the property
+selects.
+
+**The trade-off it makes.** None that we can find, which is itself worth stating: the change only
+widens what the pad may agree to, and the encoder's own src caps still come from its `profile`
+property. A pipeline that asked for `baseline` before still gets it.
+
+**How it ends.** Upstream, at `JeffyCN/mirrors` or whichever Rockchip tree succeeds it. It is a
+one-word capability fix with a reproducer, which is the easiest kind to land; if it is taken, this
+file is deleted at the next pin bump. Until then `--check` demands it be re-cut per bump.

patches/gst-plugins-rs/0001-webrtcsink-no-converter-for-mpph264enc.patch

@@ -0,0 +1,28 @@
+--- a/net/webrtc/src/webrtcsink/imp.rs
++++ b/net/webrtc/src/webrtcsink/imp.rs
+@@ -685,6 +685,25 @@
+ 
+                 ret.add_many([&vapostproc])?;
+                 (vapostproc.clone(), vapostproc)
++            } else if codec
++                .encoder_factory()
++                .is_some_and(|factory| factory.name() == "mpph264enc")
++            {
++                // Rockchip's MPP encoder accepts NV12, I420, YUY2 and a dozen more formats on its
++                // sink pad, and performs any conversion it needs on the SoC's 2D accelerator
++                // (RGA) rather than on the CPU. So unlike the NVIDIA and VA cases above, what it
++                // wants is not a *better* converter but no converter at all: a software
++                // `videoconvert ! videoscale` in front of it is a full CPU pass over every frame,
++                // which on the four Cortex-A55s of an RK3566 is precisely the cost the hardware
++                // encoder exists to avoid.
++                //
++                // The trade-off, stated plainly: without `videoscale` this bin cannot resize, so
++                // the negotiated resolution has to be one the source already produces. That is
++                // true of the robot this is used on, which pins its caps upstream of the tee.
++                let identity = make_element("identity", None)?;
++
++                ret.add_many([&identity])?;
++                (identity.clone(), identity)
+             } else {
+                 let convert = make_element("videoconvert", None)?;
+                 let scale = make_element("videoscale", None)?;

patches/gstreamer-rockchip/0001-mpph264enc-advertise-constrained-baseline.patch

@@ -0,0 +1,11 @@
+--- a/gst/rockchipmpp/gstmpph264enc.c
++++ b/gst/rockchipmpp/gstmpph264enc.c
+@@ -98,7 +98,7 @@ GST_STATIC_PAD_TEMPLATE ("src",
+         GST_MPP_H264_ENC_SIZE_CAPS ","
+         "stream-format = (string) { byte-stream }, "
+         "alignment = (string) { au }, "
+-        "profile = (string) { baseline, main, high }"));
++        "profile = (string) { constrained-baseline, baseline, main, high }"));
+ 
+ static GstStaticPadTemplate gst_mpp_h264_enc_sink_template =
+ GST_STATIC_PAD_TEMPLATE ("sink",

scripts/build.sh

@@ -37,6 +37,34 @@ case "$WANT" in
     *) die "unknown target: ${WANT} (both, rockchip, webrtc)" ;;
 esac
 
+# Apply this project's patches to a fresh checkout, and record each in the MANIFEST.
+#
+#   apply_patches <project> <checkout>
+#
+# `<project>` is a directory under `patches/`, named for the upstream it patches — the routing has
+# to be explicit, because a patch against `net/webrtc/src/webrtcsink/imp.rs` applied to the
+# rockchip tree fails in a way that reads like a stale patch rather than a misdirected one.
+#
+# `--check` runs first so a patch that no longer applies stops the build naming itself, rather
+# than producing a plugin quietly missing the change it was carried for.
+apply_patches() {
+    project="$1"
+    checkout="$2"
+    dir="${ROOT}/patches/${project}"
+    [ -d "$dir" ] || return 0
+    for patch in "$dir"/*.patch; do
+        [ -e "$patch" ] || continue
+        name="$(basename "$patch")"
+        say "applying ${project}/${name}"
+        ( cd "$checkout" && git apply --check "$patch" ) \
+            || die "${project}/${name} does not apply to this ref.
+  It was written against a specific version of the file it touches. Re-cut it against the pin, or
+  drop it if upstream has taken the change — see patches/README.md."
+        ( cd "$checkout" && git apply "$patch" ) || die "${project}/${name} failed to apply"
+        printf 'patch %s/%s\n' "$project" "$name" >> "${DIST}/MANIFEST"
+    done
+}
+
 check_environment() {
     [ "$(id -u)" = 0 ] || die "run as root — it installs build dependencies"
     arch="$(uname -m)"
@@ -112,6 +140,8 @@ build_rockchip() {
     git -C "${src}/s" checkout -q "$GST_ROCKCHIP_REF" \
         || die "${GST_ROCKCHIP_REF} is not on ${GST_ROCKCHIP_BRANCH}"
 
+    apply_patches gstreamer-rockchip "${src}/s"
+
     # `rkximage` and `kmssrc` are the X11 and KMS *sinks* in the same tree. A headless robot has
     # no use for either, and they are why the prebuilt Radxa deb depends on libx11-6. Dropping
     # them is the concrete thing building ourselves buys, beyond provenance.
@@ -173,6 +203,8 @@ build_webrtc() {
     git clone -q --depth 1 --branch "$GST_PLUGINS_RS_REF" "$GST_PLUGINS_RS_REPO" "${src}/s" \
         || die "cannot clone ${GST_PLUGINS_RS_REPO} at ${GST_PLUGINS_RS_REF}"
 
+    apply_patches gst-plugins-rs "${src}/s"
+
     install -d "$DIST" "$STAGE"
 
     # Two crates, not one: the same stack wants `libgstrswebrtc.so` *and* `libgstrsrtp.so`.