brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
16.98k stars 2.21k forks source link

Support JPEG/XL (JPEG XL / JPEGXL) #28411

Open NathanaelA opened 1 year ago

NathanaelA commented 1 year ago

Description

In Chromium v110 the google team removed JPEG/XL support (with a lot of controversy around this). It was behind a flag in the prior versions. In my tests converting (losslessly I might add) from regular JPG to JPX I saved about 20%-25% of the size. The image could be re-converted back to the exact bit identical JPG image when using the lossless conversion. If using a lossy conversion the saving was even more dramatic. This codec has the potential to save massive amounts of bandwidth which means time to first paint is even faster.

Steps to Reproduce

In Brave v109 (and before) you can do Brave://flags image

When brave upgrades to the v110 engine of Chromium, this feature will be removed unless Brave adds the code from v109 to their fork. I propose this is unflagged and released in Brave v110 as there are several other browsers who have also broken away from Google on this issue and are supporting it in thier Chromium or Gecko based browsers and I prefer to use brave as my day to day browser on both my phone and desktop...

Desktop Brave version:

Anything based on v110 of Chromium or later will have this feature removed.

antonok-edm commented 1 year ago

as there are several other browsers who have also broken away from Google on this issue and are supporting it in thier Chromium or Gecko based browsers

Could you provide links to the relevant decisions/announcements from other browsers?

NathanaelA commented 1 year ago

For Chromium Based browsers, Thorium has these two items:

For Gecko based browsers:

It is possible that @jonsneyers might has more insights, since he works with the Jpeg XL team, this is just from some quick googles as I'm one of the web developers very annoyed by Google dropping this amazing codec, I really hope more browsers pick it up and we get can get this into mainstream use...

goodusername123 commented 1 year ago

I would like to note there is quite a large number of websites currently showing/shipping JXL files to users who support them due to the majority if not all sites made using Shopify having JPEG-XL support setup by default as a part of Shopify's automatic image optimization system/content delivery network.

jonathansampson commented 1 year ago

Relevant: https://bugs.chromium.org/p/chromium/issues/detail?id=1178058

jonsneyers commented 1 year ago

Summarizing some recent discussion in https://github.com/Alex313031/Thorium/issues/104:

Basically the libjxl team can maintain up-to-date rebased chromium patches that keep the jxl-chromium integration working also after M109. Then any chromium-derived browser that wishes to keep jxl support around (or even enable it by default, like Thorium did) should be able to do so relatively easily.

antonok-edm commented 1 year ago

I've opened up a draft PR for experimenting with this in Brave Browser.

With some manual effort, I was able to successfully apply the changes from Alex313031/thorium-libjxl into brave-core, build, and then view the images on the JPEG XL test page. 2023-03-22-222749_658x546_scrot

I believe there are still several issues that need to be resolved before we can consider merging:

Chromium versions

Brave follows upstream Chromium's release schedule very closely; Brave Nightly is built on Chromium 112.0.5615.29 as of 5 days ago. We've also been actively working on the upgrade to Chromium 113 since even earlier this month.

As far as I can see, the libjxl patches are only available for 111.0.5563.69. For my draft PR, I had to branch off of Brave 1.51.43, which was the last version on Chromium 111.

We'll definitely need to see a faster update cadence of the libjxl Chromium patches for this to be feasible.

Patching mechanism

The Alex313031/thorium-libjxl repo contains source files that are meant to be copied directly over Chromium's source files, in addition to two files that can't be copied directly because they would override other Thorium changes unrelated to JPEG XL.

Brave also has its own changes in some of the files relevant to JPEG XL. Copying directly is (as Thorium has encountered) not going to work for all source files, and manually syncing changes in some files is not a scalable solution.

Brave saves many of its own changes to Chromium source code as diffs in .patch files. I suspect that storing the JPEG XL changes as diffs would make sharing with other browsers much more tractable as well.

That being said, carrying around a directory of patch files has problems of its own (primarily integration with dev tooling, and likelihood of conflicts over time), which brings me to...

Patch structure

Brave already has a long history of patching Chromium and has developed an extensive set of "best practices", which we publish and keep updated here.

