Add vainfo feature-set check against DUT manifest (New) - #2768
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2768 +/- ##
==========================================
+ Coverage 61.29% 61.37% +0.07%
==========================================
Files 497 498 +1
Lines 50273 50383 +110
Branches 8834 8854 +20
==========================================
+ Hits 30815 30921 +106
+ Misses 18619 18618 -1
- Partials 839 844 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds VA-API codec feature verification by comparing vainfo’s advertised VA profiles/entrypoints against DUT manifest feature flags, ensuring both that (1) the driver doesn’t expose undeclared codec capabilities and (2) manifest-declared capabilities are actually supported by the driver.
Changes:
- Extend the VA-API automated test plan to run a new
vainfo-based resource and generated per-profile checks, plus a reverse “manifest vs driver” validation job. - Add a VA-API manifest feature-set matrix (one manifest entry per VAProfile × encoder/decoder direction).
- Introduce
va_profile_check.py(resource/forward/reverse modes) with unit tests validating parsing and manifest comparison behavior.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| providers/base/units/va-api/test-plan.pxu | Includes the new vainfo resource, per-profile generated jobs, and the reverse manifest-vs-driver job in the automated VA-API plan. |
| providers/base/units/va-api/manifest.pxu | Adds a comprehensive set of VAProfile encoder/decoder manifest entries for DUT declaration and verification. |
| providers/base/units/va-api/jobs.pxu | Defines the new resource job, the template-generated forward-check jobs, and the reverse-check job. |
| providers/base/tests/test_va_profile_check.py | Adds unit tests for parsing, mapping, and forward/reverse command behavior. |
| providers/base/bin/va_profile_check.py | Implements the vainfo parsing and bidirectional manifest/driver feature verification logic. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thank you for the feedback @mckees : your feedback is mostly resolved: see comments. I requested a review from copilot balanced. |
mckees
left a comment
There was a problem hiding this comment.
LGTM. Aside from the review, I ran the test plan and ensured that anything present in vainfo but not the manifest fails the test. Anything declared "false" explicitly which is actually supported by the driver also fails correctly. I inserted a manual, nonexistent "profile3" for AV1 encode and made sure that it also failed when the driver doesn't advertise that it exists.
|
@ahresse I just noticed that you've used pxu files. Checkbox has started moving to .yaml files instead. Please use |
The DUT manifest declares the VA-API codec features the device is expected to support, one encoder and decoder entry per VAProfile. This adds a resource job that parses the profiles and entrypoints advertised by vainfo and checks them in both directions against the manifest. The resource job emits one record per (profile, direction) pair mapped to its manifest feature. A template generates one pass/fail job per feature so the driver cannot silently advertise an undeclared codec, and a reverse job fails when a declared feature is not exposed by the driver. Signed-off-by: Alexandre Esse <[email protected]>
Signed-off-by: Alexandre Esse <[email protected]>
The generated summary for VAProfileH264ConstrainedBaseline exceeded 80 characters, triggering validation warnings at runtime. Shorten the template summary so the longest instantiated job stays within the limit. Signed-off-by: Alexandre Esse <[email protected]>
The va-profile-* template jobs are generated from the va-api/vainfo resource, which must run during the bootstrap phase for the jobs to be instantiated in time. Move va-api/vainfo to bootstrap_include. Signed-off-by: Alexandre Esse <[email protected]>
Previously cmd_reverse returned 0 (pass) when the manifest declared no VA-API codec features. A renamed or absent feature set then produced a silent green pass, hiding a real misconfiguration. Fail loudly when no declared features are found so the problem surfaces instead of passing by having zero cases. Signed-off-by: Alexandre Esse <[email protected]>
The false-valued feature (has_h264_main_decoder) was also present in the vainfo test output, so the test could never fail even if the skip logic broke. Use a feature that is False and absent from the output so the test genuinely proves False entries are ignored. Also add a test for the new fail-on-empty behaviour when no features are declared. Signed-off-by: Alexandre Esse <[email protected]>
TestMain.test_forward_declared and test_forward_undeclared duplicated TestCmdForward.test_declared_feature_succeeds and test_undeclared_feature_fails. Command dispatch is already covered by test_resource and test_reverse, so remove the duplicates. Signed-off-by: Alexandre Esse <[email protected]>
Co-authored-by: ahresse <[email protected]>
Signed-off-by: Alexandre Esse <[email protected]>
mckees
left a comment
There was a problem hiding this comment.
Re-ran testing after the PXU to YAML conversions, and all LGTM
Description
The DUT manifest declares the VA-API codec features the device is expected to support, one encoder and decoder entry per VAProfile. This adds a resource job that parses the profiles and entry points advertised by vainfo and checks them in both directions against the manifest.
The resource job emits one record per (profile, direction) pair mapped to its manifest feature. A template generates one pass/fail job per feature so the driver cannot silently advertise an undeclared codec, and a reverse job fails when a declared feature is not exposed by the driver.
Resolved issues
This is in preparation of: https://github.com/canonical/checkbox/commits/ffmpeg/
Since it relies on vainfo, we want to make sure vainfo is right.
Somehow also related to: https://warthogs.atlassian.net/browse/PEK-2747
Documentation
Documentation is not affected by these changes.
Tests
Sanity-check the resource parser on the local machine (should list the profiles the local driver advertises, each mapped to a has_*_decoder/_encoder feature):
cd providers/base/bin && ./va_profile_check.py resource
./va_profile_check.py forward has_h264_high_decoder # fails (exit 1) with no DUT manifest
./va_profile_check.py reverse # passes (exit 0) with no declared feature