Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

App lifecycle problems #26

Closed
clbemre opened this issue Jun 24, 2024 · 63 comments
Closed

App lifecycle problems #26

clbemre opened this issue Jun 24, 2024 · 63 comments
Assignees

Comments

@clbemre
Copy link

clbemre commented Jun 24, 2024

Hello, firstly this is a great framework.

I have a problem. When the application goes to the background and comes to the front again, the image or video preview closes and returns to the shooting screen, this is a problem, the second problem is that when I try to shoot again after this scenario, I get a crash.

Why doesn't the app continue where I left off when I put it in the background and then bring it back to the foreground?

var cameraView: UIView { cameraLayer.superview ?? .init() } -> and i get a crash for take a picture after background/foregroung. Because I guess CameraInputView is being recreated.

Thank you

@FulcrumOne
Copy link
Contributor

Hey @clbemre, thanks for this ticket!

I'm currently busy developing an update for our other library (NavigationView), but I'll be back here as soon as I finish that task (probably at the end of this week).

I hope it suits you, thanks again for your ticket and have a nice day,
T.K.

@clbemre
Copy link
Author

clbemre commented Jun 26, 2024

Hello again @FulcrumOne, thank you for your interest.

Additionally, when there is a CameraPreview screen displayed, if I display an alert on the screen with MJCameraController().alert { }, the CameraPreview closes. I want to maintain its state and prevent the preview screen from closing unless I close it myself. I get the same alert problem if I present it with .alert {} in Camera Preview.

The alert appears on the preview screen, but this situation when the cancel button is pressed. So, disappeared preview screen.

I think this alert problem and the background/foreground problem of the application that I mentioned before are caused by the same problem.


Additionally, I’m just curious, why do you use DispatchQueue.main.async { [self] in } instead of DispatchQueue.main.async { [weak self] in } ?

@clbemre
Copy link
Author

clbemre commented Jul 1, 2024

Hello again @FulcrumOne, when will you fix these issues? or will?

Thank you.

@FulcrumOne
Copy link
Contributor

hey @clbemre,

Sorry, had a lot of work related to our other libraries. In my calendar, work on this library is scheduled to start this Thursday
CleanShot 2024-07-01 at 10 44 38

Apologies for the "communication issues" once again; I believe creating a Discord / Slack channel would help us stay better connected and updated on things related to libraries. I'll try to organize it this week 😉

Have a great day,
Tomasz K.

@clbemre
Copy link
Author

clbemre commented Jul 1, 2024

Thank you @FulcrumOne , I will be waiting for your response.
Thank you for your interest. 🙏🏻

Have a nice day.
Best Regards

@clbemre
Copy link
Author

clbemre commented Jul 4, 2024

Hello again, @FulcrumOne , have you started this issues solutions? I'm sorry for bothering you but its a little important for me :)
Thank you again.

@FulcrumOne
Copy link
Contributor

Hey, I'm starting this evening😉 Hope it will be ready by the weekend

@clbemre
Copy link
Author

clbemre commented Jul 4, 2024

This is great! Thank you :) Good luck :)

@FulcrumOne
Copy link
Contributor

Hello, firstly this is a great framework.

I have a problem. When the application goes to the background and comes to the front again, the image or video preview closes and returns to the shooting screen, this is a problem, the second problem is that when I try to shoot again after this scenario, I get a crash.

Why doesn't the app continue where I left off when I put it in the background and then bring it back to the foreground?

var cameraView: UIView { cameraLayer.superview ?? .init() } -> and i get a crash for take a picture after background/foregroung. Because I guess CameraInputView is being recreated.

Thank you

hey @clbemre, just to confirm - you're telling that you experienced a crash when you backgrounded the app and then returned to the foreground, correct?

@FulcrumOne FulcrumOne added the bug label Jul 4, 2024
@FulcrumOne FulcrumOne self-assigned this Jul 4, 2024
@FulcrumOne
Copy link
Contributor

