Bug 220972 – Color Gamut for VP8 and VP9 seems wrong

• Status: RESOLVED
• Resolution: FIXED
• Component: WebRTC

[Alex. Gouaillard]

Problem:
Safari and iOS incorrectly interprets video levels with VP8/9 encoded content.

My understanding is that most UVC devices as well as Chrome and OBS publish methods by default use “partial” video levels. (Partial is OBS terminology, This is similar to the “legal” vs “Full Range" broadcast video levels). Almost all UVC/Webcams that I’ve used transmit with partial video levels by default. This is also true of hardware encoders. Chrome and Firefox both assume and interpret partial video levels, thus displaying a correct looking image. Safari and iOS for some reason expects and interprets video as Full Range levels. This means that blacks appear dark grey instead of truly black, and whites appear light grey instead of truly white. This has a significantly negative effect on the image accuracy and quality.

This is not a problem with H.264. Safari/iOS correctly interprets video as “partial” and looks correct and similar to Chrome.

I've found this issue is present in Millicast, Google Meet, and Jitsi Meet. It appears to be an issue specifically with how Safari/iOS interprets video that’s encoded in VP8/VP9.

Here is a link to a test video clip with a greyscale ramp. Also in this folder are two screenshots from a waveform monitor showing the difference between Chrome output and Safari output
https://f.io/qdncjeuk

Let me know if you have any questions.

[Radar WebKit Bug Importer]

<rdar://problem/73616455>

[Jean-Yves Avenard [:jya]]

Created attachment 426812 [details]
Patch

[youenn fablet]

Comment on attachment 426812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426812&action=review

> LayoutTests/webrtc/video-vp8-videorange.html:43
> +        testRunner.setUserMediaPermission(true);

Not needed.

> LayoutTests/webrtc/video-vp8-videorange.html:45
> +    return new Promise((resolve, reject) => {

You could do: promise_test(async (test) => {
And then use await:
const remoteStream = await new Promise(...)

> LayoutTests/webrtc/video-vp8-videorange.html:81
> +}, "Ensuring connection state is connected");

Not sure this is actually needed, video.play() will resolve once it gets a remove video frame which means connection is ok.

> LayoutTests/webrtc/video-vp8-videorange.html:84
> +    return checkVideoBlack(false, canvas2, video).then(() => {

async (test) and await checkVideoBlack(...); would make it a bit nicer.

> LayoutTests/webrtc/video-vp8-videorange.html:88
> +        assert_equals([ pixelData[0], pixelData[1], pixelData[2] ].join(), [0, 0, 0].join());

Are we sure they will always be 0,0,0 given we do compression?

> LayoutTests/webrtc/video-vp8-videorange.html:91
> +        assert_equals([ pixelData[0], pixelData[1], pixelData[2] ].join(), [255, 255, 255].join());

Ditto here.

[Jean-Yves Avenard [:jya]]

Comment on attachment 426812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426812&action=review

>> LayoutTests/webrtc/video-vp8-videorange.html:45
>> +    return new Promise((resolve, reject) => {
> 
> You could do: promise_test(async (test) => {
> And then use await:
> const remoteStream = await new Promise(...)

Always amazed on how much code using JS promises can get simplified.

>> LayoutTests/webrtc/video-vp8-videorange.html:81
>> +}, "Ensuring connection state is connected");
> 
> Not sure this is actually needed, video.play() will resolve once it gets a remove video frame which means connection is ok.

I copied an existing test (video-mute-vp8.html) likely can be removed there too then.

>> LayoutTests/webrtc/video-vp8-videorange.html:88
>> +        assert_equals([ pixelData[0], pixelData[1], pixelData[2] ].join(), [0, 0, 0].join());
> 
> Are we sure they will always be 0,0,0 given we do compression?

I can't imagine sur simple pattern, with colours at the edge of the range to ever show colour artefacts.

[Jean-Yves Avenard [:jya]]

Created attachment 426816 [details]
Patch

[Jean-Yves Avenard [:jya]]

Created attachment 426817 [details]
Patch

Set reviewer name in Changelog

[Eric Carlson]

Comment on attachment 426817 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=426817&action=review

> Source/WebCore/ChangeLog:18
> +        In memory of Dr. Alex. May he rest in peace.

❤️

> LayoutTests/webrtc/video-vp8-videorange.html:38
> +// Repeatidly redraw the checker to get the video stream going.

s/Repeatidly/Repeatedly/

[Jean-Yves Avenard [:jya]]

Created attachment 426874 [details]
Patch

Fix typo, add link to bug 224960

[EWS]

Committed r276478 (236937@main): <https://commits.webkit.org/236937@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 426874 [details].

[Truitt Savell]

It looks like the new test webrtc/video-vp8-videorange.html

added in https://trac.webkit.org/changeset/276478/webkit

is a constant failure on macmini8,1

History:
https://results.webkit.org/?suite=layout-tests&test=webrtc%2Fvideo-vp8-videorange.html

filed https://bugs.webkit.org/show_bug.cgi?id=225257