Closed Bug 1654461 Opened 2 years ago Closed 8 months ago

AVIF files tagged with full range are decoded as limited range

Categories

(Core :: ImageLib, defect)

defect

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox90 --- fixed

People

(Reporter: TD-Linux, Assigned: tongyuan200097)

References

(Depends on 1 open bug)

Details

Attachments

(5 files)

Attached image dst_lossless.avif

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

Attached image src.png
Severity: -- → S3
Attached image dst_420_full.avif

4:2:0 YUV version

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.

Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED

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.

Attached image 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.

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.

Assignee: nobody → tongyuan200097

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

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

Flags: needinfo?(jbauman)

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

Status: RESOLVED → REOPENED
Flags: needinfo?(jbauman)
Resolution: FIXED → ---
Blocks: avif-default
No longer blocks: AVIF

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

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).

Depends on: 1698699
Duplicate of this bug: 1702085

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.

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.

Status: REOPENED → RESOLVED
Closed: 11 months ago8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch
You need to log in before you can comment on or make changes to this bug.