Bug 211229 - Disable low latency encoder on iOS
Summary: Disable low latency encoder on iOS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-04-30 06:13 PDT by youenn fablet
Modified: 2020-04-30 11:54 PDT (History)
9 users (show)

See Also:


Attachments
Patch (1.73 KB, patch)
2020-04-30 06:15 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2020-04-30 06:13:34 PDT
Disable VCP on iOS until ready
Comment 1 youenn fablet 2020-04-30 06:15:33 PDT
Created attachment 398050 [details]
Patch
Comment 2 EWS 2020-04-30 11:36:43 PDT
Committed r260956: <https://trac.webkit.org/changeset/260956>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 398050 [details].
Comment 3 Radar WebKit Bug Importer 2020-04-30 11:38:04 PDT
<rdar://problem/62662886>
Comment 4 Darin Adler 2020-04-30 11:54:53 PDT
When possible we should try to make more careful use of HAVE vs. USE and Platform headers vs. SPI headers vs. SoftLink headers.

It’s a little peculiar to define a HAVE in an SPI header. Seems even more peculiar to have it in a SoftLink header.

Our decision here seems to be about whether to *use* something, not whether it’s present, so that sounds like USE instead of HAVE. In this situation, I might have changed the conditionals in the code to USE instead of HAVE, and turned it off that way.

And moved the HAVE to a PlatformHave header, out of the SoftLink header.

And defined USE to a PlatformUse header with a default of 0.

There may be arguments agains this such as:

- This small change is effective and low risk; doing a bigger change instead is messy and risky.

- It’s nice to concentrate information about whether the platform has something with the header for that thing, so annoying that the HAVE macro is supposed to go in one of the PlatformHave headers.

- For something that’s not ready at all yet, seems unnecessary to have it in a PlatformUse header.

But I also think it’s valuable to separate HAVE and USE, and separate configuration and platform capability information from the headers with specific classes and functions.

I suggest that when we bring this macro back we do it in the platform machinery, not by adding it back to the SoftLink header.