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). Patched; see [`patches/`](patches/). |
+| `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. 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. |
 
 `webrtcbin` is **not** here: it comes from `gstreamer1.0-plugins-bad` in Debian and needs no
 build.
@@ -104,25 +104,8 @@ 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`.
 
-**Both upstreams are patched, and that matters for more than tidiness.** MPL-2.0 asks that
+Nothing here is modified — no patches, no forks. Each release's `MANIFEST` names the repository
-modifications be identifiable, so they are stated here, listed in [`patches/`](patches/) with what
+and the exact ref per plugin, which is both the licence answer and the reason a media bug found on
-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

@@ -1,82 +0,0 @@
-# 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

@@ -1,28 +0,0 @@
---- 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

@@ -1,11 +0,0 @@
---- 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,34 +37,6 @@ 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)"
@@ -140,8 +112,6 @@ 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.
@@ -203,8 +173,6 @@ 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`.