2079913 - VideoFrame created from an unpremultiplied ImageBitmap is drawn with wrong colors

Status: NEW

(Core :: Audio/Video: Web Codecs, defect, P2)

Description

[Calixte Denizet (:calixte)]

Attachment: videoframe-imagebitmap-alpha.html

Steps to reproduce:

1. Open the attached videoframe-imagebitmap-alpha.html test case.
2. Compare the two squares and the RGBA values printed below them.

The test creates an ImageBitmap with unpremultiplied alpha, draws it directly, then creates a VideoFrame from it and draws that:

const image = new ImageData(new Uint8ClampedArray([200, 100, 50, 128]), 1, 1);
const bitmap = await createImageBitmap(image, { premultiplyAlpha: "none" });
const frame = new VideoFrame(bitmap, { timestamp: 0 });
const ctx = new OffscreenCanvas(1, 1).getContext("2d");
ctx.drawImage(frame, 0, 0);
console.log([...ctx.getImageData(0, 0, 1, 1).data]);
frame.close();
bitmap.close();

Expected results:

Both drawings preserve the original color and alpha, approximately (200, 100, 50, 128), allowing a few units of canvas rounding. The test prints PASS.

Actual results:

On a local Firefox 160.0a1 build on Windows, drawing the ImageBitmap directly reads back (201, 101, 51, 128), while drawing the VideoFrame reads back (142, 199, 99, 128). The squares have visibly different colors and the test prints FAIL.

The same failure was reproduced in both window and dedicated-worker tests.

Implementation notes:

The VideoFrame constructor taking an ImageBitmap wraps ImageBitmap::ToCloneData()'s mSurface and calls InitializeFrameWithResourceAndSize() without using data->mAlphaType. There is already a TODO: Take care of data->mAlphaType at this call:
https://searchfox.org/firefox-main/rev/23fd2ecbdb459f63606a188fdea2d4dbee05507c/dom/media/webcodecs/VideoFrame.cpp#1749

This treats the unpremultiplied bitmap pixels as premultiplied. The readback is consistent with unpremultiplication overflowing: 200 * 255 / 128 is approximately 398, which wraps to 142.

Comment 1

[Thomas Nibler [:tnibler]]

Should be the same fix as bug 1939711.

Comment 2

[Thomas Nibler [:tnibler]]

No that's phrased the wrong way round, handling alpha should also do the cropping.

Comment 3

[media-triaging-bot]

This is a confirmed defect in the VideoFrame constructor for an ImageBitmap source. The attached testcase gives reliable steps to reproduce.

Analysis

ImageBitmap::ToCloneData() returns the source surface without premultiplication, and it puts the alpha type in mAlphaType. The VideoFrame constructor wraps only the surface and ignores mAlphaType, as the TODO in VideoFrame.cpp shows. At draw time, nsLayoutUtils::SurfaceFromVideoFrame reports gfxAlphaType::Premult for all formats with alpha. Thus the draw path reads unpremultiplied bytes as premultiplied bytes. A direct draw of the ImageBitmap is correct, because ImageBitmap::PrepareForDrawTarget premultiplies a NonPremult surface. This gap exists since bug 1774300 added the constructor, so it is not a regression.

Fix direction

In the constructor, premultiply a NonPremult surface with alpha into a new DataSourceSurface, as PrepareForDrawTarget does. The same copy can crop to mPictureRect, which is the problem in bug 1939711. The open revision D330607 on bug 1939711 now has the title "Handle ImageBitmap source's crop and alpha type". We recommend that its review confirm that it premultiplies NonPremult surfaces. If it does, this bug can close as a duplicate of bug 1939711 when that patch lands.

The patch on bug 2079836 changes only the RGBA and BGRA buffer path. If both patches land, make sure that no path premultiplies the pixels two times.

This bug now blocks the WebCodecs meta bug 1746557.

---

To give feedback on this comment, add a thumbs-up or thumbs-down reaction.