@clbemre, that second problem (with alert and crash I was able to reproduce, so I'll start from this one)

@clbemre
Copy link
Author

clbemre commented Jul 4, 2024

Hello again @FulcrumOne
First of all, there are a few problems; after the first startup, it puts it in the background without doing anything, and when it comes back to the foreground, it crashes while trying to take a photo/video. Then, after showing an alert while on the preview screen, both the preview screen disappears and when I try to take a photo or video again, it crashes again, and sometimes it is because of the cameraView superView and sometimes it is because of the blurViews, but I think the problem is common, because as far as I understand, the camera manager is deinit/init and the swiftUI randomly appears on the screen. because of their renderings.

Additionally , independently of these, the snapshot and blurview you showed while flipping the camera are turned landscape.

Sorry for my english, sometimes i use translate :)

@FulcrumOne
Copy link
Contributor

hey @clbemre,

No problem, English is not my native language either (it would be nice if we both spoke Polish 😅)

Could you please test the branch called 'patch-1.2.1', it should fix your problem with alert that causes the app to crash?
You should also add this method to your MCameraController - .equatable() (it's a temporary solution)
CleanShot 2024-07-04 at 19 36 35

@clbemre
Copy link
Author

clbemre commented Jul 5, 2024

Hey @FulcrumOne this temporary solution works. Thank you. 💪🏻 (we can't speak Polish, I'm Turkish 😀)
Are we going to continue with this temporary solution or are you continuing to work on resolving the issue at its base? As far as I can see, you are working on memory leaks in particular.

@FulcrumOne
Copy link
Contributor

hey @clbemre, thanks for letting me know!

I want to spend some time today and check the library for problems with memory leaks, and then I will move on to fixing this issue with the background mode.

@clbemre
Copy link
Author

clbemre commented Jul 5, 2024

Hey @FulcrumOne , also we have a little problem :) when First launch MCameraController, the system want to permission. If I deny the system permission, the UI does not update and switch to the error view, the camera layer visible black view.

IMG_F5B502CD1829-1

@FulcrumOne
Copy link
Contributor

@clbemre I'll have to spend little more time on this task, as the problem is little bigger than I initially thought; there are several problems that I discovered today. I'll keep you in the loop😉

@clbemre
Copy link
Author

clbemre commented Jul 5, 2024

okey @FulcrumOne thank you for your response. I'll be wating for you. Good luck :)

@FulcrumOne
Copy link
Contributor

FulcrumOne commented Jul 6, 2024

Hey @clbemre,

So we have two potential solutions to our problem:

  1. We can move CameraManager to the level above (i.e. the parent view in which MCameraController is implemented, managed by the developer). Additionally if we opt for this option, I would also like to move the functions from the Public+MCameraController file lines 15 -> 43 to the CameraManager initialiser.
  2. We can make CameraManager singleton; this will allow us to avoid major changes to the public API, however, there may be some issues when trying to port the library to iPadOS and macOS.

Therefore, I think that, in the long term, a better solution will be to implement point 1. What do you think?

@FulcrumOne
Copy link
Contributor

FulcrumOne commented Jul 7, 2024

@clbemre,

The branch named patch-1.2.1-beta contains fixes to the following problems:

  1. MCameraController and error screen
  2. Alert interrupting camera session
  3. Memory leaks when app switches to and from the background mode

@FulcrumOne
Copy link
Contributor

Hello, firstly this is a great framework.

I have a problem. When the application goes to the background and comes to the front again, the image or video preview closes and returns to the shooting screen, this is a problem, the second problem is that when I try to shoot again after this scenario, I get a crash.

Why doesn't the app continue where I left off when I put it in the background and then bring it back to the foreground?

var cameraView: UIView { cameraLayer.superview ?? .init() } -> and i get a crash for take a picture after background/foregroung. Because I guess CameraInputView is being recreated.

Thank you

I cannot reproduce this problem unfortunately. However, there is a chance that this branch has already fixed the issue. If not, please let me know

@clbemre
Copy link
Author

clbemre commented Jul 7, 2024

@FulcrumOne Hello, Is the temporary solution not sufficient at the moment?
Also I think long term solution is better.

  • patch-1.2.1-beta solutions is finished or will?

  • var cameraView: UIView { cameraLayer.superview ?? .init() } -> It got resolved after applying the equatable() workaround.

  • Actually after I applying the equatable() all problems resolved. I guess just have a little memory leak, thats all.

@FulcrumOne
Copy link
Contributor

Additionally , independently of these, the snapshot and blurview you showed while flipping the camera are turned landscape.

I'm working on this now. If you need a faster solution, I strongly recommend locking the rotation for the current screen (lockOrientation method).

@clbemre
Copy link
Author

