Bug 218637 - Textures Fail to Render in WebGL from HLS Stream on iPhone 12 [iOS 14.2]
Summary: Textures Fail to Render in WebGL from HLS Stream on iPhone 12 [iOS 14.2]
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Other
Hardware: iPhone / iPad Other
: P2 Critical
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on: 215908
Blocks: 216915 218970 218971 219026
  Show dependency treegraph
 
Reported: 2020-11-05 15:29 PST by Tony Tomarchio
Modified: 2020-11-17 01:38 PST (History)
15 users (show)

See Also:


Attachments
Image comparing HLS WebHL playback on iPhone12 vs iPhone11 in iOS 14.2 (2.07 MB, image/png)
2020-11-05 15:29 PST, Tony Tomarchio
no flags Details
Patch (22.73 KB, patch)
2020-11-12 05:44 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff
Patch (8.76 KB, patch)
2020-11-16 00:21 PST, Kimmo Kinnunen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (8.76 KB, patch)
2020-11-16 00:39 PST, Kimmo Kinnunen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Tomarchio 2020-11-05 15:29:35 PST
Created attachment 413357 [details]
Image comparing HLS WebHL playback on iPhone12 vs iPhone11 in iOS 14.2

As a follow-up to https://bugs.webkit.org/show_bug.cgi?id=215908 HLS WebGL playback is still broken on iPhone 12 devices in today's iOS 14.2 release (18B92)

*Examples to Reproduce Issue*

Device: iPhone 12

three.js example: Visit https://8w.8thwall.app/hls-threejs on an iPhone 12 running iOS 14.2 (18B92). The rotating cube has a video texture applied from an HLS stream. The video in the top left corner is using the same HLS stream but rendering on top of the page outside WebGL.

babylon.js example: Visit https://www.babylonjs-playground.com/#9FSDC7#49 on iPhone 12 running iOS 14.2. The plane has a video texture applied from an HLS stream.

Both examples work correctly on:
iPhone 11 (Model MWLC2LL/A) running iOS 14.2 (18B92)
iPhone SE (Model MX9Q2LL/A) running iOS 14.2 (18B92)
Comment 1 Alexey Proskuryakov 2020-11-05 17:45:00 PST
I can reproduce this in iPhone 12 Pro.
Comment 2 Radar WebKit Bug Importer 2020-11-05 17:45:19 PST
<rdar://problem/71102126>
Comment 3 Kimmo Kinnunen 2020-11-11 08:36:27 PST
On iPhone 12 fails with: 
Asked to copy an unsupported pixel format ('&8v0').
when interpreting the result of CVPixelBufferGetPixelFormatType.
The supported ones are '420v' and '420f'. Unsure if the above pixel format is real fourcc format or just misinterpretation of OSType enumeration.
Comment 4 Kimmo Kinnunen 2020-11-11 11:00:11 PST
And to clarify:
Not all WebGL video copies fail. It appears that at least the bunny one fails, but at least one other test case in bug 215908 succeeds
Comment 5 Kenneth Russell 2020-11-11 12:18:38 PST
Kimmo, since you've triaged this already could you officially take it? Unfortunately James and I can't feasibly debug anything that happens only on real iOS hardware.
Comment 6 Kimmo Kinnunen 2020-11-12 05:44:52 PST
Created attachment 413929 [details]
Patch
Comment 7 Kenneth Russell 2020-11-12 11:55:47 PST
Comment on attachment 413929 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=413929&action=review

Very nice and well thought through fix. It looks good to me, especially given you've verified the fix on hardware - two small comments above. However I think it would be best if jer.noble reviewed this too and approved it.

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2216
> +        (__bridge NSString *)kCVPixelBufferIOSurfacePropertiesKey : @{ /*empty dictionary*/ },

Just checking - it's okay to specify this property for all video playback?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateAVFoundationObjC.mm:2363
> +    CVPixelBufferRef pixelBuffer = updateLastPixelBufferForWebGL();

One note - the software upload path into GraphicsContext goes through the call chain paintCurrentFrameInContext -> paintWithVideoOutput -> updateLastImage, which calls updateLastPixelBuffer. However, this doesn't have the same requirements on the pixel buffer format that copyVideoTextureToPlatformTexture does, so that call to updateLastPixelBuffer doesn't need to be updated to updateLastPixelBufferForWebGL.
Comment 8 Kenneth Russell 2020-11-13 10:42:48 PST
Comment on attachment 413929 [details]
Patch

Since other reviewers seem busy, let me set r+ since you've tested this locally and it fixes a major regression. Please set cq? if you'd like me to cq+ it too.
Comment 9 Kimmo Kinnunen 2020-11-16 00:21:53 PST
Created attachment 414198 [details]
Patch
Comment 10 Kimmo Kinnunen 2020-11-16 00:39:53 PST
Created attachment 414199 [details]
Patch
Comment 11 Kimmo Kinnunen 2020-11-16 00:50:58 PST
(In reply to Kenneth Russell from comment #8)
> Comment on attachment 413929 [details]
> Patch
> 
> Since other reviewers seem busy, let me set r+ since you've tested this
> locally and it fixes a major regression. Please set cq? if you'd like me to
> cq+ it too.

Thanks for the review.
Jer explained to me that using PixelBufferTransferSession is a software fallback. I moved the implementation to bug 218971 where we should improve the software fallback, since currently it's not really working (contributing to two regressions in iOS releases).

I uploaded Jer's patch for treating the currently frequent-but-not-working format similar to a format we already support.
Comment 12 EWS 2020-11-16 21:23:46 PST
Committed r269893: <https://trac.webkit.org/changeset/269893>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 414199 [details].