Skip to content

vc1: restore bitstream framing and complete field pairs before resolve - #470

Merged
elFarto merged 1 commit into
elFarto:masterfrom
donbernhardo:prepare/vc1-decode
Oct 5, 2026
Merged

elFarto merged 1 commit into
elFarto:masterfrom
donbernhardo:prepare/vc1-decode

Conversation

@donbernhardo

@donbernhardo donbernhardo commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

vc1: restore bitstream framing and complete field pairs before resolve

Advanced-profile VC-1 decoding can report success while returning a frozen
picture. FFmpeg submits slice bytes without the Advanced-profile start code,
but NVDEC expects it. Field-coded pictures also share an output surface which
must not be resolved after only the first field.

This change:

  • Restores Advanced-profile frame (0x0d), second-field (0x0c) and additional
    slice (0x0b) markers. Preserves markers already supplied by a client and
    leaves Simple/Main WMV3 data markerless.
  • Decodes the first field without queueing its surface for resolve, then resolves
    after the matching second field. Releases pending surfaces on errors,
    replacement pictures and surface/context teardown.
  • Maps all eight field-picture type pairs to the submitted field's intra/reference
    flags, including the VC-1-specific CUVID flags. Corrects progressive and field
    order metadata, uses the supplied coded dimensions and copies panscan metadata.
  • Validates picture parameters and slice ranges, handles allocation failures and
    returns submission errors instead of silently skipping invalid buffers.
    Copies slice parameters so their client buffer may be destroyed between calls.
  • Keeps decoded VA-API bitplane flags out of the compressed stream; NVDEC parses
    the original coded bitplanes from the slice bytes.
  • Keeps VC-1 submission validation, field-pair tracking, error recovery and surface
    cleanup in vc1.c, using optional NVCodec lifecycle callbacks. VC-1 state is
    private to the codec through codecData; the shared backend calls generic hooks.
  • Consistently guards codec callbacks and rejects decode contexts without a codec.
    VC-1 hooks handle missing private state defensively; initial state allocation
    uses malloc, and the redundant initial render-state assignment is removed.

The branch is rebased and squashed into one commit on upstream master.
This PR is a draft pending manual inspection. The VC-1 test sources and their
Meson integration remain in the local repository and are not included in this PR.

Reproduce and verify

Requires FFmpeg with VA-API, an NVIDIA GPU supporting VC-1, and
/dev/dri/renderD128. Run from the repository root in both unpatched and patched
checkouts, using each checkout's build directory:

meson setup build -Dbuildtype=debugoptimized
ninja -C build

curl -fL https://samples.ffmpeg.org/V-codecs/WVC1/FlightSimX_720p60_51_15Mbps.wmv \
  -o build/vc1.wmv

ffmpeg -y -i build/vc1.wmv -frames:v 120 -fps_mode passthrough \
  -pix_fmt nv12 -f rawvideo build/software.nv12

LIBVA_DRIVER_NAME=nvidia LIBVA_DRIVERS_PATH="$PWD/build" NVD_BACKEND=direct \
  timeout -k 1s 30s ffmpeg -y \
  -hwaccel vaapi -hwaccel_device /dev/dri/renderD128 \
  -hwaccel_output_format vaapi -i build/vc1.wmv \
  -frames:v 120 -fps_mode passthrough -vf 'hwdownload,format=nv12' \
  -f rawvideo build/hardware.nv12

cmp build/software.nv12 build/hardware.nv12

Tested on an RTX 3070 with NVIDIA driver 615.71.09 and FFmpeg n9.0.2:

  • Unpatched upstream f77ef1c returns frozen Advanced-profile output which differs
    from software decode.
  • With this change, cmp exits with status zero; the 120-frame outputs contain
    165,888,000 identical bytes.

Testing performed

Local translation and callback tests (runs without GPU; test sources are not included in this PR):

  • Multiple slices, all eight field-picture type combinations
  • Marker preservation when client supplies markers
  • Parameter-buffer lifetime (parameters copied to driver storage)
  • Malformed ranges and allocation failure handling
  • Submission ordering, first/second-field resolve decisions, failed decode cleanup,
    surface retirement, missing-state handling and recovery after context-state allocation failure
  • Passes with AddressSanitizer and UndefinedBehaviorSanitizer
  • Real two-slice picture reconstruction matches NVIDIA parser byte-for-byte (107,692 bytes, offsets [0, 11093])

Decode validation (byte-for-byte match with FFmpeg software decode):

Stream Profile / coverage Frames
FlightSimX_720p60_51_15Mbps.wmv Advanced, progressive 120
VC1_interlaced_1080i60_with_artifacts_crashes.mkv Advanced, mixed frame/field coding 86
vc1-interlaced-bframes.m2ts Advanced, interlaced B pictures 120
Test_1440x576_WVC1_6Mbps.wmv Advanced 120
sb-spl-q80.avi Simple 120
sb-main-q80.avi Main 120
nokia_n90.wmv Main 120
H.264 control Regression check 90

All streams use native frame output (-fps_mode passthrough) to avoid timestamp duplication artifacts.

The mixed-interlace stream passes three consecutive runs and a separate run with client-supplied markers already present.

Local GPU lifecycle tests (requires capture setup; test sources are not included in this PR):

  • First-field context/surface teardown
  • Replacement pictures during field pair
  • Malformed submissions and recovery
  • Second field on wrong surface
  • All return values and surface waits verified under watchdog

Refactor verification