clbemre commented Jul 7, 2024

Additionally , independently of these, the snapshot and blurview you showed while flipping the camera are turned landscape.

I'm working on this now. If you need a faster solution, I strongly recommend locking the rotation for the current screen (lockOrientation method).

I noticed this about this problem. When I tested it on the iPhone X, it rotates, but when I tested it on the iPhone 11 Pro Max, it does not rotate. I also completely turned off the device rotation, it only works in portrait mode.

@FulcrumOne
Copy link
Contributor

@FulcrumOne Hello, Is the temporary solution not sufficient at the moment? Also I think long term solution is better.

  • patch-1.2.1-beta solutions is finished or will?
  • var cameraView: UIView { cameraLayer.superview ?? .init() } -> It got resolved after applying the equatable() workaround.
  • Actually after I applying the equatable() all problems resolved. I guess just have a little memory leak, thats all.

This temporary solution was a bit unstable and caused other problems (the error screen was not displayed for example). I would therefore recommend using the patch-1.2.1-beta branch 😉

@clbemre
Copy link
Author

clbemre commented Jul 7, 2024

@FulcrumOne Hello, Is the temporary solution not sufficient at the moment? Also I think long term solution is better.

  • patch-1.2.1-beta solutions is finished or will?
  • var cameraView: UIView { cameraLayer.superview ?? .init() } -> It got resolved after applying the equatable() workaround.
  • Actually after I applying the equatable() all problems resolved. I guess just have a little memory leak, thats all.

This temporary solution was a bit unstable and caused other problems (the error screen was not displayed for example). I would therefore recommend using the patch-1.2.1-beta branch 😉

Okey I will, thank you for your interest and for this great library :)

@FulcrumOne
Copy link
Contributor

Thanks! If you have any further problems, let me know😉

PS. Check out our other solutions too!

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

Hello @FulcrumOne , So now we can create MJCameraController(manager: .init()), is it correct?

Additionally, when Camera first opened, Flip transition and back/front camera transition works very slow now(I think there is too much UI rendering going on). Also, there is a delay when opening for the first time and it opens late.

I'm not sure but I think when I click the apply media button while on the preview screen. the UI freezes for a while because I show an alert after clicking apply, normally it would appear immediately, now it appears after a few seconds, I think the UI is not rendering. Also, when i preview video screen and i have a alert, alert is gone when replayed video. There is too much rendering issue.

Actually, just the .equatable() temproray solution works nicely. Whats problem for .equatable() solutions.

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

@FulcrumOne and , I applied patch-1.2.1-beta. but if i remove Equatable and .equatable() modifier, still i am getting crash, I guess, its have to stay, is it correct?

I’m just curious, why do you use DispatchQueue.main.async { [self] in } instead of DispatchQueue.main.async { [weak self] in } ? :)

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