Brave Browser has ~12000 lines of patch files in total. My draft PR with the libjxl changes adds 3151 lines of patches1, which would inflate our count by more than 25%.

I don't intend to force any particular development practices on the libjxl team, especially considering that these patches wouldn't even be held in Brave's repo. However the changes will ultimately have to be applied to the Chromium source code either before or after Brave's own patches. I'm not personally part of Brave's Chromium rebase team, but patch application is a natural chokepoint for conflicts, and I know they'll take any amount of patch reduction they can get.

footnotes

[1] - granted, those patches are a result of my own naive diffing the Thorium files against the Chromium ones. Already at a quick glance, there are some irrelevant changes.

gz83 commented 1 year ago

I've opened up a draft PR for experimenting with this in Brave Browser.

With some manual effort, I was able to successfully apply the changes from Alex313031/thorium-libjxl into brave-core, build, and then view the images on the JPEG XL test page. 2023-03-22-222749_658x546_scrot

I believe there are still several issues that need to be resolved before we can consider merging:

Chromium versions

Brave follows upstream Chromium's release schedule very closely; Brave Nightly is built on Chromium 112.0.5615.29 as of 5 days ago. We've also been actively working on the upgrade to Chromium 113 since even earlier this month.

As far as I can see, the libjxl patches are only available for 111.0.5563.69. For my draft PR, I had to branch off of Brave 1.51.43, which was the last version on Chromium 111.

We'll definitely need to see a faster update cadence of the libjxl Chromium patches for this to be feasible.

Patching mechanism

The Alex313031/thorium-libjxl repo contains source files that are meant to be copied directly over Chromium's source files, in addition to two files that can't be copied directly because they would override other Thorium changes unrelated to JPEG XL.

Brave also has its own changes in some of the files relevant to JPEG XL. Copying directly is (as Thorium has encountered) not going to work for all source files, and manually syncing changes in some files is not a scalable solution.

Brave saves many of its own changes to Chromium source code as diffs in .patch files. I suspect that storing the JPEG XL changes as diffs would make sharing with other browsers much more tractable as well.

That being said, carrying around a directory of patch files has problems of its own (primarily integration with dev tooling, and likelihood of conflicts over time), which brings me to...

Patch structure

Brave already has a long history of patching Chromium and has developed an extensive set of "best practices", which we publish and keep updated here.

Brave Browser has ~12000 lines of patch files in total. My draft PR with the libjxl changes adds 3151 lines of patches1, which would inflate our count by more than 25%.

I don't intend to force any particular development practices on the libjxl team, especially considering that these patches wouldn't even be held in Brave's repo. However the changes will ultimately have to be applied to the Chromium source code either before or after Brave's own patches. I'm not personally part of Brave's Chromium rebase team, but patch application is a natural chokepoint for conflicts, and I know they'll take any amount of patch reduction they can get.

footnotes

[1] - granted, those patches are a result of my own naive diffing the Thorium files against the Chromium ones. Already at a quick glance, there are some irrelevant changes.

@antonok-edm

I don't know whether brave needs to update the DEPS file. The modification of the DEPS file in Thorium's libjxl repo is not necessarily suitable for brave. In addition, there are image simulations in devtools that may also need to be restored.

At the same time, to restore libjxl also need to process the decode.h file in the libjxl code tree, otherwise there will be compilation errors.

In addition, Thorium has recently updated the minor version number again (from 69 to 111), and the patches in the ibjxl repo (two patch files, only used as a reference for developing the Thorium browser) also need to be updated.

Finally, it should be noted that the current patch may still cause some tests (such as unit tests) to fail, and we are still waiting for upstream to fix them.

jonsneyers commented 1 year ago

Looks like the OpenMandriva linux distribution is also shipping a chromium with jxl support restored: https://github.com/OpenMandrivaAssociation/chromium-browser-stable/blob/master/chromium-restore-jpeg-xl-support.patch

Having patches according to best practices would indeed be good. I also agree that it would be good to keep the patch size small. One way to reduce the patch size is to not make it optional anymore. I think quite a bit of lines of patches are there because things are conditional on the flag being enabled. Also I think some things like unit tests possibly don't need to be part of the main patch set (can have those separately, only for debugging purposes, or something, I dunno what the best practices are regarding that).