After moving the lifecycle logic behind codec callbacks:

  • GCC 16.2.1 and Clang 22.1.8 builds pass with warnings treated as errors.
  • Local translation/callback tests pass, including AddressSanitizer and
    UndefinedBehaviorSanitizer checks and the real two-slice parser capture.
  • All six local GPU lifecycle scenarios pass under a watchdog.
  • The seven VC-1 streams and H.264 control listed above still match software
    decode byte-for-byte.
  • Both AV1 regression cases pass byte-for-byte; JPEG output matches the earlier
    driver, and the VP8 control matches software decode byte-for-byte.

Known limitations

buggyDMOdecoding.wmv still differs from FFmpeg software decode. For 120 native
frames, this driver and NVIDIA VDPAU produce byte-identical output, including
those differences (723,202 of 13,824,000 bytes differ; 595,240 by one; maximum
difference 79). Both hardware decoders agreeing suggests the issue is not in this
driver's VA-API translation layer.

Legacy WMV3 samples with RES_RTM=0 also differ. These clips heavily use the
legacy transform sub-block-pattern syntax gated on that flag (tens of thousands
of uses in 120 frames; zero in modern clips). NVDEC does not implement the legacy
syntax: forcing FFmpeg to decode with modern syntax reproduces this driver's
output exactly over most frames. Neither VAPictureParameterBufferVC1 nor
CUVIDVC1PICPARAMS exposes RES_RTM, so the driver cannot detect or forward it;
any fallback for such streams belongs to the client. Modern Simple/Main samples
pass byte-for-byte.

@donbernhardo
donbernhardo marked this pull request as ready for review October 1, 2026 12:17
@elFarto

elFarto commented Oct 1, 2026

Copy link
Copy Markdown
Owner

Thanks for the patch, but I'm not sure I would be happy to merge this in it's current state. It weaves a lot of codec specific code through the main vabackend.c file which I was trying to avoid. And it's for a codec that's not that widely used now. If you can come up with a way to have less VC1 specific code in the main file, I'd be more inclined to accept it.

You can add more codec functions that can be called at certain times.

@donbernhardo
donbernhardo marked this pull request as draft October 1, 2026 20:12
Advanced-profile VC-1 decode can report success while returning frozen output:
VA-API clients such as FFmpeg submit slice bytes without the start codes NVDEC
expects. Field-coded pictures also share an output surface which must not be
resolved until both fields have been decoded.

Restore frame (0x0d), second-field (0x0c) and additional-slice (0x0b) markers,
preserve valid client-supplied markers, and leave Simple/Main WMV3 markerless.
Keep decoded VA-API bitplane flags out of the compressed stream because NVDEC
parses the original coded bitplanes from the submitted slice bytes.

Add optional NVCodec callbacks for submission checks, buffer dispatch, decode
preparation/completion, aborts and surface retirement. Pass the render surface
ID to beginPicture and adapt JPEG's existing callback. Keep VC-1 lifecycle
handling and state private to vc1.c through codecData, so vabackend.c invokes
generic hooks without VC-1-specific lifecycle branches.

Decode the first field without queueing its surface for resolve. Publish only
a matching completed pair, and release pending waits on errors, replacement
pictures, surface retirement and normal context teardown. Retain surface IDs
rather than pointers between submissions and synchronize codecData reallocation
with surface cleanup.

Map all eight field-picture type pairs to the submitted field's intra/reference
flags, including CUVID's VC-1-specific flags. Correct progressive and field-order
metadata, use the supplied coded dimensions, and forward panscan metadata.
Validate picture parameters, slice ranges and whole-slice submissions; retain
copied slice parameters when the client releases its buffers between calls.
Report allocation and submission failures instead of silently skipping buffers.

Use malloc for initial codec-state allocation, eliminate the unused initial
render-state assignment, consistently guard codec hook calls, reject decode
contexts without a codec, and handle absent VC-1 state defensively.

Validation (local tests and fixtures are intentionally excluded from this PR):
- GCC 16.2.1 and Clang 22.1.8 builds with werror=true
- VC-1 translation/callback tests under AddressSanitizer and UBSan, including
  allocation failures, missing state and real two-slice parser reconstruction
- All six GPU lifecycle scenarios under watchdog
- Seven VC-1 fixtures and H.264 control match verified software pixels exactly
- AV1 frame-size override and ordinary-size regressions pass bit-exactly
- JPEG/VP8 controls passed during refactor validation

Tested on an RTX 3070 with NVIDIA 615.71.09 and FFmpeg n9.0.2. Existing NVDEC
pixel differences for buggyDMOdecoding.wmv and legacy RES_RTM=0 WMV3 remain;
modern Simple/Main and the tested Advanced streams pass byte-for-byte.
@donbernhardo

Copy link
Copy Markdown
Contributor Author

Your very welcome and I fully agree with your finding. I refactored the code accordingly:

I moved VC-1 lifecycle handling and state into vc1.c, behind optional NVCodec callbacks. The shared backend now calls generic hooks without VC-1-specific lifecycle branches, while preserving field-pair resolution and error recovery. Builds, sanitizer checks, GPU lifecycle tests, and decode comparisons all pass.

@donbernhardo
donbernhardo marked this pull request as ready for review October 1, 2026 20:56
@elFarto

elFarto commented Oct 5, 2026

Copy link
Copy Markdown
Owner

Thanks for the patch!

@elFarto
elFarto merged commit a15fe35 into elFarto:master Oct 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants