Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cocoa] Support HEIC/HEIF images for Apple ports #3731

Conversation

shallawa
Copy link
Contributor

@shallawa shallawa commented Aug 27, 2022

d9e697a

[Cocoa] Support HEIC/HEIF images for Apple ports
https://bugs.webkit.org/show_bug.cgi?id=244424

Reviewed by NOBODY (OOPS!).

Patch by Said Abou-Hallawa <said@apple.com>.

Add the mime type and the UTI of the HEIC/HEIF to the list of the allowed image
formats. The system frameworks will be used to render the HEIC/HEIF images on
Apple ports. Because of sand-boxing limitations, software decoding has to be used
for these images.

* Source/WTF/wtf/PlatformHave.h:
* Source/WebCore/loader/cache/CachedResourceRequest.cpp:
(WebCore::acceptHeaderValueForAVIFImageResource):
(WebCore::acceptHeaderValueForHEICImageResource):
(WebCore::acceptHeaderValueForImageResource):
* Source/WebCore/platform/MIMETypeRegistry.cpp:
* Source/WebCore/platform/graphics/cg/UTIRegistry.cpp:
(WebCore::defaultSupportedImageTypes):
(WebCore::setAdditionalSupportedImageTypes):

d9e697a

Misc iOS, tvOS & watchOS macOS Linux Windows
βœ… πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ§ͺ bindings βœ… πŸ›  ios-sim βœ… πŸ›  mac-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl ⏳ πŸ§ͺ ios-wk2 βœ… πŸ›  mac-AS-debug βœ… πŸ§ͺ gtk-wk2
⏳ πŸ§ͺ api-ios   πŸ§ͺ api-mac βœ… πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk1 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-wk2 βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  watch   πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ›  jsc-mips
βœ… πŸ›  watch-sim βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ§ͺ jsc-mips-tests

@shallawa shallawa requested a review from cdumez as a code owner August 27, 2022 05:49
@shallawa shallawa self-assigned this Aug 27, 2022
@shallawa shallawa added Images For bugs in image handling. WebKit Nightly Build labels Aug 27, 2022
@webkit-early-warning-system

This comment was marked as outdated.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 27, 2022
@mcatanzaro
Copy link
Contributor

Supporting new patent-encumbered formats in WebKit in 2022 is totally unacceptable. Please do not do this. This is unhealthy for the web, and creates new web compat risk for WebKit ports.

@litherum
Copy link
Contributor

litherum commented Oct 25, 2022

Supporting new patent-encumbered formats in WebKit in 2022 is totally unacceptable.

This patch doesn't indicate whether or not the feature will ship or not; the HAVE(HEIC) flag separates policy from implementation.

I'll start a discussion in the WebKit Slack workspace about HEIC on the web with you + a few other interested parties.

@litherum litherum assigned litherum and unassigned shallawa Oct 25, 2022
@litherum
Copy link
Contributor

litherum commented Oct 25, 2022

Oh, we shouldn't land this patch yet because tests are failing.

@litherum litherum removed the merging-blocked Applied to prevent a change from being merged label Oct 25, 2022
@litherum litherum force-pushed the eng/Cocoa-Support-HEICHEIF-images-for-Apple-ports branch from 614d9d6 to bc7c4e4 Compare October 25, 2022 09:04
https://bugs.webkit.org/show_bug.cgi?id=244424

Reviewed by NOBODY (OOPS!).

Patch by Said Abou-Hallawa <said@apple.com>.

Add the mime type and the UTI of the HEIC/HEIF to the list of the allowed image
formats. The system frameworks will be used to render the HEIC/HEIF images on
Apple ports. Because of sand-boxing limitations, software decoding has to be used
for these images.

* Source/WTF/wtf/PlatformHave.h:
* Source/WebCore/loader/cache/CachedResourceRequest.cpp:
(WebCore::acceptHeaderValueForAVIFImageResource):
(WebCore::acceptHeaderValueForHEICImageResource):
(WebCore::acceptHeaderValueForImageResource):
* Source/WebCore/platform/MIMETypeRegistry.cpp:
* Source/WebCore/platform/graphics/cg/UTIRegistry.cpp:
(WebCore::defaultSupportedImageTypes):
(WebCore::setAdditionalSupportedImageTypes):
@litherum litherum force-pushed the eng/Cocoa-Support-HEICHEIF-images-for-Apple-ports branch from bc7c4e4 to d9e697a Compare October 25, 2022 09:09
@litherum
Copy link
Contributor

