1654461 - AVIF files tagged with full range are decoded as limited range

Status: RESOLVED FIXED

(Core :: Graphics: ImageLib, defect)

Description

[Thomas Daede [:TD-Linux]]

Attachment: dst_lossless.avif

The colors on the attached AVIF should match the attached PNG.

Comment 1

[Thomas Daede [:TD-Linux]]

Attachment: src.png

Comment 2

[Thomas Daede [:TD-Linux]]

Attachment: dst_420_full.avif

4:2:0 YUV version

Comment 3

[Jon Bauman [:jbauman:]]

As far as I can tell the colors do match now, and this code looks correct. If I'm missing something, please let me know.

Comment 4

[Yuan Tong]

The attached images are all highly saturated colors so the difference is not very visible.
The range info is correctly set but ignored during conversion from YUV to RGB. The libyuv::U444ToARGB functions or likely are all for limited range YUV data. In the version of libyuv firefox currently using there's only libyuv::J444ToARGB which is the full range version of libyuv::I444ToARGB, and the other two are missing. They are added recently to libyuv. (libyuv::F444ToARGB as the full range version of libyuv::H444ToARGB in this change, and libyuv::V444ToARGB as the full range version of libyuv::U444ToARGB in this change) Moreover, AVIF supports much more colorspaces than firefox currently have (see here), and there's plan in libyuv to provide the ability to generate custom constants, to cover most colorspaces.

Comment 5

[Yuan Tong]

Attachment: full-range-1-13-6.avif

This is a full range AVIF image, tagged as BT.709 color primary, sRGB transfer function, BT.601 matrix coefficients (the same color configuration of JPEG). The background color of the four areas should be what the text says respectively.

Comment 6

[Yuan Tong]

Attachment: Bug 1654461 - Make ConvertYCbCrToRGB32 color range aware r?jbauman

A quick fix for ConvertYCbCrToRGB32 to use full range convert functions for full range data.

Current version of libyuv Firefox have only supports BT.601 full range.
In the long term, we should update libyuv to get support on other colorspaces as well.

Comment 7

[Jon Bauman [:jbauman:]]

Thanks for the additional info! I'll take a look at this soon.

Comment 8

[Mathew Hodson]

If there is a patch being reviewed, then I think this bug should be reopened. The status is confusing right now.

Comment 9

[Jon Bauman [:jbauman:]]

I should be able to review the change this week, hopefully

Comment 10

[Jon Bauman [:jbauman:]]

Looks like it'll be beginning of next week (Mar 8)

Comment 11

[Jon Bauman [:jbauman:]]

Thanks again for this submission. I've looked it over, and it's definitely something we'll incorporate, but as you mention there are additional pieces we need from an updated libyuv first, so I'll be coming back to this once that's done. Unfortunately it may take a bit of effort since it's been awhile since we've updated and in the meantime libyuv has switched from gyp (which we support as part of mozbuild) to gn (which we do not, yet).

Comment 13

[Yuan Tong]

I'm not familiar with either moz-build or gn, so I'm not able to update libyuv and make it work.
However this bug is blocking enable of AVIF by default for quite long, so I backported necessary changes from new version of libyuv to fix this bug.

We should still update libyuv to get new features, including one-step 10/12 bit convert functions, bilinear UV scaling, unclamped limited range coefficients, performance improvement on arm64, and probably more.

Comment 14

[Jon Bauman [:jbauman:]]

This is great! I'd looked a bit at doing the backport, but didn't have enough familiarity with libyuv to get it done in a reasonable amount of time compared to upgrading, so this is very helpful!

I'll take a look at this hopefully by the middle of this week.

Comment 15

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/4c1ded00c10c