[iOS 12.1, iPhone XS/XS Max/XR] TVIVideoView displays video with incorrect rotation

JiyarMahmod commented 6 years ago

Iam having some trouble with the twilio video previewView functions. the view renders the local video frames from the front camera at viewdidload like normal but in ios 12.1 the view rotates 45 degrees if you set it to shouldmirror. But if you dont flip the view so that it mirrors then it is working and is not automatically rotating 45 degrees and everything becomes turned around so that left is right and right is left.. its really weird but this happend just when i updated my phone and then i tryed it on my other phone that has ios 12.0 and there it works fine.

Steps to Reproduce

I have tryed to rotate the view back 45 degrees. [no success]

2.I have tryed to not flip the camera and just let it be. but then the preview is really ugly when it is not mirrored [some success]
tryed it on a older ios update. [Success]

Code

// The Code is the normal code to flip the camera in the delegate like this 
extension ViewController : TVICameraCapturerDelegate {
    func cameraCapturer(_ capturer: TVICameraCapturer, didStartWith source: TVICameraCaptureSource) {
        self.previewView.shouldMirror =  (source == .frontCamera)
    }
}

Expected Behavior

I expected the local previewView to not rotate 45 degrees and show a normal preview like it allways have done

Actual Behavior

The preview view rotates 45 degrees if previewView.shouldMirror = true. If it is set to false it will work but look really ugly so the behavior is not correct at all. the code above is just like setting shouldMirror to true

Reproduces How Often

Allways on Iphone xs max iOS 12.1

Logs

// There is no log error everything is normal with the code...

Video iOS SDK

TwilioVideo iOS SDK Version 2.5

Xcode

Xcode Version 10.1 (10B61)

iOS Version

iOS 12.0 [Success] iOS 12.1 (New update) [no success]

iOS Device

Tested on an IPhone XS Max and on a IPhone 7

ceaglest commented 6 years ago

Hi @JiyarMahmod,

I just tried this on our QuickStart using an iPhone X, iOS 12.1 and version 2.5.3, and I couldn't reproduce the issue. We do use mirroring in the example app when you select the front camera.

Is it reproducible on the sample apps for you as well? Could you provide a video or screenshot to demonstrate the 45 degree rotation? I will give this a shot on some other test devices.

Regards, Chris

JiyarMahmod commented 6 years ago

Hmm thats weird... @ceaglest I Will try this on a iphone x as well, are you sure you have the latest update on the device? IOS 12.1 i think The error comes from there but sure iwill fix a screenhot

JiyarMahmod commented 6 years ago

This is the weirdest thing... On the screenshot The view is rotated correct but no problem i Will show You a picture

JiyarMahmod commented 6 years ago

Here is a photo of my iphone xs max from my iphone 7 and both phones are in portrait mode and iam holding both phones standing, not on the side https://imgur.com/a/yvUTlTA

piyushtank commented 6 years ago

@JiyarMahmod Thanks for posting a picture. Can you answer following questions in order to debug the problem -

Are you able to reproduce the problem on any device other than iPhone XS Max ?
Are you able to reproduce the problem on our QuickStart app ?

JiyarMahmod commented 6 years ago

okey so i have done some testing and i have come to conclussion that it is not the update and it is all the iphone xs max. the view rotates on the quickstart version aswell but i tryed on a other iphone x (IOS 12.1) and there were no problemes on the iphone x so i guess it is not the ios update and it is something wrong with the iphone xs. and i dont know if it is only my iphone xs max that is doing this or if it is all the xs max's, so it would be really nice if you guys could test this with an iphone xs max with the newest update 12.1 because before i updated the phone it was all working great. so i guess it has something to do with the update on the xs max. Because the error does not appen when i do it on any other device that is on ios 12.1 aswell. @piyushtank @ceaglest

JiyarMahmod commented 6 years ago

@piyushtank @ceaglest , I fixed the problem with some wierd way so let me walk you through it and maybe you can fix the issue if it is from the sdk. so what i did first was try to rotate the viewback 45degrees and that didnt work because the uiview was not rotate itself it was the rendered fotage that had been rotated so when i tryed to rotate it back the the rendered fotage became normal but the uiview now had a 45 degree rotation on itself and that didnt look really nice. so i understood that it had to be updated or something must happen with the uiview itself. so i connected a new outlet with the previewView to a UIView and not TVIVideoView but i keept the TVIVideoView outlet so that the sdk could the its work. but then instead of rotating the uiview i just devided the corner radius from the uiview outlet and not the TVIVideoView outlet and poof for some reason it worked hahah... thanks for your time anyways guys.

piyushtank commented 6 years ago

Thanks @JiyarMahmod We are going to try reproducing the problem on iPhone XS Max and let you know our findings.

lukaspili commented 5 years ago

Hello, I'm seeing the same issue with the unmodified VideoQuickStart project, ios 12.1, iphone XR.

JiyarMahmod commented 5 years ago

@lukaspili write previewView.frame.cornerRadius = previewView.frame.size.width/16 or 32 and it Will be fixed depending on how Much cornerradius You want to remove

paynerc commented 5 years ago

@JiyarMahmod ,

I am seeing that on my Xs Max running iOS 12.1, I am experiencing a 90º rotation of the preview view. I am not sure that I am following the cornerRadius workaround. How and where are you setting this?

