RedApparat / Fotoapparat

Making Camera for Android more friendly. 📸
Apache License 2.0
3.82k stars 405 forks source link

Catch IllegalStateException and throw CameraException #138

Closed r2DoesInc closed 6 years ago

r2DoesInc commented 6 years ago

Should resolve the issue as seen below. Starting the preview can throw an IllegalStateException with no way to catch it. This catches the ISE and replaces it with the CameraException allowing you to catch the failure and process it rather than forcing a crash.

Fatal Exception: java.lang.IllegalStateException: Target display surface has not been set. at io.fotoapparat.hardware.v2.session.SessionProvider.getPreviewSession(SessionProvider.java:67) at io.fotoapparat.hardware.v2.session.SessionManager.startPreview(SessionManager.java:26) at io.fotoapparat.hardware.v2.Camera2.startPreview(Camera2.java:103) at io.fotoapparat.routine.StartCameraRoutine.tryToStartCamera(StartCameraRoutine.java:67) at io.fotoapparat.routine.StartCameraRoutine.run(StartCameraRoutine.java:47) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636) at java.lang.Thread.run(Thread.java:764)

r2DoesInc commented 6 years ago

I get that crash reported occasionally but have been unable to reproduce it myself. It seems to me that the issue is this ISE which is impossible to catch. Giving the error to the CameraException manager allows the app to gracefully manage that failure and to continue processing.

Diolor commented 6 years ago

Hi Ken and thanks for the submission! Please use the v1 hardware camera for now because v2 is lower level and produces more issues.. It will work actually better. If you see this issue in v1 please feel free to open an issue/pr :)

r2DoesInc commented 6 years ago

I have been using v2 in my app without issues other than this one, and this change has fixed the issue for my users while using v2. Due to some of the requirements of the app I think that v2 suits our needs better, but just needs this fix. I have had to pull your library in as a sub project so far to implement this fix, would love to see it pulled in upstream so we can go back to using this as angradle dependency rather than me maintaining a custom for just for this one change.

On Tue, Nov 28, 2017, 9:27 AM Dionysis Lorentzos notifications@github.com wrote:

Hi Ken and thanks for the submission! Please use the v1 hardware camera for now because v2 is lower level and produces more issues.. It will work actually better. If you see this issue in v1 please feel free to open an issue/pr :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Fotoapparat/Fotoapparat/pull/138#issuecomment-347539735, or mute the thread https://github.com/notifications/unsubscribe-auth/AAftlQqd-hdbRQ-cgUwLinmFOHZHxuSTks5s7BhhgaJpZM4QscSI .

-- [image: photo] Ken Kyger Founder, FutureHax 407.965.8510 | r2doesinc@futurehax.org | futurehax.org | Skype: r2doesinc <#> | Orlando Florida http://www.linkedin.com/in/r2doesinc http://twitter.com/r2doesinc http://github.com/futurehax Get your own email signature https://wisestamp.com/email-install?utm_source=promotion&utm_medium=signature&utm_campaign=get_your_own