@FulcrumOne I have checked just before and applied. But still;

  • grid not visible ❌ ( maybe i have missed one thing, i don't know :/ )
  • UI freezing works correct ✅ ( Im not sure but i guess rarely works wrong)
  • first open is fast now ✅
  • flip rotate animation works correct. ✅

thank you 🙏🏻

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

sorry, also The UI update failure error regarding system permission deny also persists.

@FulcrumOne
Copy link
Contributor

FulcrumOne commented Jul 8, 2024

Ok, I was able to see the second problem, but the one with the invisible grid I still cannot debug

Give me five minutes, I'll fix the problem with error screen and then we'll focus on the grid.

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

I have tried it on 2 different real devices, the grids do not appear neither at the first init method nor with the on/off grid button. 😬

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

I'm sorry :/ but Also, something like this happens, I start recording a video and the screen refreshes at that moment and the video starts recording, normally it would start directly. But now, because the screen is refreshing, the frame before I press the button appears on the screen momentarily and disappears, a UI bug. 😥

@FulcrumOne
Copy link
Contributor

could you show me a video please?

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

RPReplay_Final1720443778_480.mov

@FulcrumOne
Sometimes a much more distinct view appears on the screen.

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

(Deleted Video)

@FulcrumOne

I took a more clear video, focus on the bottom left corner and notice a momentary view appearing and disappearing. , can you take a quick look? because I'm going to delete it :D

@FulcrumOne
Copy link
Contributor

sorry, also The UI update failure error regarding system permission deny also persists.

should be fixed now

@FulcrumOne
Copy link
Contributor

346579937-05f85df7-9dd4-4dc5-86ba-504aab3c5cd4.mov

@FulcrumOne

I took a more clear video, focus on the bottom left corner and notice a momentary view appearing and disappearing. , can you take a quick look? because I'm going to delete it :D

Oh, now I see it. i'll try to fix it this night, and if I don't fix it then, I'll have to move it to Wednesday unfortunately😉

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

@FulcrumOne okey thank you again 🙏

@FulcrumOne
Copy link
Contributor

Btw, just to be sure, because the both problems (invisible grids and flashing views) are not present in demo app I'm using for testing; are you sure it's not caused by some UI elements of your app?

And if so, could you please prepare the code for me that I can use to reproduce the bug? Thanks in advance! 😅

@clbemre
Copy link
Author

clbemre commented Jul 8, 2024

I can check again, but firstly could you send me your demo project ?

@FulcrumOne
Copy link
Contributor

@clbemre
Copy link
Author

clbemre commented Jul 9, 2024

@FulcrumOne Hey I have found issue, The problem was caused by me, when I pressed the record button, I was setting captureSession.sessionPreset = .hd1280x720. This was causing a rendering problem on the screen. It was fixed when I deleted the code. Also, the system permission alert problem and the gird visibility problem were solved. Everything is perfect now, thank you very much for your interest. We don't have any other problems that I know of. 🙏🏻🙏🏻

@FulcrumOne
Copy link
Contributor

@clbemre,

I'm super glad to hear it! I'll fix some other bugs I noticed and release version 2.0.0. Thanks for your support and see you soon!⚡️

@clbemre
Copy link
Author

clbemre commented Jul 9, 2024

Thank you. 🙏 I’ll be waiting for 2.0.0 :) Good luck

Have a nice day.

@clbemre
Copy link
Author

clbemre commented Jul 11, 2024

Hello again @FulcrumOne , in CameraManager, you add observers but you don't remove. maybe you can add
deinit{
NotificationCenter.default.removeObserver(self, name: .AVCaptureSessionWasInterrupted, object: captureSession)
}

or

Do you have any spesific reason you don't remove observer?

@FulcrumOne
Copy link
Contributor

@clbemre,

Ahh, you're literally a life saver. I totally missed it. Thanks!

@clbemre
Copy link
Author

clbemre commented Jul 11, 2024

@FulcrumOne

Haha, every time, thank you too:)

@FulcrumOne
Copy link
Contributor

FulcrumOne commented Jul 11, 2024

@clbemre,

Fixed! I also changed the rotation animation a bit + fixed some other issues. Could you please check the latest commits and give me your feedback? Thanks a lot!

PS. The version was published to the main branch ;)

@FulcrumOne FulcrumOne mentioned this issue Jul 12, 2024
FulcrumOne added a commit that referenced this issue Jul 12, 2024
feat:
- (BREAKING) CameraManager must now be declared in the view containing the MCameraController
- Added attribute to indicate whether the user has blocked screen rotation

fix:
- Fixed a problem with screen rotation (#15)
- Video mirroring effect is now visible in the camera's live view
- Fixed a problem when app goes into the background and returns to the foreground (#26)
- Fixed an issue with a library causing the application to crash at random moments (#26)
- Fixed an issue with an error screen not displaying when permissions were rejected during the first launch (#26)
- Fixed other minor UI problems
@clbemre
Copy link
Author

clbemre commented Jul 12, 2024

@FulcrumOne Hey, I'll check as soon as. Thank you for the information.

@clbemre
Copy link
Author

clbemre commented Jul 12, 2024

@FulcrumOne Hello again, everything is good. but just i want to say something.

When I rotate to landscape mode my phone, the camera lens changes and goes into close-up mode. It would be better if this was optional, because I don't want it to go into close-up mode when I rotate it landscape. If there is a option, I don't know, I didn't see. :) Other than that, everything is fine, well done.

@FulcrumOne
Copy link
Contributor

Hey, could you show me the video of the problem you described? Thanks!

@clbemre
Copy link
Author

clbemre commented Jul 13, 2024

@FulcrumOne Sorry I had turned some things off. So I couldn't re-run the error scenario. If I encounter this error again, I will let you know.

@FulcrumOne
Copy link
Contributor

@clbemre, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants