Do not unwrap a frame header on_error may have already taken (aborts on corrupt input)

[nruntas]

Summary

on_error in rav1d_submit_frame unwraps f.frame_hdr, but on_error is
also what clears f.frame_hdr — so reaching it twice on the same frame data
panics. It can be reached twice, and because every entry point into this crate
is extern "C", the panic cannot unwind across the ABI boundary: the process
aborts instead of returning an error the caller can handle.

A single corrupted byte in an otherwise valid AV1 bitstream is enough to take
the host application down. We hit this in a media viewer, on a real .mp4
with one byte flipped inside its mdat.

Root cause

rav1d_submit_frame's single-frame-context branch:

if c.fc.len() == 1 {
    let res = rav1d_decode_frame(c, &fc);   // tears the frame data down on failure
    if res.is_err() {
        ...
        let mut f = fc.data.try_write().unwrap();
        on_error(fc, &mut f, ...);          // second teardown of the same data

rav1d_decode_frame has already cleaned up by the time this runs, so
on_error gets frame data whose header is gone. Instrumenting on_error with
#[track_caller] confirms it, reaching it from that call site with
frame_hdr=false seq_hdr=false.

Only n_fc == 1 is affected, which is presumably why it has survived: with
more than one frame context, decode errors route through the task thread's
retval and this branch is never taken. Since
n_fc = min(max_frame_delay, n_threads), any caller pinning
max_frame_delay = 1 — a natural choice for single-shot, deterministic
decoding — selects the broken path.

The change

Check the option rather than unwrapping it. Behaviour is identical wherever the
header exists (i.e. every path that works today); the only case that changes is
the one that currently aborts. on_error clears f.frame_hdr a few lines
later regardless.

Verification

Built a patched rav1d 1.1.0 and re-ran the case that aborted: 160 systematic
mutations of a real AV1 MP4 (truncations, single-byte flips, and 0xFFFFFFFF
written over header fields), decoded with max_frame_delay = 1. Before: abort
at decode.rs:4997. After: every mutation returns cleanly, either a picture or
an error, and no other assertion in our suite moved.

The same one-line change applies to main, which is what this PR targets.

Note

We also worked around it on our side by raising max_frame_delay to 2, so this
PR is not urgent for us — but the abort is reachable by anyone decoding
single-threaded, and an unwinding panic would at least be catchable where an
abort is not.

[ASCtheone]

Independent corroboration for this, from a downstream that hit it without knowing #1497 existed.

kaleb (Tekcore-Technologie/kaleb), a pure-Rust media stack, wraps rav1d as its AV1 decoder. Writing an AV1 encoder against it, a malformed stream took the whole test process down with SIGABRT. We arrived at exactly the change in this PR — f.frame_hdr.as_ref().is_some_and(|hdr| hdr.refresh_context != 0) — before finding #1496, and we are shipping it as a patch over a vendored 1.1.0 because there is no release carrying it.

The trigger is broader than the dropped-temporal-unit reproducer in #1497. A single byte flipped inside the tile data of an otherwise valid stream is enough — the frame header parses, and the failure happens inside rav1d_decode_frame exactly as the issue describes.

#1497's n_fc == 1 diagnosis is confirmed, and it is n_fc rather than thread count. Same corrupt stream, same build, only the settings changing:

n_threads	max_frame_delay	valid stream	corrupt stream
1	default	frame	SIGABRT
2	1	frame	SIGABRT
4	1	frame	SIGABRT
2	default	no frame	no frame
2	2	no frame	no frame

The configurations that survive are the ones where n_fc > 1 — and they survive only by taking the delayed-output path, so they are not a workaround: a caller that wants a frame back from the packet it just submitted has no setting that avoids the abort.

Effect of this PR, over 600 mutated streams (single- and multi-bit flips plus truncations, across four seeds including a real libaom encode), each decoded in a child process so aborts could be counted:

build	aborted	clean error	decoded a frame	no frame
stock 1.1.0	170	233	161	36
with this PR	0	403	161	36

Every abort became a clean error and nothing else moved — the same 161 streams still decode to a frame, so the change does not make the decoder reject anything it previously accepted.

Worth adding that this is not a robustness nit for a wrapping library: dav1d_send_data is extern "C", so the panic is non-unwinding and no catch_unwind on the caller's side can reach it. rav1d 1.1.0 exports only the C ABI, so there is no Rust entry point to call instead either. For anything decoding untrusted media the only options are patching rav1d or isolating it in a subprocess.

Happy to share the mutation harness if it would be useful.

[nruntas]

Thanks for the report. I can't speak to your numbers, but the n_threads 2 / max_frame_delay 1 row matches what I saw here: it's n_fc that selects the
branch, not the thread count.

One thing worth flagging, since it may change what kaleb has to carry:

a caller that wants a frame back from the packet it just submitted has no
setting that avoids the abort

A one-shot caller does have one, if it drains rather than reading EAGAIN as
"no picture". dav1d_get_picture arms its drain on the first call and flushes
on the next, so at n_fc = 2 a single-picture decode comes back on the retry.
Our AVIF still path is exactly that caller — one OBU payload in, one frame out,
no streaming — and it returns the picture every time. Does your harness retry
the get? The "no frame on a valid stream" cells in your max_frame_delay 2
rows look like it might not; we lost the last frames of every clip to that same
mistake before draining at end-of-packets.

Worth noting too that n_fc = min(max_frame_delay, n_threads), so
max_frame_delay = 2 alone isn't enough — on a single-core machine rav1d's
automatic n_threads puts n_fc back to 1.

If the harness is easy to post publicly, please do — a standalone reproducer
would be a better thing for the issue to point at than mine, which is wired into
a private codebase's format registry.