Skip to content

Alpha support #207

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chcunningham opened this issue Apr 29, 2021 · 7 comments · Fixed by #274
Closed

Alpha support #207

chcunningham opened this issue Apr 29, 2021 · 7 comments · Fixed by #274
Labels
extension Interface changes that extend without breaking.

Comments

@chcunningham
Copy link
Collaborator

chcunningham commented Apr 29, 2021

I've been meaning to send a PR for the latest thinking from the Chrome explorations... I'll try to have it out this week.

Generally, my proposal will be:

  • add a attribute bool hasAlpha to VideoFrame. The semantics for planar formats would be that the last plane (highest index) is the alpha plane. More on this in Alpha support #200 (comment)
  • Define an enum AlphaOption { keep, discard }, and add attributes/members using this enum to VideoFrameInit and VideoEncoderConfig. Choose defaults based on expected use cases.
    • For VideoEncoderConfig.alpha, default to =discard. Alpha encoding is a thing, but relatively rare. In the event that the VideoFrame is backed by a texture, we can use =discard to optimize readback to a alpha-less format that is supported by the VideoEncoder (e.g. NV12)
    • For VideoFrameInit.alpha, default to =keep. Seems least disruptive in the event you weren't planning to encode the frame (which is already covered above).
  • For VideoEncoder, if configured with alpha=keep (and if supported), we emit the alpha data as side data in the chunk output callback. This follows the path of existing underlying encoders (e.g. ffmpeg's AV_PKT_DATA_MATROSKA_BLOCKADDITIONAL).
    • The output callback already emits chunk metadata in the form of the VideoDecoderConfig needed to decode the chunk. PR Add SVC modes and metadata #187 is expanding that into a metadata bucket for homing chunk SVC metadata. I propose to just add alpha side data there as well... TBD some naming bikeshed.

Note: we haven't yet thought about how to do decoding of alpha. Something to consider.

@sandersdan
Copy link
Contributor

Chrome's implementation has RGBA, BGRA, RGBX, and BGRX. Would this be trimmed down to RGBA and BGRA by your proposal?

@chcunningham
Copy link
Collaborator Author

I don't immediately see the value in having the 'X' formats externally. IIUC, they have an alpha channel that conveys no meaningful info (all 255)? If so, I'm inclined to always call RGBX as RGB and so on, and hide the all-255 alpha channel from JS.

@dalecurtis
Copy link
Contributor

dalecurtis commented Apr 30, 2021

I think we still want the X variants since we aren't reducing 4 bytes to 3 bytes during readInto. Alternatively having just RGBA, BGRA would also be okay.

@chcunningham
Copy link
Collaborator Author

Ah, good point.

I don't have a strong intuition about whether sites could implement impact optimizations for RGBX (ignoring the alpha plane). If not, I'd vote to just simplify to RGBA.

@dalecurtis
Copy link
Contributor

If we go with RGBA, BGRA only we need the hasAlpha flag.

@chcunningham chcunningham added the extension Interface changes that extend without breaking. label May 12, 2021
@chcunningham
Copy link
Collaborator Author

Triage note: marking 'extension' as the proposals are primarily about adding attributes for keeping/discarding/having alpha.

@chcunningham
Copy link
Collaborator Author

Editors call:

  • Marking need-definition to highlight that we should iron this out w/ priority before launch.
  • General proposal sounds good.
  • For videoFrame.hasAlpha - sounds good, analogous to ffmpeg. For planar formats, hopefully we can just always define the alpha plane as full resolution as the last plane. Will look more at ffmpeg etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
extension Interface changes that extend without breaking.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants