gorastudio-git / SCNRecorder

The best way to record your AR experience!
MIT License
201 stars 51 forks source link

Wrong color profile on iOS 12, iPhone 6 device when recording at high resolution. #45

Open giomurru opened 2 years ago

giomurru commented 2 years ago

Hi, I encountered some problems with the output colors while using SCNRecorder on iPhone 6 running iOS 12.5.5. iPhone X and iPad 9th gen on iOS 15.5 are fine though. The recording made with iOS 12.5.5 and iPhone 6 has dull colors that appears less saturated than normal. I found that the issue can be solved if we do not set a custom value for the AVVideoColorPropertiesKey and we leave this key to its default value. I created a pull-request which includes this fix.

v-grigoriev commented 2 years ago

Hi @giomurru I added comment to your PR.

Unfortunately I don't have such device and iOS version to reproduce.

Can you tell me what pixel format was used? Did you record SCNView or ARSCNView or something else? Is this issue reproducible with the Example app on the same device?

giomurru commented 2 years ago

Hi @v-grigoriev, I tried the Example app at commit 387614f and there was no issue. So I investigated the differences with my app and I noticed that in the Example app you record at a specific size: 720x1280. Source/Controls/ControlsViewController.swift:96 Changing the recording size triggers the problem in iOS 12 on iPhone 6. I found the size CGSize(width: 749, height: 1335) to be the limit where no issue is generated. When you increase the width to numbers >= 750, or the height to numbers >= 1336 then the color problem is reproducible with the Example app. These are the screenshot of the issue IMG_0317 IMG_0318 IMG_0319 IMG_0320 s

v-grigoriev commented 2 years ago

Firstly, you can see green lines at the right and the bottom. They appear because in some encoding formats the size must be a multiple of 4 (or 8). I can't remember concrete values.

Secondly, iPhone 6 resolution is 750 x 1334, it means that in the Example project (or at any usual project) the SCNView has equal or lower size.

I can't say how exactly AVAssetWriter will process pixels when input pixels buffer is smaller than expected resolution. (It should scale it up, but who knows how exactly). It feels like it scales it in a wrong way.

In other words the video size is expected to be at least sample buffer size (SCNView size * screen scale).

v-grigoriev commented 2 years ago

What video size do you set in your app? Do you pass the size parameter at all? If so, will the issue go away if you set the size to nil? (in startVideoRecording)

giomurru commented 2 years ago

iPhone 6 resolution is 750x1334 good catch! However I have no color problem when recording at full resolution 1125x2436 with iPhone X. In my app I am not specifying the size so its recording at sceneView size (*scale), and since sceneView occupies the full screen I am recording at full resolution 750x1334 on iPhone 6 and 1125x2436 o iPhone X. iPhone 6 is giving me color problem, iPhone X is not. It would be helpful if we could determine if this problem is related to iPhone 6 or iOS 12 or both. FYI In the example app I also tried passing double the size 720x1280, which is a multiple of 4 and 8 and the problem persists on iPhone 6, iOS 12. Unfortunately I don't have any iOS 13 or iOS 14 devices to test the issue. IMG_0321

giomurru commented 2 years ago

Ok now, in an attempt to temporary fix the issue for iOS 12 I discovered something strange. Now I am doing this : if #available(iOS 13.0, *) { size = CGSize(width: sceneSize.width * UIScreen.main.scale, height: sceneSize.height * UIScreen.main.scale); } else { size = CGSize(width: sceneSize.width * UIScreen.main.scale - 1, height: sceneSize.height * UIScreen.main.scale); } This dirty trick fixes the color problem on iPhone 6 - iOS 12 because I am passing a size of 749x1334 to startRecording (which is less than 750x1336, that triggers the issue). Cool, but the coolest (strangest) part is that video output is still 750x1334 but with no color problem! LOL

v-grigoriev commented 2 years ago

LOL

May I ask you to record and send me 2 videos from iPhone 6 - iOS 12? With and without issue? You can share videos from the Example app. (share button at the top right corner)

I hope to extract some information from them.

giomurru commented 2 years ago

Ok the one with the dull colors is recorded setting size 750x1334 while the other is recorded setting size to 749x1334. However as you will notice the video output is still 750x1334.

https://user-images.githubusercontent.com/2816576/181030756-58c2c562-46dc-4e8e-b936-2902b7fb8b4f.MOV

https://user-images.githubusercontent.com/2816576/181030800-b3d58b90-63a1-4763-8935-e5738d825d05.MOV

v-grigoriev commented 2 years ago

The issue is definitely with sRGB gamma correction. Unfortunately without the device/iOS I can't debug the issue to figure out a generic fix.

It seems when AVAssetWrite needs to scale down something it adjusts color profile correctly. There is a chance that something should be changed in makePixelBufferAttachements.. but I'm not sure.

giomurru commented 2 years ago

I have to report that unfortunately when using iOS 12 on iPhone 6 and recording with the modified color profile the resulting video is very laggy. At first I thought that it was a coincidence, but then I did a bunch of tests that confirmed the correlation. For now I am opting to exclude the videoColorProperties in iOS 12 and including it in iOS 13 and higher, hoping that this problem is only related to iOS 12. When iOS 16 is going to be released I am probably going to drop support for iOS 12.

v-grigoriev commented 2 years ago

Interesting, I definitely tested a lot with iOS 12 time ago. I will make videoColorProperties to be optional and configurable next version.

giomurru commented 2 years ago

Well in that case it's probably a problem of iPhone 6's limited capabilities. Maybe the hardware is not fully compatible with these videoColorProperties. Anyway I am going to adopt the iOS 12 strategy because I believe if you are still on iOS 12 you probably have a very old device.