Repository navigation
av1: decode frames smaller than the sequence maximum at their real size - #460
Conversation
|
Thanks for this PR. After a review I propose two small changes you might consider merging: I tested a change on top of this PR that fixes a use-after-free risk in Commit: donbernhardo/nvidia-vaapi-driver@b65b70d Feel free to cherry-pick it: git cherry-pick b65b70dAnother patch fixes an inconsistent CUVID picture parameter block. The AV1-specific width and height now describe the current frame, but Commit: donbernhardo/nvidia-vaapi-driver@4e8bf7a Feel free to cherry-pick it independently of the first commit: git cherry-pick 4e8bf7a |
FFmpeg can destroy a VA decode context during a resolution switch while previously decoded frames are still awaiting transfer. The resolve thread previously exited as soon as teardown was requested, discarding queued pictures and leaving those surfaces without backing images. A later vaGetImage then dereferenced a null backing image and crashed. Wake the resolver under its mutex, let it process queued surfaces before exiting, and return a decoding error if a surface still has no backing image. The combined PR elFarto#456/elFarto#460 branch now passes the dynamic-resolution and teardown suite, including ten repeated quick runs.
PR elFarto#460 passes the current AV1 frame width and height to NVDEC, but the generic PicWidthInMbs and FrameHeightInMbs fields still describe the VA context maximum. CUVID documents those fields as coded frame dimensions, and FFmpeg derives them from its current AV1 frame. Calculate both macroblock counts from the same per-frame width and height used by CUVIDAV1PICPARAMS. This keeps the parameter block internally consistent for frame_size_override_flag streams. On an RTX 3070, two generated resize streams decoded identically before and after this change; this aligns the API parameters but does not demonstrate a visible output fix on that GPU.
3e131cf to
77618e4
Compare
|
Thanks for reviewing this so carefully — both points were worth making, and the branch is now rebased onto current master with them addressed.
Re-verified after the rebase, decoding through the driver with
The driver log shows a single |
PR elFarto#460 passes the current AV1 frame width and height to NVDEC, but the generic PicWidthInMbs and FrameHeightInMbs fields still describe the VA context maximum. CUVID documents those fields as coded frame dimensions, and FFmpeg derives them from its current AV1 frame. Calculate both macroblock counts from the same per-frame width and height used by CUVIDAV1PICPARAMS. This keeps the parameter block internally consistent for frame_size_override_flag streams. On an RTX 3070, two generated resize streams decoded identically before and after this change; this aligns the API parameters but does not demonstrate a visible output fix on that GPU.
AV1 lets a stream declare a maximum frame size in its sequence header and then code individual frames smaller than that (frame_size_override_flag). Hardware encoders do this routinely: NVENC, for example, declares a 1920x1088 maximum for 1080p content (and 3840x2176 for 2160p) because it works in 16-pixel-aligned blocks, but codes every frame at 1920x1080. Players create the VA context and surfaces at the maximum size, which is correct, but the driver then assumed every frame was that size too. Two things went wrong because of that: * copyAV1PicParam told NVDEC the frame was the context size, and gave every reference frame the size of its surface, instead of the size the frame and its references were actually coded at. The first frames after a keyframe still decode correctly, but prediction is done against the wrong frame geometry, so small errors appear wherever there is motion and keep compounding until the next keyframe. On screen this looks like a picture that starts out fine and turns into an ever-growing smear of broken blocks the more things move. It is very visible in Chromium WebRTC calls and screen shares that use a hardware AV1 encoder. * Once the right sizes are passed, NVDEC does not crop the smaller frame out of the surface-sized decoder output: it stretches it to fill the display area the decoder was created with, so a 1080-line frame would come out scaled to 1088 lines. The fix: * Pass the real coded frame size for the current frame, remember that size on the surface it was decoded into, and pass each reference frame's remembered size rather than its surface size. With this the picture parameters match what ffmpeg's own NVDEC AV1 hwaccel sends for the same stream. * Keep the decoder's display area in step with the frame size using cuvidReconfigureDecoder, placing the frame unscaled at the top-left of the unchanged surface-sized target, so the existing copy into the surface does not need to change. NVDEC refuses a reconfigure before the decoder has decoded anything, so the first one is applied right after the first picture is decoded and before it is handed to the resolve thread. If the size changes again later, frames that are already queued for output are allowed to finish first, since the display area is applied when a frame is mapped. Streams whose frames are all coded at the maximum size never request a display area change, so they take exactly the same path as before. Tested on an RTX GPU with driver 610.57.04 by decoding through this driver with ffmpeg (-hwaccel vaapi) and comparing frame by frame against libdav1d: * 1920x1080 frames in a 1920x1088 sequence: before, 59 of 150 frames bit-exact (every frame from the 31st after a keyframe was corrupt); after, 150 of 150. * 3840x2160 frames in a 3840x2176 sequence: before, 30 of 90; after, 90 of 90. * An AV1 stream whose frames match the sequence maximum (ffmpeg av1_nvenc): 150 of 150 both before and after. * H.264 decode output is byte-identical before and after. NVIDIA's Vulkan Video AV1 decoder on the same GPU also decodes the affected streams bit-exactly, which is what pointed at the driver rather than the hardware. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JyWW73TaBFHr9CoqHoWqdx
PR elFarto#460 passes the current AV1 frame width and height to NVDEC, but the generic PicWidthInMbs and FrameHeightInMbs fields still describe the VA context maximum. CUVID documents those fields as coded frame dimensions, and FFmpeg derives them from its current AV1 frame. Calculate both macroblock counts from the same per-frame width and height used by CUVIDAV1PICPARAMS. This keeps the parameter block internally consistent for frame_size_override_flag streams. On an RTX 3070, two generated resize streams decoded identically before and after this change; this aligns the API parameters but does not demonstrate a visible output fix on that GPU.
77618e4 to
380cea7
Compare
The frame_size_override fix had no automated coverage: the repro lived in a PR description, so nothing would notice the corruption coming back. Decode a stream whose frames are smaller than the sequence maximum through the driver and through libdav1d and require them to agree. A correct AV1 hardware decode is bit-exact with a software one, and the broken driver scores about 13 dB on this fixture. A second stream whose frames match the maximum guards the ordinary path, and keeps the test honest: on the unpatched driver that one still passes, so a failure points at the override handling rather than at decode in general. The fixture is generated rather than checked in, like the other tests here, using SVT-AV1's forced-max-frame-width/height — the only widely available encoder knob that emits this layout. The test verifies the generated stream really carries frame_size_override_flag with a maximum larger than the coded frames, and skips itself if the encoder ignored the request or if ffmpeg lacks libsvtav1 or libdav1d, so it can never pass while silently covering nothing. Verified against the unpatched decoder: FAIL (13.10 dB) on the override stream, PASS on the control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JyWW73TaBFHr9CoqHoWqdx
|
Added a regression test, since the repro for this only existed in the PR description.
Against the unpatched decoder the two cases separate cleanly, which is what makes the test worth having: With the fix both are bit-exact. A few notes on how it is built, in case you would rather it looked different:
|
|
Thanks for the patch! |
Brings in upstream elFarto#469 (vp8: reconstruct frame headers from VA-API parameters) and elFarto#460 (AV1 frames smaller than the sequence maximum). Conflict resolution: - src/vp8.c: take upstream. It rebuilds the same uncompressed data chunk this branch already synthesized, and adds slice bounds checking and a guard for slice data without slice parameters. - src/vabackend.c: keep the NVENC coded-buffer path; drop this branch's VP8 note and the always-zero offset in favour of upstream's equivalent. - meson.build: keep this branch's test block, which already registers av1_frame_size_override; upstream's standalone registration would duplicate the test name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
PR elFarto#460 passes the current AV1 frame width and height to NVDEC, but the generic PicWidthInMbs and FrameHeightInMbs fields still describe the VA context maximum. CUVID documents those fields as coded frame dimensions, and FFmpeg derives them from its current AV1 frame. Calculate both macroblock counts from the same per-frame width and height used by CUVIDAV1PICPARAMS. This keeps the parameter block internally consistent for frame_size_override_flag streams. On an RTX 3070, two generated resize streams decoded identically before and after this change; this aligns the API parameters but does not demonstrate a visible output fix on that GPU.
Brings in upstream elFarto#469 (vp8: reconstruct frame headers from VA-API parameters) and elFarto#460 (AV1 frames smaller than the sequence maximum). Conflict resolution: - src/vp8.c: take upstream. It rebuilds the same uncompressed data chunk this branch already synthesized, and adds slice bounds checking and a guard for slice data without slice parameters. - src/vabackend.c: keep the NVENC coded-buffer path; drop this branch's VP8 note and the always-zero offset in favour of upstream's equivalent. - meson.build: keep this branch's test block, which already registers av1_frame_size_override; upstream's standalone registration would duplicate the test name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
av1: decode frames smaller than the sequence maximum at their real size
What you see
With some AV1 streams, hardware decode through this driver starts out fine and
then falls apart: a few seconds after each keyframe, moving parts of the picture
turn into broken blocks, and the damage keeps spreading the more things move
until the next keyframe arrives. Static content can look almost normal, which
makes it easy to mistake for a network or encoder problem.
We hit it in Chromium WebRTC loopback and screen sharing with a hardware AV1
encoder, but it is not Chromium-specific:
ffmpeg -hwaccel vaapishows exactlythe same corruption on the same files, while libdav1d and NVIDIA's own Vulkan
Video decoder (same GPU) decode them perfectly.
Which streams are affected
AV1 lets a stream declare a maximum frame size in its sequence header and then
code individual frames smaller than that (
frame_size_override_flag). Hardwareencoders do this all the time. NVENC, for example, works in 16-pixel-aligned
blocks, so for 1080p content it declares a 1920x1088 maximum and codes every
frame at 1920x1080 (and 3840x2176 vs 3840x2160 for 4K).
Streams whose frames are coded at the full maximum size (for example most
software-encoded content) are not affected, which is probably why this has
gone unnoticed.
Why it happens
Players correctly create the VA context and surfaces at the sequence maximum.
The driver then assumed every frame was that size as well:
copyAV1PicParamtold NVDEC the current frame was the context size(1920x1088), and gave every reference frame the size of its surface
(also 1920x1088), instead of the sizes the frames were really coded at
(1920x1080). The first frames after a keyframe still come out right, but
prediction runs against the wrong frame geometry, so small errors appear
wherever there is motion and compound frame after frame.
Once the right sizes are passed, a second issue shows up: NVDEC does not
crop a smaller frame out of the decoder output, it stretches it to fill
the display area the decoder was created with. A 1080-line frame would come
out scaled to 1088 lines.
The first point was found by recording the exact
CUVIDAV1PICPARAMSthisdriver and ffmpeg's own NVDEC AV1 hwaccel send for the same stream: the only
fields that differed were the frame and reference sizes.
What the patch changes
Real sizes in the picture parameters. The current frame gets its coded
size; that size is remembered on the surface the frame is decoded into, and
each reference frame is described with its remembered size instead of its
surface size. The picture parameters now match ffmpeg's NVDEC hwaccel.
Display area follows the frame size. When a frame's size differs from the
decoder's display area, the display area is changed with
cuvidReconfigureDecoder. The frame is placed unscaled in the top-left of theunchanged, surface-sized target, so the existing copy into the surface keeps
working as before.
Two details worth knowing when reviewing:
first change is applied right after the first picture is decoded and before
that picture is handed to the resolve thread (the display area takes effect
when a frame is mapped, not when it is decoded).
output are allowed to finish before the display area changes, so they are
not cropped with the new size.
Streams whose frames are all coded at the maximum size never request a display
area change and take exactly the same path as before.
How it was tested
RTX GPU, NVIDIA driver 610.57.04, direct backend. Each stream was decoded
through this driver with
ffmpeg -hwaccel vaapiand compared frame by frameagainst libdav1d:
av1_nvenc)No reconfigure failures or CUDA errors were logged. Also confirmed by eye in a
Chromium WebRTC AV1 loopback with heavy motion: before, the picture degraded
into blocks within seconds; after, it stays clean.
Not covered: a stream whose frame size changes in the middle of playback (no
such sample was available). The code path for it is described above.
Reproducing it yourself
You need a stream whose frames are smaller than the sequence maximum. The
simplest way to check an existing file:
frame_size_override_flag = 1withframe_height_minus_1smaller thanmax_frame_height_minus_1means the stream is affected.Then compare the hardware decode with libdav1d (1080p example):
Without the patch the count stops matching roughly 30 frames after every
keyframe; with it, every frame matches.
🤖 Generated with Claude Code