search

CameraKit / camerakit-android

Library for Android Camera 1 and 2 APIs. Massively increase stability and reliability of photo and video capture on all Android devices.
https://camerakit.io
MIT License
5.36k stars 876 forks source link

CaptureImage callback works #504

Closed huntj88 closed 5 years ago

huntj88 commented 5 years ago

I fully expect this to be rejected, but The callback issue (#467) is fixed with this code. Hopefully this will gives some clues as to one of the two problems I found.

The first is a place I found a null getting set when I don't think it was intended to be, which caused issues on certain devices.

The second issues was something with coroutines. They were choking somewhere, but removing them did the trick. Doing this doesn't seem to have major performance drawbacks (none that i can see) for taking a simple picture, which fits my use case. (doesn't mean it should't be fixed properly though)

emersoncloud commented 5 years ago

Hey @huntj88 I was testing out your pull request. In light of #467, which condition did you aim to fix with this PR?

huntj88 commented 5 years ago

the nulling out of the captureSession prematurely seems to be a real issue. But the thing with the coroutines was also affecting the callback from working. This is just everything that i needed to change to get it working for me.

This PR is less of a fix, and more of bringing attention to where those two problems exists.

I can play around with the coroutines later today, and see if I can find out whats going on.

glenatron commented 5 years ago

I think those coroutines might be the problem in #508 as well - the changes in this patch seem to fix that for me.

emersoncloud commented 5 years ago

@huntj88 thanks for opening this pull request. We are going to merge these changes in with our v1.0.0-beta3.12 PR we have open here: #498 along with a few other changes. This is not the best long term solution but will solve the issues with #467 and #508 while we work on a more permanent fix.

Thanks you again for the contribution! We are making the changes ourselves to prevent the need for a Contributor License Agreement at this stage. We truly appreciate your help to CameraKit and welcome future PRs and Issues to further improve CameraKit.

huntj88 commented 5 years ago

I notice that the changes in camera2.kt were not included in the update. Without that code the camera fails to work on the LG G5

@emersoncloud

austinkettner commented 5 years ago

Closing in favor of #498 but would totally dig more commits here and reOpening the PR with more robust fixes. Thanks for the effort here! Add your Camera2.kt changes @huntj88