Introduce a single atomic VideoDecoder property setter API to replace multiple Binder calls

[shafi12]

Currently, the client configures VideoDecoder properties using multiple individual Binder calls, such as:

setMasteringDisplayInfo
setContentLightLevel
setColorimetry
setStreamResolution
setFrameRate
setDolbyVisionLayerFlags
setPixelAspectRatio
Dynamic range–related setters

Each of these APIs results in a separate Binder transaction between the client and server. This increases IPC overhead and introduces the risk of partial or inconsistent VideoDecoder state during configuration and reconfiguration.

Proposed Solution
Introduce a single atomic VideoDecoder‑level API that allows all related properties to be set in one Binder call.
Plain Textaidl isn’t fully supported. Syntax highlighting is based on Plain Text.boolean setPropertyMultiAtomic( in int VideoDecoderResourceIndex, in PropertyKVPair[] propertyKVListShow more lines

VideoDecoderResourceIndex identifies the target VideoDecoder instance
propertyKVList contains all VideoDecoder‑specific properties such as HDR metadata, colour information, stream resolution, frame rate, Dolby Vision layer flags, pixel aspect ratio, and dynamic range information

This consolidates multiple setter calls into one Binder transaction.

[Ulrond]

Real concern (IPC overhead + partial-state risk during reconfiguration), but the proposed mechanism doesn't fit the existing types and has a redundant parameter. Two corrections + a counter-proposal:

Issues with the proposal as written

1. The values aren't PropertyKVPair-shaped

The 6 typed setters listed take structured parameters, not enum + PropertyValue:

Setter	Parameter type
setMasteringDisplayInfo	@nullable MasteringDisplayInfo (parcelable)
setContentLightLevel	@nullable ContentLightLevel (parcelable)
setColorimetry	Colorimetry (enum — but expanding to 4 fields per #367)
setStreamResolution	int width, int height
setFrameRate	int numerator, int denominator
setDolbyVisionLayerFlags	boolean blPresent, boolean elPresent
setPixelAspectRatio (#441 proposal)	int parX, int parY

The current Property enum (lines 1-12 of Property.aidl) only contains structural items (RESOURCE_ID, INPUT_QUEUE_DEPTH, OPERATIONAL_MODE, LOW_LATENCY_MODE, etc.) — none of the HDR/colorimetry/resolution/framerate values. To make PropertyKVPair[] carry these, PropertyValue would have to grow into a giant union supporting MasteringDisplayInfo, ContentLightLevel, Colorimetry, paired ints for resolution/framerate/PAR/DV flags, etc. That's a worse type-safety story than the current per-setter approach.

2. VideoDecoderResourceIndex is redundant

IVideoDecoderController is already per-instance — the controller you hold is for one specific decoder (acquired via IVideoDecoder.open()). There's no need to pass a resource index in every call. The implicit this of the controller already identifies the target.

Counter-proposal — structured config parcelable

Define a single VideoDecoderStreamConfig parcelable that bundles all the stream-description fields, and one setStreamConfig() method. Atomic, type-safe, no PropertyValue expansion:

@VintfStability
parcelable VideoDecoderStreamConfig {
    // Resolution
    @nullable Resolution resolution;        // {width, height}, null = no change

    // Frame rate
    @nullable Fraction frameRate;           // {numerator, denominator}, null = no change

    // Pixel aspect ratio (per #441)
    @nullable Fraction pixelAspectRatio;    // null = no change

    // Colour
    @nullable Colorimetry colorimetry;      // null = no change
    @nullable MasteringDisplayInfo masteringDisplayInfo;
    @nullable ContentLightLevel contentLightLevel;

    // Dolby Vision
    @nullable DolbyVisionLayerFlags dolbyVisionLayerFlags;  // {blPresent, elPresent}, null = no change
}

// On IVideoDecoderController:
void setStreamConfig(in VideoDecoderStreamConfig config);

Properties of this shape:

• Atomic: all fields land in one binder transaction
• Partial: null fields mean "no change" — clients send only what's actually changing
• Type-safe: structured parcelables, no PropertyValue gymnastics
• Discoverable: clients see the full set of stream-description fields in one place
• Per-instance: lives on the controller, no resource index needed

The existing 6 individual setters can stay as convenience aliases (or be deprecated and removed in the next major version — pre-restructure timing).

What about non-stream-description properties?

Properties that are genuinely key-value (DECODE_ERROR_POLICY, OPERATIONAL_MODE, LOW_LATENCY_MODE, etc.) keep using setProperty(Property, PropertyValue). If atomic batching is needed for those too, a setPropertyMulti(PropertyKVPair[]) is straightforward and uses existing types — but it's a separate concern from the stream-description bundling.

cc @hari22yuva @srinivasgtl

Two questions before this becomes a tracking issue:

1. Do you agree the structured-parcelable shape is the right answer (vs the PropertyValue-expansion path)?
2. Should the existing 6 typed setters be kept as convenience aliases or removed (pre-restructure)?

[Ulrond]

Correction to my earlier counter-proposal — ABR exposes a real gap

Re-checked the existing setters: all 6 are @pre State::READY. None work mid-playback. So my setStreamConfig() would inherit the same limitation, and ABR resolution / HDR-metadata changes mid-stream are an existing gap regardless of whether we batch the setters or not.

Two distinct cases

Case A — bitstream-derived changes (resolution, frame rate, in-stream colorimetry, in-stream HDR SEI):

• Decoder picks them up from SPS/SEI automatically as the new stream arrives
• Reported via FrameMetadata
• No middleware action needed — works today, works for ABR rep switches that include new SPS/PPS

Case B — container-derived changes (HDR static metadata in MP4 mdcv/cclv boxes that differ per DASH Period, DV config that varies per segment):

• Bitstream may not carry these — they live in the manifest/container
• Middleware sees them at Period/segment boundaries
• Has nowhere to go today — the READY-only setters can't help, no per-segment API exists

Revised proposal

Two surfaces with clear scope:

1. Initial config (READY only): keep the existing setters — or batch them via a setStreamConfig() parcelable per my earlier comment — for one-time setup before start().

2. Per-segment / per-frame container-derived overrides (works in STARTED): extend InputBufferMetadata (already exists from PR feat(videodecoder): declare supportedOutputPixelFormats per CodecCapabilities (#469) #470 for EOS) with optional override fields:

@VintfStability
parcelable InputBufferMetadata {
    long nsPresentationTime;
    boolean endOfStream;
    boolean discontinuity;

    // NEW — optional overrides applied from this frame onward.
    // null = no change. Bitstream-derived values still take precedence.
    @nullable MasteringDisplayInfo masteringDisplayInfo;
    @nullable ContentLightLevel    contentLightLevel;
    @nullable Colorimetry          colorimetry;
    @nullable DolbyVisionLayerFlags dolbyVisionLayerFlags;
}

Why per-buffer rather than a separate updateStreamConfig():

• Atomic with the frame the override applies from — no state-machine race between the call ordering and the buffer queue
• Naturally aligns with discontinuity boundaries where you'd send these anyway in DASH/HLS
• Reuses existing decodeBufferWithMetadata() plumbing — no new method
• Bitstream-derived precedence still applies — the override is itself a hint, the bitstream wins if the new SPS/SEI say something different (matches the rule we're proposing in To Propagate Pixel Aspect Ratio from GstCaps to HAL and expose new setPixelAspectRatio() API #441 / Task: [videodecoder] Add setPixelAspectRatio() and clarify hint precedence + persistence (#441) #486)

What this resolves vs leaves open

Concern	Solved by
Atomic batched setup config	Existing setters or setStreamConfig() parcelable (READY-only, original proposal)
ABR resolution/framerate change	Already works — bitstream-derived (Case A)
ABR HDR static metadata change per Period	NEW — InputBufferMetadata overrides (Case B)
In-stream HDR SEI changes	Already works — Case A, decoder handles

Note on VideoDecoderResourceIndex

Still redundant for the per-buffer case — decodeBufferWithMetadata() is on IVideoDecoderController which is per-instance.

cc @hari22yuva @srinivasgtl @shafi12 — does this two-surface model fit the ABR scenarios you're hitting?

[Ulrond]

Agreed direction (confirmed by @Ulrond / Gerald):

Both surfaces, scoped by lifecycle:

Surface 1 — VideoDecoderStreamConfig (READY-state, atomic setup batching)

A parcelable with all optional/nullable fields, set once via a single setStreamConfig() call. null = "no value" / "use existing setter default". Solves the multi-call IPC and partial-state-during-setup problems shafi12 raised:

@VintfStability
parcelable VideoDecoderStreamConfig {
    @nullable Resolution            resolution;          // null = no hint
    @nullable Fraction              frameRate;
    @nullable Fraction              pixelAspectRatio;
    @nullable Colorimetry           colorimetry;
    @nullable MasteringDisplayInfo  masteringDisplayInfo;
    @nullable ContentLightLevel     contentLightLevel;
    @nullable DolbyVisionLayerFlags dolbyVisionLayerFlags;
}

// On IVideoDecoderController:
void setStreamConfig(in VideoDecoderStreamConfig config);  // @pre READY

Existing typed setters (setStreamResolution, etc.) can either stay as convenience aliases or be deprecated in this version and removed in the next major — pre-restructure timing makes either viable. Whichever way we go, PR #487's Doxygen tightening on those setters about precedence/persistence still applies — the same rules govern the parcelable path.

Surface 2 — Extended InputBufferMetadata (STARTED-state, per-buffer overrides)

Per-frame overrides for container-derived metadata that changes mid-stream (ABR HDR profile change per DASH Period, SSAI/CSAI ad insertion with different encoding, live→VOD transitions, manifest reload):

parcelable InputBufferMetadata {
    long nsPresentationTime;
    boolean endOfStream;
    boolean discontinuity;

    // NEW — optional overrides applied from this buffer onward. null = no change.
    // Bitstream-derived values still take precedence; these are still hints.
    @nullable MasteringDisplayInfo  masteringDisplayInfo;
    @nullable ContentLightLevel     contentLightLevel;
    @nullable Colorimetry           colorimetry;
    @nullable DolbyVisionLayerFlags dolbyVisionLayerFlags;
    @nullable Fraction              pixelAspectRatio;
    // Note: resolution / frameRate intentionally omitted — these are bitstream-driven
    // on every modern codec via SPS, so middleware should never need to override them mid-stream.
}

What this does NOT solve

Codec changes and encryption-mode changes mid-stream still need decoder restart (CSAI with different codec, ad with no DRM after main content with CENC). Those are out of scope — middleware orchestrates a teardown/reopen at those boundaries.

Coverage matrix

Scenario	Solved by
Atomic initial config	Surface 1 (setStreamConfig)
ABR resolution / framerate change	Already works — bitstream-derived
ABR HDR profile change per DASH Period	Surface 2 (per-buffer overrides)
SSAI ad insertion (same codec, same encryption)	Surface 2 (per-buffer overrides on the ad boundary frame)
SSAI with codec or encryption-mode change	Decoder restart (middleware policy)
Live→VOD transition	Surface 2 (per-buffer overrides)

Next steps

1. Raise a tracking issue for both surfaces (or convert this discussion to one).
2. Implementation can be done in a single PR once feat(videodecoder): add setPixelAspectRatio() and clarify hint precedence + persistence (#486) #487 lands, since feat(videodecoder): add setPixelAspectRatio() and clarify hint precedence + persistence (#486) #487 establishes the precedence and persistence rules that Surface 1 and Surface 2 inherit.
3. Audio-side equivalent should be considered in parallel — same pattern likely applies for audiodecoder (ad with different audio codec / channel count) and audiomixer (input format changes).

cc @shafi12 @hari22yuva @srinivasgtl — happy to raise the tracking issue. Any objections to the shape above before I do?

[Ulrond]

Closing — design agreed across the comments above; no objections from cc'd reviewers in three weeks.

Agreed shape:

• Surface 1 — VideoDecoderStreamConfig parcelable + setStreamConfig() on IVideoDecoderController, @pre State::READY, all fields @nullable (null = no change). Replaces the multi-call setup pattern.
• Surface 2 — Extended InputBufferMetadata with @nullable MasteringDisplayInfo, ContentLightLevel, Colorimetry, DolbyVisionLayerFlags, pixelAspectRatio. Works in STARTED, applies from the buffer onward, bitstream-derived values still win.

Implementation tracked under #539 against milestone 0.22.0. Depends on #487 landing first (the precedence and persistence rules for the individual setters that Surface 1 and Surface 2 inherit).

Further design refinement can happen in the issue thread.