feat(upload): reject video outside the board's hardware decode envelope - #3309
Open
mickzijdel wants to merge 13 commits into
Open
mickzijdel wants to merge 13 commits into
mickzijdel wants to merge 13 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3309 +/- ##
=========================================
Coverage ? 90.78%
=========================================
Files ? 86
Lines ? 10379
Branches ? 1187
=========================================
Hits ? 9423
Misses ? 703
Partials ? 253 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mickzijdel
force-pushed
the
feat/decode-envelope-gate
branch
4 times, most recently
from
September 3, 2026 17:55
1ff9176 to
b13f621
Compare
Contributor
|
Ran this on the Screenly testbed — all seven boards (pi2, pi3, pi3-64, pi4-64, pi5, rockpi4, x86), against Your extrapolations hold
Pi 5 has no H.264 decoder node at all — the only coded device is The gap: the frame bound is necessary but not sufficientThe driver enumerates 1920x1920, but
The gate's per-axis-only rule therefore accepts 1920x1200, 1920x1440, 1600x1600 and 1920x1920 — all hardware-refused, all silently falling back to software, which is the exact failure this PR exists to prevent. Confirmed end to end, not just at the driver. Uploading through a patched build on a Pi 4 with
On the rig's actual 1 GB Pi 4 the low-RAM 1080p cap masks all of these, so the gap is only reachable on 2/4/8 GB Pi 4s — precisely the board in the original bug report. Software cost on the Pi 4 for the two most realistic ones (real High profile, CABAC, B-frames): 1920x1200 runs 51.7 fps on four idle cores and 21.4 fps single-threaded; 1920x1920 runs 36.7 and 14.6. Single-thread is the figure your own module docstring argues a loaded viewer approaches. One knock-on if you add the predicate: The Pi 5 advisory is measurably too tightDecode-only fps on the Pi 5, real High-profile content:
The advisory fires on 2560x1440, which holds 59 fps under heavy contention — 2.4x realtime. It stays silent on 1920x1920, which also holds 59 fps. Only 4K is genuinely marginal. So the docstring's worry that a per-axis rule lets 1920x1920 slip through is unfounded — that size is fine — and "unlikely to keep up at this size" is wrong at 1440p, the commonest over-1080p signage size. If the tier stays, the honest boundary is nearer 4K than 1920 per axis. Annotation-only, so low stakes. Everything else checked out on deviceThe three must-not-flag cases all pass on real hardware: 1920x1080 tagged L5.1, 1080x1920 portrait, and plain 1080p. High 10 is correctly rejected on the colour format, and 2560x720 on the frame. The ultrawide remedy is the nicest thing in here — 2560x720 is 1.84 Mpx, under the low-RAM cap, and
The per-row board-lookup fix holds on the one board class where the lookup actually reaches Redis — rockpi4, 47 assets: 171.5 ms median on master vs 177.9 ms on this branch, within noise. Memoisation confirmed live, and Merge conflicts need addressingThe branch is currently Both imports are wanted, so the resolution is to keep both. Everything else ( SummaryThe design is sound and the evidence standard in this PR is high — the removed bitrate rule, the refusal to gate on the declared level, and the fail-open-at-the-leaves discipline all survived contact with the hardware. Before merge I would want the macroblock predicate next to One caveat on my numbers: the benchmark clips are synthetic (mandelbrot plus noise, High profile with B-frames and CABAC). The hardware accept/refuse results are exact and content-independent; treat the absolute software fps as optimistic, since real content is more expensive again. |
The codec gate answers "can this board hardware-decode this codec",
and stops there. It has no opinion on the stream itself, so a
3840x2160 H.264 clip uploaded to a Pi 4 passes cleanly: H.264 is in
pi4-64's supported set, the row lands in rotation, and the screen
plays it at roughly 4 fps. Nothing in the pipeline says a word, so the
first anyone hears about it is the screen.
Measured on a Pi 4B, a 4K High L5.1 clip at 116 Mbps against a 1080p
re-encode of the same content:
4K 1080p
hardware decode REFUSED 75 fps
software, 4 cores 10 fps 37 fps
software, 1 thread 3 fps 12 fps
The 4K clip does not decode slowly in hardware, it never gets in.
h264_v4l2m2m fails outright because 3840 is past the device's frame
bound, so libavcodec drops to software, where 10 fps on an idle
four-core box is already under the 25 fps the clip needs — before the
viewer spends anything on presentation.
This adds the missing check as a standalone module, with no callers
yet. Two tiers, because the evidence differs in kind. BLOCKING covers
driver-enforced facts: bcm2835-v4l2-codec.c pins MAX_W_CODEC and
MAX_H_CODEC at 1920 and restricts the capture queue to 8-bit 4:2:0, so
an oversized or High 10 stream is refused by VIDIOC_S_FMT rather than
merely being slow. Confirmed by asking the hardware — v4l2-ctl reports
"Stepwise 32x32 - 1920x1920 with step 2/2" and YU12/YV12/NV12/NV21/
NC12. Every board decoding H.264 through that device inherits it: pi2
and pi3 via GStreamer, pi3-64 via the kmssink overlay, pi4-64 via
QtMultimedia. ADVISORY covers judgement calls — currently only
software H.264 above 1080p on a Pi 5, which has no H.264 block at all.
Deliberately NOT gating on the declared H.264 level, which is the
obvious-looking rule and the wrong one. The driver exposes
V4L2_CID_MPEG_VIDEO_H264_LEVEL read-only and never validates the
bitstream against it. Plenty of 1080p files carry an inflated level
tag and play perfectly, so the level is a symptom of an oversized
stream, never the cause. Recorded for diagnostics; not branched on.
Deliberately no bitrate rule either. One was drafted at the Level 4.2
ceiling and measurement killed it: 1080p25 through h264_v4l2m2m runs
93 fps at ~9 Mbps and still 62 fps at ~137 Mbps, so it would only have
flagged files that play fine.
Every predicate fails open. An unmeasured dimension, an unparseable
pixel format or an uncharacterised board yields no warning, and the
pixel-format check is a denylist of formats known to be unsupported
rather than an allowlist of the ones we thought of. A false positive
costs an operator a working asset and teaches them to ignore the
badge; a false negative leaves them where they are today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
The decode envelope needs a field the probe summary was throwing away. video_pix_fmt drives the blocking 8-bit-4:2:0 check, since the VideoCore capture queue has no 10-bit or 4:2:2 fourcc and a High 10 source therefore falls to software decode. video_bit_rate, video_level and video_profile are recorded too, and nothing branches on any of them. They are here so an operator looking at a misbehaving asset can see what they actually uploaded, and so the next person who wonders whether the level is the problem can find the answer in the metadata instead of guessing. Bitrate reads the video stream's own figure and falls back to the container's, because Matroska and some MP4 muxers omit the per-stream value entirely. ffprobe writes -99 for a container carrying no level, so non-positive values normalise to None rather than surviving as a real reading. Every new field collapses to None when ffprobe cannot supply it, including on the probe-failure path, which the envelope reads as "not measured" and stays quiet about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
A 3840x2160 H.264 upload to a Pi 4 passed every gate we had. H.264 is in pi4-64's supported codec set, and the 1080p resolution cap is guarded by is_low_ram_device(), which is false on any Pi 4 with 2 GB or more — measured at 3766308 kB on the device that hit this. So the row went is_processing=False, joined the rotation, and played at a few frames per second. The first report of the problem came from someone looking at the screen. The envelope check now runs after the codec gate on the accepted path, and only its BLOCKING tier rejects: frames over 1920 on either axis and pixel formats outside 8-bit 4:2:0, both refusals by the driver rather than predictions about speed. The advisory tier is deliberately not consulted here; it annotates the asset list without ever stopping an upload. Existing assets are untouched. This runs during normalisation, so it only ever sees a new upload; rows already on disk keep playing exactly as they do today. The rejection reuses the codec gate's UnsupportedVideoCodecError, so the operator gets the UI they already know: the reason inline, a copy-pasteable ffmpeg recipe, and HandBrake steps for anyone who would rather not open a terminal. Two details in the recipe matter. The downscale clause is emitted only when the frame is what failed, so a 10-bit 1080p file is not told to resize for no reason. And that case gets -pix_fmt yuv420p, because libx264 preserves the source bit depth by default — without it the operator would follow the recipe exactly and produce a second file that fails the same gate. Verified end to end against real encodes rather than fixtures: a 3840x2160 High L5.1 file is rejected, running the emitted recipe verbatim produces a 1920x1080 L4.0 file, and that output passes the gate. A 1920x1080 file carrying an inflated level=51 tag passes untouched, which is the false positive this must never produce. The module was also run on the production Pi itself, where resolve_device_key() returns 'pi4-64' — the key the rules are written against, and a mismatch there would have made every rule silently match nothing while the tests still passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
…code The upload gate only ever sees new uploads, so every asset that landed before it existed keeps playing exactly as it did — including the 4K clip that started this. Deleting or disabling those rows behind the operator's back would be worse than the problem; what they need is to be told which asset is making the screen look wrong. The asset list now carries a chip next to the name for any video outside this board's decode envelope, and the edit modal spells out every finding with its fix. Blocking findings read "Will not play well" and advisory ones "May not play well", which is the honest distinction: one is a format the decoder refuses, the other is a judgement about speed. Warnings are computed server-side and travel in the asset payload _to_dict already builds for the modal, so the per-board rules stay in one module instead of being reimplemented in Alpine. Two things the browser caught that the markup did not. The chip sits in a flex column, which blockifies inline-flex and then stretches it to the full column width — the pill ran the whole width of the name cell, fixed with align-self: flex-start. And the advisory variant was drawn as an outline with no fill, which made its label unreadable: --color-warning-on-wash is contrast-matched to the wash, not to the page, and the asset list resolves the light amber tokens over a dark surface. The softer tier is now signalled with a dashed edge and lighter weight, and both variants keep the wash the text needs to be legible against. Measured after that fix at 6.37 contrast in light and 12.84 in dark. Verified in a browser against seeded rows covering four cases: the 4K clip shows a blocking chip, a 115 Mbps 1080p clip an advisory one, and both a 1080p file tagged level 5.1 and a portrait 1080x1920 file show nothing at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
board-enablement.md gains the second half of what enabling a board means. Codec support alone never settled playback, and the doc only described the codec set, so anyone adding a board had no reason to think about frame size or pixel format. It now carries the two tiers, which boards are in each and why x86 and rockpi4 are deliberately in neither, and the v4l2-ctl probe to characterise a new one — ask the decoder for its bounds rather than timing a clip, because an out-of-range frame cannot be set at all, so there is nothing to time. Corrects an actively misleading line in the anthias-hardware skill. It listed "level>4.x" alongside 4K30 / High10 / 4:2:2 as things that stop a Pi 4 opening /dev/video10. The driver never looks at the level; the real bound is 1920 per axis plus an 8-bit-4:2:0 capture queue, and the level control is read-only. That distinction is the whole reason the gate keys on frame size, so it is corrected in place with the mechanism and flagged as a previous error rather than quietly edited. Records what the hardware actually said, so the next person does not have to rediscover it: the enumerated frame bounds and capture formats, the 4K-vs-1080p decode comparison, the bitrate sweep that retired a drafted rule, and the fact that /dev/video19 (rpi-hevc-dec) is bound by default with no dtoverlay=rpivid-v4l2 anywhere in config.txt — so the gate's HEVC entry for pi4-64 is correct and the older docs demanding that overlay are out of date. The Pi 4 HEVC node is also stateless and advertises 10-bit capture formats, which is why the 8-bit restriction is scoped to the H.264 path and not applied board-wide. Two traps worth not repeating are written down. Synthetic benchmark clips (testsrc2 + noise, ultrafast, no B-frames) software-decode roughly 5x faster than real High-profile content, so extrapolating 4K software decode from a synthetic 1080p figure is wrong by about 2x. And fuser /dev/video10 run from the host reports no holders while the viewer is actively decoding, because it cannot see across the container's namespace — scan /proc/[0-9]*/fd instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
Codecov reported 89.34% patch coverage, 13 lines short. Every one of them was a real branch rather than a coverage-metric artefact, so they are now tested rather than waived. The two that matter most are ffprobe parse failures. ffprobe writes the literal string "N/A" instead of omitting the key on plenty of containers, and _ffprobe_summary runs for every video upload — so an unhandled ValueError there would fail the whole normalisation task over a diagnostic field that nothing branches on. Both bit_rate and level now have tests proving they collapse to None, that a stream-level "N/A" still falls back to the container figure, and that one bad field does not poison the rest of the summary. The rest are helper guards in playback_envelope, which is now at 100%: _as_positive_int against unparseable and bool inputs (bools are ints in Python, and treating True as 1 would invent a dimension out of a flag), _dimensions_label's unmeasured-dimension fallback, and PlaybackWarning's __eq__/__repr__ — the first is what lets tests compare findings by value and defer sanely on foreign types, the second is what pytest prints when one fails. Also covers the HEVC branch of _ffmpeg_reencode_recipe. My pix_fmt change landed inside a branch that no test reached, because no board ships an HEVC-only codec set any more (Pi 5 gained an H.264 software fallback). It stays reachable-in-principle because those sets are per-board data that can change, and a recipe silently emitting libx264 for an HEVC-only board would hand the operator a file that fails the same gate again. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
evaluate() resolved the board before it looked at the codec, and the
asset list calls it for every row -- once through the playback_warnings
filter and again through each to_json in _asset_row.html. On the
catch-all arm64 image resolve_device_key() reaches for the host_agent's
published subtype, which is a fresh Redis client per call plus a
/proc/device-tree/model read when Redis has nothing, so the table paid
3 Redis round-trips and 3 device-tree reads per row. Images paid it
too: the early return only covered empty metadata, and an image row
carries upload_name.
Measured in the test container, median of 5 runs after warm-up, 40 rows:
x86 arm64
master 11.2 ms 11.2 ms
before 11.5 ms 246.8 ms
after 11.5 ms 10.8 ms
That is per render of the asset table, and _asset_table.html re-renders
the whole thing on an "every 5s" hx-trigger. pi4-64 / pi5 / x86 were
never affected -- DEVICE_TYPE is a plain env read there -- so this is
the arm64 / rockpi4 fleet, which is also the 1 GB one.
Two changes. The codec is now checked first, so every image, web page
and non-H.264 clip settles on a dict lookup and never reaches for the
board at all. And the lookup itself is memoised, keyed on the raw
DEVICE_TYPE so a process that sees the env var change still gets its
own answer rather than a neighbour's.
The cache expires rather than living forever, which is the part worth
keeping. A compose install can publish host:board_subtype seconds after
the server starts; a permanent cache would pin the board to the
un-upgraded arm64 key for the life of the process, and every rule in
this module would then silently match nothing -- the same failure the
module docstring warns about, arrived at from the other direction.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The envelope rejection reused the low-RAM gate's cap_to_1080p clause,
so a portrait rejection told the operator one thing and handed them a
command that did another. The message says "1080x1920 for portrait" --
correct, because MAX_W_CODEC / MAX_H_CODEC bound each axis rather than
the pixel count -- while the recipe carried
scale=1920:1080:force_original_aspect_ratio=decrease. Run verbatim
against the 2160x3840 clip it had just rejected, that produced:
608x1080
which is two thirds of the frame thrown away, in the one artefact the
operator copies without reading. Verified against a real encode, not a
fixture. cap_to_envelope scales onto a 1920x1920 box instead, and the
same source now comes back 1080x1920; a 3840x2160 source still comes
back 1920x1080. The two flags stay separate because the two gates
answer different questions -- the low-RAM cap really is a pixel budget
and squeezing a portrait clip into the landscape box is the point
there. They cannot collide in practice either: the low-RAM branch
returns before the envelope check ever runs.
Both clauses also gain force_divisible_by=2. force_original_aspect_ratio
rounds to wherever the aspect ratio lands, and an odd result is fatal
rather than cosmetic -- a 2100x1900 source onto the 1920 box computes
1920x1737 and libx264 refuses the job outright ("height not divisible
by 2"), so the operator pastes the command we gave them and gets an
error instead of a file. It is reachable from the 1080p box too
(3000x1001 lands on 1920x641), so the guard goes on both rather than
only on the new one.
All four rejection paths were re-run end to end on real encodes and
their outputs fed back through the gate: landscape 4K, portrait 4K,
High 10, and the odd-dimension case all now produce a file that passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six review passes on the gate produced one recurring defect in several disguises: the advice we print and the command we print underneath it were derived separately, so they disagreed. Three times. This is the corrected form, folded into one commit because every intermediate state was wrong in the same way. **The codec and the box are one decision.** They depend on each other: H.264 is bounded at 1920 on a VideoCore board while HEVC is not, and once a frame is being downscaled anyway the bound stops mattering. So deriving either alone produced "throw away three quarters of your 4K master" on a Pi 4 whose HEVC block plays it, and the mirror of that on the low-RAM path. `_recipe_plan` returns both, and the remedy sentence, the ffmpeg recipe and the HandBrake steps all read from it. Checked across 1008 board and shape combinations: no disagreements. **Rotation is a display fact, not a coded one.** ffprobe reports the coded frame, and a phone-shot vertical clip is coded 3840x2160 with a 90-degree matrix that ffmpeg applies before any filter we emit. Orientation now uses the display shape, so a portrait master no longer comes out 608x1080. The blocking bound still uses the coded numbers, because that is what the decoder acts on. **The remedy has to survive the operator's machine.** A filtergraph of shell metacharacters needs quoting or it dies on paste; `force_divisible_by` needs FFmpeg 4.4 and Ubuntu 20.04 ships 4.2; `decrease` lands on odd dimensions libx264 refuses; and `shlex.quote` leaves a leading dash bare, which ffmpeg reads as an option. Each was a recipe the operator could not run. **And it has to clear our own gate on re-upload.** Every rejection path now carries the frame size and the pixel format, including the low-RAM one left behind while its three siblings were fixed: a 1 GB Pi refusing a 4K High 10 upload emitted a recipe producing a 1080p 10-bit file, refused again for a reason we never mentioned. Fail-open is enforced at the leaves rather than assumed. `OverflowError` on every metadata conversion, non-dict metadata, and a partially measured stream all yield silence instead of an exception out of the filter that renders every asset row. `allow_unplayable_video` is the escape hatch, off by default. This is the first gate here that refuses H.264, and its bound is measured on a Pi 4 and extrapolated to pi2, pi3 and pi3-64 — so if it is wrong on one of those, an operator on an appliance they cannot patch would have no way out but a re-flash. Scoped to this tier: the codec gate still refuses formats with no decoder at all, and the low-RAM cap still refuses frames that OOM-loop the device rather than merely look bad. The board memoisation's clearing fixture ships here rather than later, because it is load-bearing: disable it and an existing test pair fails deterministically. It sits in the root conftest, gated on `_APP_AVAILABLE` like `_mock_redis`, so the app-free imager suite still collects in CI's lean venv. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
… words **A refused upload wore two amber pills.** Three comments asserted that could not happen; all three were wrong. The gate writes its probe metadata before it raises and `on_failure` adds the error to that same surviving row, so the commonest path in the feature — upload a 4K file to a Pi 4 — rendered "Will not play well" beside "Failed", and a modal with two banners printing the same sentence under headings that contradicted each other, one claiming the asset was still playing when it never got in. The chip and the envelope banner now yield to `error_message`; the failure pill already carries the reason, the recipe and the HandBrake steps. An asset let through by the override reads differently again: "Check the screen" rather than "Will not play well", because once the operator has overruled us the useful instruction is to go and look, not to repeat the verdict they dismissed. The settings row says the same thing, with a badge that appears the moment the switch flips. The copy throughout says what happens rather than how it works. "Decoded in software" is not something an operator can act on, and "a few frames per second" was only ever measured at 4K while the block starts at 1921 pixels, where a 2048x858 master plays far better. "This board" became "this screen", "pixel format" became "colour format", and the board name stays because it is what makes a support ticket answerable. No em dashes, per house style. Coverage for the surfaces that had none: the templates are rendered rather than only their filters called, on more than one board, across the blocking, advisory, overridden and clean states. The modal's guard is Alpine, which no Django render can evaluate, so that test is structural and says so rather than pretending otherwise. The chip and `.error-pill` both cleared their focus outline while signalling focus only with a background shift, which is the change hover already makes. Both now use the house focus ring. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HB1hhpAJcgtFueWqnC1z2K
The gate trusted `v4l2-ctl --list-framesizes`, which reports
`Stepwise 32x32 - 1920x1920`, and treated that as the whole envelope.
It is not. The VideoCore block is a Level 4.1 decoder and separately
refuses any frame over that level's MaxFS of 8192 macroblocks, so
1920x1200 clears the enumerated range and is still refused.
Measured on the Screenly testbed, pi2 / pi3 / pi3-64 / pi4-64 all
agreeing, driver 6.18.34:
1920x1080 8160 mb accepted 1472x1440 8280 mb refused
1456x1440 8190 mb accepted 1920x1200 9000 mb refused
1440x1440 8100 mb accepted 1920x1920 14400 mb refused
1456x1440 accepted against 1472x1440 refused is the pair that settles
it: same height, 16 pixels of width apart, one either side of 8192.
So it is not a height bound, and not the declared level either, since
the accepted 1920x1080 and the refused 1920x1200 clip both carried
`-level 5.1`. Neither bound subsumes the other: 2560x720 is 7200
macroblocks and refused on width.
Until now the gate accepted 1920x1200, 1920x1920 and 1600x1600, all
hardware-refused, all falling silently to software decode, which is
the exact failure this gate exists to prevent. It is only reachable on
2/4/8 GB Pi 4s, because the low-RAM 1080p cap masks it on 1 GB units.
The recipe had the same hole, twice over. `_recipe_plan` asked only
about the axes, so a 1920x1200 source planned "re-encode to H.264 at
the same size" and handed back a file the gate rejects again. And the
box it proposed when it did resize was square, (1920, 1920), which is
itself 14400 macroblocks, so the HandBrake step told the operator to
enter dimensions the decoder will not take. Both now route through
`frame_exceeds_envelope` and `encode_box_for`, which consult both
bounds, and the box is 1920x1080 turned to match the source.
That turn introduces a distinction the square box hid: acceptance is
asked of the *coded* frame, because that is what the decoder sees,
while the box applies to the *displayed* frame, because ffmpeg applies
the rotation matrix before user filters. A coded 2160x3840 carrying a
90-degree rotation reaches `scale` as 3840x2160 and needs the
landscape box.
The rejection copy names both numbers now. "Neither side is larger
than 1920" was two kinds of wrong at once once the box stopped being
square: against a 1920x1200 source it describes something the operator
can see is already true, and against the portrait box it is advice
that permits 1920x1920, which is refused.
Verified beyond the unit tests: every rejection driven through the
real gate, its recipe executed through /bin/sh with real ffmpeg, and
the resulting file fed back through the gate. Eleven cases, all clean,
including the portrait one that now yields 1080x1920 rather than
608x1080. A separate sweep of 4620 board/size/rotation/codec
combinations finds no disagreement between the message, the ffmpeg
command and the HandBrake steps, and reintroducing either defect makes
that sweep fail.
Reported-by: Cyborg Viktor <307551610+vpetersson-bot@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The advisory fired above 1920 on either axis, which was a guess, and
the module said so: "nobody has measured where the A76 actually falls
over". Someone has now. Decode-only fps, real High-profile content,
measured on the testbed at three levels of CPU contention:
frame idle 3 cores busy 1 thread
1920x1080 2.07 Mpx 162.2 68.7 56.3
2560x1440 3.69 Mpx 93.4 59.1 33.4
1920x1920 3.69 Mpx 77.6 59.2 34.5
3840x2160 8.29 Mpx 43.9 18.9 16.0
The middle column is the honest one: a loaded viewer shares its cores
with QtWebEngine and the scene graph.
The per-axis rule was wrong in both directions at once. It fired on
2560x1440, which holds 59 fps under heavy contention, 2.4x realtime
for 25 fps content, and it stayed silent on 1920x1920, which has
exactly the same pixel count. Software decode costs pixels, and an
axis bound cannot express that. So this is an area budget, and
`frame_bound_for` stops reporting a per-axis number for software
boards entirely rather than inviting the same category error from the
recipe.
The budget is four times 1080p, which 3840x2160 meets exactly, hence
`>=` rather than `>`. That is the single measured failure and nothing
else. The band between 3.7 and 8.3 Mpx is unmeasured, and this module
does not invent thresholds. A power law through the two contended
figures puts the real 30 fps crossover nearer 6 Mpx, so the budget is
probably generous, but correcting it wants a reading inside that band
rather than a curve through its edges. The comment says where to take
one.
Still advisory, never blocking: nothing on a Pi 5 refuses an H.264
frame, so no size may cost an operator the upload. In practice the
box is unreachable anyway, because the plan prefers HEVC there and the
BCM2712 block decodes that at 4K in hardware, so a 4K master keeps its
frame.
Reported-by: Cyborg Viktor <307551610+vpetersson-bot@users.noreply.github.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both files told the next person to characterise a board by asking its decoder rather than timing a clip, and the skill went further: "better evidence than any playback measurement". That advice produced the bug the previous two commits fix. The enumeration is necessary and not sufficient, and nothing said so. So the method is now two steps, with the second one marked as the part not to skip: enumerate the bounds, then actually try to decode at the sizes you care about. A capability a device advertises is a bound on what can be asked for, not a promise about what will be accepted. Includes the measured accept/refuse table, the ffmpeg commands to reproduce it, and the advice to bracket a suspected limit with two frames 16 pixels apart rather than sampling one size, which is what made 1456x1440 against 1472x1440 conclusive. Also records the Pi 5 software figures and corrects the skill's claim that the H.264 advisory applies "above 1080p", which measurement puts nearer 4K. Reported-by: Cyborg Viktor <307551610+vpetersson-bot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mickzijdel
force-pushed
the
feat/decode-envelope-gate
branch
from
September 12, 2026 09:26
b13f621 to
b665007
Compare
|
Contributor
|
Heads up: this PR currently has merge conflicts with Flagged by an automated PR-hygiene sweep — no action needed beyond the rebase, and no reply expected. |
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
2 participants
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.
Issues Fixed
A 4K H.264 clip uploaded to a Pi 4 was accepted with no warning and then played
at very low fps on the screen. H.264 is in
pi4-64's supported codec set, andthe existing 1080p resolution cap only applies to boards under 1.5 GiB of RAM,
so nothing in the pipeline had an opinion about it. The first sign of trouble
was the screen itself.
Description
The codec gate answers "can this board decode this codec". It says nothing
about the stream, so this adds a second check for whether the frame is inside
what the decoder can take.
Measured on a Pi 4B, a 4K High L5.1 clip at 116 Mbps against a 1080p re-encode
of the same content:
4K does not decode slowly in hardware, it never gets in:
h264_v4l2m2mfailsoutright because 3840 is past the device's frame bound, so libavcodec drops to
software, where 10 fps is already under the 25 fps the clip needs before the
viewer spends anything on presentation.
Two tiers, split by how good the evidence is:
the existing ffmpeg recipe and HandBrake steps. Covers frames over 1920 on
either axis, frames over 8192 macroblocks, and pixel formats outside 8-bit
4:2:0, all confirmed against real silicon.
software H.264 at 4K and above on a Pi 5.
There are two frame bounds and neither implies the other.
v4l2-ctlreports
Stepwise 32x32 - 1920x1920, and I first took that as the wholeenvelope. It is not: the block is a Level 4.1 decoder and separately refuses
anything over that level's MaxFS of 8192 macroblocks, so 1920x1200 clears the
enumerated range and is still refused. Measured on the testbed, pi2 / pi3 /
pi3-64 / pi4-64 all agreeing:
1456x1440 against 1472x1440 is the decisive pair: same height, 16 pixels of
width apart, one either side of 8192. Going the other way, 2560x720 is only
7200 macroblocks and is still refused on width, so both rules earn their keep.
A useful side effect: the accepted 1920x1080 and the refused 1920x1200 clip
both carried
-level 5.1, which is independent confirmation that not gating onthe declared level was right.
Existing assets are never touched. The gate only runs during normalisation, so
anything already on disk keeps playing and gets a chip in the asset list
instead, with the full explanation in the edit modal.
Not gating on the declared H.264 level, which is the obvious-looking rule
and the wrong one. The driver exposes
V4L2_CID_MPEG_VIDEO_H264_LEVELread-only and never validates the bitstream against it, and plenty of ordinary
1080p files carry an inflated level tag and play fine. A 1080p file tagged
level 5.1 is a test case here precisely because it must not be flagged.
A bitrate rule was drafted and then removed after measuring it: 1080p through
h264_v4l2m2mstill runs at 62 fps at ~137 Mbps, so it would only ever haveflagged files that play fine.
Known gaps
Most of what was listed here has now been measured on the Screenly testbed
across all seven boards, and the results are in the thread below. What that
run changed:
pi2/pi3/pi3-64bound is no longer extrapolated — all fourboards report the same
bcm2835-codeclimits on driver 6.18.34, andresolve_device_key()returns the expected key on every board, so the"every rule silently matches nothing" failure does not happen anywhere.
accepted 1920x1200, 1920x1920 and 1600x1600, all hardware-refused, all
falling silently to software. Only reachable on 2/4/8 GB Pi 4s, since the
low-RAM cap masks it on 1 GB units.
59 fps under heavy CPU contention, and so does 1920x1920; only 4K is
marginal at 18.9. It is an area budget now rather than a per-axis one,
because the old rule managed to be wrong in both directions at once.
Still open:
x86androckpi4are deliberately in neither tier; their decode paths arenot characterised, and a guess would produce false rejections.
through the two contended figures suggests the real crossover is nearer
6 Mpx than the 8.3 the budget uses, so it is probably generous. Correcting
it wants a reading inside that band rather than a curve through its edges;
the comment in the module says so and names the size to try.
A simpler shape, if you want one
Worth saying explicitly, because it is the obvious lever and I would
rather offer it than have it asked for: this could be warn-only, and
that would remove most of the complexity.
Blocking is what creates the recipe. Once an upload is refused, we owe
the operator a way out, and that remedy is its own product surface: it
has to survive their ffmpeg version, their shell, their file's
dimensions, and then pass our own gate on re-upload. Nearly every
defect found in review has been in the recipe, not in the gate. The
gate itself has been correct since the first hardware measurement and
is roughly forty lines.
Dropping the blocking tier would mean the chip and the modal do all
the work, the
_ffmpeg_reencode_recipechanges here mostly disappear,and the per-board decode facts, which are the part that took the
measuring, stay exactly as they are. The cost is that an operator who
ignores the badge still ships a few frames per second to a screen,
which is the failure that started this.
Happy either way, and happy to cut it back if you would rather take
the smaller change first and add blocking later.
Checklist
On the Pi testing: run on a live Pi 4B — the module against real asset
metadata,
resolve_device_key()returningpi4-64, thev4l2-ctlcapabilityprobes, and the decode measurements above. The testbed run in the thread below
covers what I could not: full uploads through a patched build on a 4 GB Pi 4,
including the override path, and the rendered asset table with no doubled
badges.
On x86: no rules fire there by design. Verified in a dev container where
DEVICE_TYPE=x86produces no warnings, and confirmed on the testbed's realx86 board, where
resolve_device_key()returnsx86and no rule matches.Behaviour is unchanged.
Every rejection in this PR has also been driven end to end: the real gate
refuses the file, its recipe is executed through
/bin/shwith real ffmpeg,and the resulting file is fed back through the gate. Eleven cases, all clean.
A separate sweep of 4620 board/size/rotation/codec combinations checks that
the message, the ffmpeg command and the HandBrake steps never disagree.
🤖 Generated with Claude Code