Bug 229792

Summary: <video> element rendered incorrectly when provided with a portrait orientation stream in Safari 15
Product: WebKit Reporter: Alexander Khovansky <alex>
Component: WebRTCAssignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: daginge, davy.de.durpel, eric.carlson, ews-watchlist, glenn, hta, jer.noble, philipj, rex, sandabu.www, sergio, teodor.atroshenko, tommyw, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: iPhone / iPad   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=231059
Attachments:
Description Flags
Bug screenshot in codepen debug view
none
Same demo running on iOS 14.7.1
none
Basic peer connection demo
none
Patch
none
Patch
none
Use new testRunner API none

Description Alexander Khovansky 2021-09-01 21:51:14 PDT
Created attachment 437113 [details]
Bug screenshot in codepen debug view

<video> element with vertical video appears to be rendered with a black landscape-oriented rectangle behind it.
First noticed when using getUserMedia on iPhone in iOS 15 (19A5340a). Same issue is reproducible on iPadOS 15 (same build) when used in portrait mode.

Reproduction codepen: https://codepen.io/alx-khovansky/pen/PojNgqR
Attached a screenshot taken in codepen debug view, notice the black rectangle sticking from the sides of the video.
Comment 1 Alexey Proskuryakov 2021-09-05 13:33:38 PDT
> First noticed when using getUserMedia on iPhone in iOS 15 

Could you please confirm whether this behaves as expected on iOS 14.x?
Comment 2 Radar WebKit Bug Importer 2021-09-05 13:33:52 PDT
<rdar://problem/82776741>
Comment 3 Alexander Khovansky 2021-09-05 18:44:11 PDT
Created attachment 437373 [details]
Same demo running on iOS 14.7.1

Attached a new screenshot: same demo running on iOS 14.7.1.
Video is rendered normally.
Comment 4 Naoto Takahashi 2021-09-08 22:25:21 PDT
I'm facing the same problem.

I found that setting the object-fit of the video element to none temporarily solves the problem, however it changes that setting.

The black rectangle disappears when there is inline content to the left of the video, so adding transparent colored text with a negative margin seems to be a temporary workaround.

CodePen :
https://codepen.io/sandabu/pen/xxrRZqx
Comment 5 daginge 2021-09-15 02:22:47 PDT
I can reproduce this in the basic getUserMedia test over on webrtc samples as well. 

https://webrtc.github.io/samples/src/content/getusermedia/gum/

Tested on iPhone XR MRY42QN/A running 15.0 Public Beta 8 (19A5340a)
Comment 6 daginge 2021-09-15 06:21:40 PDT
Created attachment 438241 [details]
Basic peer connection demo

Shows that this is broken also in basicPeerConnection demo for both local and remote video in portrait mode
Comment 7 daginge 2021-09-15 06:43:07 PDT
Got an update to 19A344, still not working
Comment 8 daginge 2021-09-21 01:48:30 PDT
This hit production now on iOS 15 (19A346) and it's still as broken as before.
Comment 9 youenn fablet 2021-09-22 04:11:26 PDT
Created attachment 438940 [details]
Patch
Comment 10 youenn fablet 2021-09-22 13:47:42 PDT
Created attachment 438972 [details]
Patch
Comment 11 Eric Carlson 2021-09-23 06:25:07 PDT
Comment on attachment 438972 [details]
Patch

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

> Source/WebCore/ChangeLog:10
> +        We also need to ompute m_sampleBufferDisplayLayer position based on m_rootLayer coordinates.

s/ompute/compute/

> LayoutTests/fast/mediastream/video-rotation.html:22
> +    await new Promise(resolve => setTimeout(resolve, 3000));

This hard coded wait is unfortunate because it will almost always be way too long, but it can presumably be too short on a very heavily loaded machine. Can we avoid it by doing something like rendering to a canvas until we see that someone has been rendered?
Comment 12 youenn fablet 2021-09-23 07:00:56 PDT
(In reply to Eric Carlson from comment #11)
> Comment on attachment 438972 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=438972&action=review
> 
> > Source/WebCore/ChangeLog:10
> > +        We also need to ompute m_sampleBufferDisplayLayer position based on m_rootLayer coordinates.
> 
> s/ompute/compute/
> 
> > LayoutTests/fast/mediastream/video-rotation.html:22
> > +    await new Promise(resolve => setTimeout(resolve, 3000));
> 
> This hard coded wait is unfortunate because it will almost always be way too
> long, but it can presumably be too short on a very heavily loaded machine.
> Can we avoid it by doing something like rendering to a canvas until we see
> that someone has been rendered?

Hum, we can try to improve things.
A canvas might not be good enough since it is not tightly related to the rendering code path.
But I can add a test runner API that gets a screenshot and we can then do some measurements on it (and redo screenshots as needed).
Comment 13 youenn fablet 2021-09-24 03:14:23 PDT
Created attachment 439138 [details]
Use new testRunner API
Comment 14 EWS 2021-09-24 04:13:40 PDT
Committed r283035 (242095@main): <https://commits.webkit.org/242095@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439138 [details].
Comment 15 youenn fablet 2021-10-11 09:08:21 PDT
*** Bug 231461 has been marked as a duplicate of this bug. ***
Comment 16 youenn fablet 2021-10-11 09:08:48 PDT
This should be fixed in latest iOS 15 beta.
Comment 17 youenn fablet 2021-10-11 09:11:36 PDT
(In reply to youenn fablet from comment #16)
> This should be fixed in latest iOS 15 beta.

I take it back actually.
Comment 18 Alexander Khovansky 2021-10-11 19:16:31 PDT
Looked fixed to me on 15.1 beta 3.
Comment 19 youenn fablet 2021-10-20 03:51:12 PDT
*** Bug 231903 has been marked as a duplicate of this bug. ***
Comment 20 youenn fablet 2021-10-21 01:43:17 PDT
*** Bug 232039 has been marked as a duplicate of this bug. ***