Michael has made a convincing argument that this is harmful to the web. Closing as WONTFIX.

https://webkit.slack.com/archives/CU64U6FDW/p1666701570103969?thread_ts=1666686680.288109&channel=CU64U6FDW&message_ts=1666701570.103969

@danielcompton
Copy link

@litherum would you be able to copy the argument from Slack to here or https://bugs.webkit.org/show_bug.cgi?id=244424?

@mcatanzaro
Copy link
Contributor

mcatanzaro commented Dec 6, 2022

mcatanzaro
1 month ago
The problem I fear is: in the past, with JPEG 2000, when Safari supported it, websites started to actually depend on it... but only when the user agent header looked like Safari

mcatanzaro
1 month ago
So we were eventually forced to add support for JPEG 2000 to WebKitGTK to keep the web working, because changing the user agent header was not an option

mcatanzaro
1 month ago
The risk is a little lower with HEIC because you advertise support for that format via HTTP headers, which was not the case for JPEG 2000, so hopefully websites will look at that instead of the user agent header

mcatanzaro
1 month ago
Another reason the risk is lower: AVIF exists today, so why would anybody want to use HEIC when AVIF works everywhere and HEIC is Apple-specific? (Surely no other browsers are going to support HEIC.)

mcatanzaro
1 month ago
But the consequences if websites do start depending on HEIC when they see WebKit are far more dire than with JPEG 2000, because JPEG 2000 was free to implement, whereas it is illegal to implement HEIC without paying for it...

mcatanzaro
1 month ago
So patented stuff is very high risk stuff for us. We still in 2022 cannot handle H.264 very well because we cannot ship OpenH264 updates directly to users, all updates have to go through Cisco, which pays $$$$$$$ millions per year for the right... it's very expensive. H.265 is completely out of reach due to much higher cost: it's just not going to happen.

mcatanzaro
1 month ago
I don't see any point in adding support for a new image format that other browsers will not implement

mcatanzaro
1 month ago
We were just discussing AVIF yesterday (coincidence) which seems to have gained traction as the next-gen image format for browsers; I'll be trying to turn that on for WebKitGTK now since I see Safari and all major browsers have shipped it. There is also JPEG XL, which we support behind the experimental feature build flag; I hear it is better, but doesn't seem to have traction with browsers for whatever reason. (edited)

mcatanzaro
1 month ago
Also I'll note that Apple is a member of the Alliance for Open Media, whose primary goal is to crush H.265/HEIC before they threaten the web like H.264 already has for so long. Supporting HEIC in WebKit is inconsistent with that effort.

mcatanzaro
1 month ago
I'd propose a simple requirement for new image and video formats in WebKit: supporting the format should be legal without paying $$$$$$$.

mcatanzaro
1 month ago
By the way, Fedora Legal has banned even source code that implements patented algorithms, out of fear that distributing the source code could be considered inducement to infringe patent rights, which is also illegal. That's why the existing support for H.264 goes via plugins (GStreamer elements) that aren't actually directly part of WebKit itself.

mcatanzaro
1 month ago
It's seriously annoying. :( OK, there's my take on it. No good can come from this, just use AVIF instead. ;)

mcatanzaro
1 month ago
And if AVIF isn't good enough for whatever reason, maybe look at JPEG XL (which is "supported" in Chrome/Firefox/WebKitGTK behind a flag that's off by default). No need for encumbered formats; a future where you need to pay up to display images just seems awful, and if that's not the goal, then there's no reason to support it....

The rest of the conversation is basically me complaining about the related topics of H.264, which is still causing serious problems, and H.265, which is absolutely terrifying. I really wish H.265 support would be restricted to native apps only so that websites cannot depend on it. Asides from EME, which we can't do much about, H.265 support in Safari is the biggest threat to non-Safari WebKit right now.

@shallawa shallawa deleted the eng/Cocoa-Support-HEICHEIF-images-for-Apple-ports branch May 10, 2023 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Images For bugs in image handling.
Projects
None yet
6 participants