Thanks,

Ryan

piyushtank commented 5 years ago

@JiyarMahmod and @lukaspili for reporting the problem. The rotation bug is reproducible only on A12 (iPhone XS/XR/XSPlus) devices running iOS 12.1. The bug is not reproducible on the same set of devices running iOS 12.0.1.

I apologize for the inconvenience.

We have prioritized this issue and looking into the problem in SDK. Following workarounds I figured fixes the problem -

By commenting out the mirroring on previewView.
Thanks @JiyarMahmod for suggesting this workaround. Set previewView.layer.cornerRadius = previewView.frame.size.width/16 in viewDidLoad()

(Another workaround is creating TVIVideoView programmatically with rendering type OpenGL ES, however I am keeping this as not recommended option as OpenGL ES rendering is deprecated on iOS 12 and on latest Video SDK)

We are investigating our Metal rendering pipeline to make sure it is not causing the issue. I have created a bug CSDK-2272 for internal tracking.

We will be out for ThanksGiving break for the rest of the week but we will resume working on this issue next week.

Best, Piyush

JiyarMahmod commented 5 years ago

@paynerc sorry for the late response but in the viewdidload function

piyushtank commented 5 years ago

@JiyarMahmod and @lukaspili After some experimentation, I noticed that the problem is in the local side. When I used the same preview view for remote rendering, it worked as expected. So using the camera's preview view instead is the best work around until we fix the problem in our SDK.

Following is the code snippet. We haven't had any success in reproducing this bug for the remote previews.

Make sure previewView is not a TVIVideoView. you have to update the storyboard as well.
```
@IBOutlet weak var previewView: UIView!
```

Initiate camera with enablePreview arg set to true, and add camera.previewView as subview to the UIView instantiated by storyboard in step 1. Change this to following -

    if let camera = TVICameraCapturer(source: .frontCamera, delegate: self, enablePreview: true) {
        localVideoTrack = TVILocalVideoTrack.init(capturer: camera, enabled: true, constraints: nil, name: "Camera")
        if (localVideoTrack == nil) {
            logMessage(messageText: "Failed to create video track")
        } else {
            // Add renderer to video track for local preview
            camera.previewView.frame = self.previewView.bounds;
            self.previewView.addSubview(camera.previewView)

            logMessage(messageText: "Video track created")

            // We will flip camera on tap.
            let tap = UITapGestureRecognizer(target: self, action: #selector(ViewController.flipCamera))
            camera.previewView.addGestureRecognizer(tap)
        }
    }

piyushtank commented 5 years ago

@JiyarMahmod and @lukaspili While we are investigating SDK's capturer and rendering pipeline, I have created a branch with the workaround of using camera.previewView in the last post. Here is the branch - https://github.com/twilio/video-quickstart-swift/tree/rotation-workaround

ceaglest commented 5 years ago

Hi folks,

Just to add one more point to this discussion, you can also try AudioSinkExample for more runnable code which uses TVICameraPreviewView.

I am looking into this bug today, reassigning the ticket to myself.

Regards, Chris

ceaglest commented 5 years ago

Hello,

After investigating this issue I determined that using CGAffineTransform with MTKView is not reliable on impacted devices running iOS 12.1.0. The solution I've implemented is to account for orientation and mirroring as a part of the Metal rendering process itself.

We hope to release a fix as soon as possible in our next 2.5.5 update.

Best, Chris

lukaspili commented 5 years ago

Thanks for the quick and thorough updates on the issue @piyushtank @ceaglest, much appreciated.

ceaglest commented 5 years ago

We've landed the bug fix, and I am releasing it to internal stakeholders for further testing. Expect it to be included in the 2.5.5. release later this week.

ceaglest commented 5 years ago

Hi @JiyarMahmod and @lukaspili,

We have just released 2.5.5 with the Metal rendering bug fix. Please give it a try in your applications.

Thanks, Chris

ceaglest commented 5 years ago

Hi,

Just checking in, did 2.5.5 resolve the bug for you @JiyarMahmod?

Best, Chris

ceaglest commented 5 years ago

Closing, since this bug has been fixed for a few releases now. Please reopen if you're still able to reproduce Metal rendering issues on these devices / OS version.

teslasleep commented 5 years ago

Hi all.

@ceaglest, @piyushtank

I still see this problem on iPhone XR iOS 12.1 (16B94). The preview is rotated at 90º when I set shouldmirror=true for TVIVideoView instance.

Xcode Version 10.1 (10B61) Twilio SDK Version "2.6.0"

Tested on:

iPhone XR - has problem
iPhone 6 - no problems

piyushtank commented 5 years ago

@teslasleep Sorry for responding late.

We have fixed this problem in version 2.5.5 (changelogs). And between 2.5.5 and 2.6.0 I am not aware of any change that could cause this problem, and also, no other customer has reported this specific issue after 2.5.5 was released.

We will try it out locally to make sure nothing has broken, but I was wondering, is it possible that the app is using an old cached version of the SDK? You can validate this by checking the sdk version by API [TwilioVideo version].

Best, Piyush

teslasleep commented 5 years ago

@piyushtank Thanks, the problem was in the cached version of the framework.

twilio / video-quickstart-ios