Skip to content

Update audio video decoder hfps - #768

Open
foumia wants to merge 1 commit into
developfrom
feature/767-update-hfp-decoder-w3c-media-capabilities
Open

Update audio video decoder hfps#768
foumia wants to merge 1 commit into
developfrom
feature/767-update-hfp-decoder-w3c-media-capabilities

Conversation

@foumia

@foumia foumia commented Aug 3, 2026

Copy link
Copy Markdown

Update audio video decoder hfps to support w3c media capabilities
Existing attributes in hfp are not modified unless it is needed for new schema to support media capabilities

Task: #767

Copilot AI review requested due to automatic review settings August 3, 2026 10:31
@github-project-automation github-project-automation Bot moved this to Architecture Review Required in halif_aidl Aug 3, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the AudioDecoder and VideoDecoder HAL Feature Profile (HFP) YAMLs to express richer codec capability information intended to support W3C Media Capabilities, including per-codec profile limits and (for video) dynamic range per codec.

Changes:

  • Restructured videodecoder/current/hfp-videodecoder.yaml to use codecCapabilities with per-profile limits and per-codec dynamicRange.
  • Replaced the simple supportedCodecs list in audiodecoder/current/hfp-audiodecoder.yaml with detailed per-codec/per-profile constraints (bitrate/channels/sample rate/bit depth).
  • Adjusted top-level keys/resource layout in both HFPs (e.g., introducing Capabilities blocks).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
videodecoder/current/hfp-videodecoder.yaml Refactors video decoder HFP to a per-codec/per-profile capability structure and adds per-codec dynamic range declarations.
audiodecoder/current/hfp-audiodecoder.yaml Expands audio decoder HFP from a codec list into detailed codec/profile constraint declarations.
Suppressed comments (1)

videodecoder/current/hfp-videodecoder.yaml:260

  • Same as above: the resource no longer declares supportedDynamicRanges, but Capabilities.aidl still exposes that field and older HFP versions provided it. Consider retaining supportedDynamicRanges as the union of all codec dynamicRange entries for this resource.
      supportedColorimetries:
        - BT601_525
        - BT601_625
        - BT709
      supportsSecure: true

Comment on lines 146 to 151
supportedColorimetries:
- BT601_525
- BT601_625
- BT709
- BT2020
supportsSecure: true
Comment on lines +41 to +43
Capabilities: # Resource list
- 0: # Resource object begins (high-tier decoder)
codecCapabilities:
Comment on lines +25 to +30
audiodecoder: # Component object begins
interfaceVersion: current
Capabilities:
- 0:
codecCapabilities:
- PCM:
@Ulrond

Ulrond commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks for picking this up — the direction is right, and splitting codec/profile with per-profile limits is what we need for W3C Media Capabilities alignment. A few things to resolve before this can land.

Blocking: the audio HFP doesn't parse into the intended structure

The two files use different indentation conventions and the audio one nests incorrectly.

Video is correct — profiles is indented under the codec key:

- MPEG2_VIDEO:
    profiles:
      - MPEG2_SIMPLE:
          maxLevel: MPEG2_LEVEL_HIGH

parses to {MPEG2_VIDEO: {profiles: [...], dynamicRange: [...]}}

Audio is not — profiles sits at the same level as the codec key:

- PCM:
  profiles:
    - BASE:
      maxBitrateInBps: 1536000

parses to {PCM: null, profiles: [{BASE: null, maxBitrateInBps: 1536000, ...}]}

The codec name and the profile name both become null-valued siblings rather than parents. This affects every codec and every profile in hfp-audiodecoder.yaml.

Same issue on the resource index, in both files. On develop, resource 0 parses as {0: {...}}. On this branch it parses as {0: null, codecCapabilities: [...], supportsSecure: true} — the index is no longer the parent of the resource object. That's a regression against the existing structure.

Worth adding an HFP parse/schema check to CI so this class of error can't merge.

maxLevel contradicts the frame dimensions

Resource 1 declares H265_MAIN_10 with maxLevel: H265_LEVEL_6_2 and maxFrameWidth: 1920. Level 6.2 permits 8192×4320@120. Both resources carry identical maxLevel values with only width/height differing, which suggests these aren't yet real vendor figures.

This matters specifically for Media Capabilities: a client calling decodingInfo() with a Level 6.2 codec string will get back "supported" and then fail at decode. Either the levels need to be the real ones, or the header needs to state explicitly that maxFrameWidth/Height/FrameRate clamp the declared level — and the clamping needs to be normative, not a comment.

dynamicRange double-declares HDR

H265_MAIN_10_HDR10 exists as a profile and HDR10 appears in dynamicRange. Two sources of truth that can disagree. We should pick one — I'd suggest dynamicRange and drop HDR from the profile enum, but either way it needs to be one place.

Also worth confirming DOLBY_VISION under H264 and AV1 is real for target silicon rather than inherited from the template.

Bitrates and audio limits look like placeholders

Every H264, VP9 and AV1 profile is 50 Mbps, identical across profiles and across both resources. The file header says to use 0 where the vendor doesn't publish a cap — most of these should probably be 0 until we have real numbers, otherwise the per-profile granularity carries no information.

In audio, AAC LC / HE_V1 / HE_V2 / ELD / X_HE all carry 576 kbps / 8ch / 96 kHz / 24-bit. HE-AAC v2 is parametric-stereo by definition so 8 channels looks wrong, and 96 kHz isn't a plausible xHE-AAC ceiling. These need checking against the codec specs.

Duplication

Audio resource 0 and resource 1 are byte-identical — 17 codecs duplicated wholesale, taking the file from 1.7 KB to 11.6 KB. That will drift on the first edit. If resources share a capability set we should be able to declare it once and reference it, rather than copying.

Capabilities vs IAudioDecoder

Renaming the resource list key from IAudioDecoder to a generic Capabilities loses which interface the resource list belongs to. If a component ever exposes more than one interface this becomes ambiguous. Was there a reason for the rename?

supportedDynamicRanges

Per Copilot's comment — the resource no longer declares it but Capabilities.aidl still exposes the field. Related to the point below.

Coordination with #149

The structures this PR introduces — codecCapabilities, per-profile limits, the resource-list layout — have no AIDL backing on develop. They're defined in #149 (CodecSupport.aidl, Profile.aidl, reworked Capabilities.aidl), which also touches both of these YAML files. So this is effectively a schema change expressed only in data.

Suggest we land #149 first with its scope frozen to exactly what's needed here, then rebase this onto it — at which point this becomes a small, pure-data PR. Anything here that needs new AIDL (per-codec dynamicRange placement, the resource-list construct) should be pulled back into #149 as a minimal addition.

One forward-looking note

Resource 0 at 4K and resource 1 at HD is a static split. Is 1080p decoder 1's standalone hard limit, or its limit while decoder 0 is running 4K? If it's the latter, we're solving concurrency by hardcoding the pessimistic case and permanently giving away capability.

Either way, the header should state now that these per-resource maxima are standalone maxima, not guaranteed-concurrent ones — so that when a joint-constraint declaration lands alongside Capabilities, it isn't a breaking reinterpretation of what these numbers already meant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Architecture Review Required

Development

Successfully merging this pull request may close these issues.

3 participants