antermin commented 1 year ago

As Apple has added JPEG XL support in Safari 17 Beta, I hope Brave developers will be more willing to support this format.

antonok-edm commented 1 year ago

@antermin we are interested, but the issues I outlined in https://github.com/brave/brave-browser/issues/28411#issuecomment-1480668264 when building the initial proof of concept are still blockers for integration in Brave.

Just to make sure there's visibility - @gz83 @mo271 @midzer @jonsneyers @gz83 @Alex313031 is the above actionable so far?

midzer commented 1 year ago

@antonok-edm There is https://github.com/Alex313031/thorium-libjxl now, so it should be doable for any chromium based browser. For more info about the process, I have to refer to @Alex313031 and @gz83.

Edit: Oooops, didnt read the whole thread. Linked repo is alread mentioned here...

gz83 commented 1 year ago

I think you can try to apply the two patch files to the brave browser according to the license requirements of the thorium-libjxl repo and the instructions in the README file. Currently, the two patch files in the thorium-libjxl repo have been rebased to the M113 version.

In the future, we intend to update the versions of libjxl and highway, and synchronize the upstream update of the two patch files.

If you guys need any help with this issue feel free to ping me.

@antonok-edm

goodusername123 commented 1 year ago

I think one of the main concerns was patch size since it is quite large and there is a bunch of stuff that could be removed from it like @jonsneyers point out such as ifdefs, flag page support(?) and possibly the unit tests, however I'm not sure if anyone has made a reduced/minimized version of the patch(s) yet.

mo271 commented 1 year ago

fyi, I think the thorium patch is based on https://chromium-review.googlesource.com/c/chromium/src/+/4255409 which I suppose could also be adapted directly and a minimal patch can be made not adding all the testing (although it might be good to do some testing...) and the flag, ifdefs etc.

One think to keep in mind when adding libjxl is the highway version. Highway is already in chrome since another part of chrome uses it. The patch linked above also bumps highway from 1.0.3 to 1.0.4, but I suppose one could also work with a libjxl-version to works well with 1.0.3 and don't bump it. https://source.chromium.org/chromium/chromium/src/+/main:third_party/highway/README.chromium

gianni-rosato commented 8 months ago

The privacy-oriented Cromite browser now also features JPEG-XL support. Source: https://github.com/uazo/cromite/issues/351#issuecomment-1780530654

kremzli commented 4 months ago

Whats the status on this? Many sites and CDN's want and do use JXL already, and with JXL saliency progressive decoding, Brave would look like it loads images instantly, and use less bandwidth too

ballo commented 3 months ago

Google lost. It's only a matter of time before they're dragged kicking and screaming. No reason for Brave not to support it

antermin commented 3 months ago

@ballo

Are you perhaps referring to a recent Reddit thread linking to a 4chan thread which claims that "JPEG XL Won. Some sites are starting to implement JXL image delivery in their backend, filling in for AVIF as a higher-quality alternative"?

As one of the comments pointed out, some Cloudinary customers enabled JPEG XL, so e.g. Safari users visiting those sites will be served JPEG XL.

At the time of writing, @mo271's patch (4255409) is still in the status of "Merge Conflict" "Work in Progress", and the Chromium team has not merged it.

ballo commented 3 months ago

@ballo

muh plebbit muh 4chins

I don't use either. JXL won based on merit and Google is dragging their ass because they're invested in webp and av1 (literally). Google likes to think the entire webternet is their "ecosystem" but outside YouTube (a site of waning importance) and 4chans, nobody adopted vp9. People are waking up to amp and other shenanigans, too (calling Dr. Epstein). https://www.youtube.com/watch?v=w7UDJUCMTng

antermin commented 2 months ago

Some days ago (Apr 16), @mo271 posted this on JPEG XL Discord:

I made a fresh patch for chrome: https://chromium-review.googlesource.com/c/chromium/src/+/5454316 so that chromium based browsers can use it. Please let the developers of those browsers know, and let me how we can support them...

Will his patch make it easier to restore JPEG XL support in Brave Browser?

gz83 commented 2 months ago

If the brave team can be more specific with some details on how to contribute patches (whether it can be in the form of pure .patch files), then I can help brave restore libjxl support.