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

Video Recording implementation using CameraX lib #419

Draft
wants to merge 33 commits into
base: master
Choose a base branch
from

Conversation

archie94
Copy link
Collaborator

The primary objective of this PR is to solve #360 #413

We are using the camerax library instead of the camera view library we have been using. CameraX will use the native Hardware encoding to record videos which should provide us better quality of output and possible improvement to stability.

Implementation details:

  • Kept switch in preference to toggle simultaneous video monitoring
  • when on along with video monitoring both image and video will be captured; this will stop camera preview
  • when off but video monitoring on only video will be captured for the camera events and we will have image preview
  • video monitoring off will keep only image capture on

Cons:

  • minSdk is now api 21. This is a requirement from the CameraX Preview library
  • cameraX lib is still in beta. The api may change with future releases.

Testing is always appreciated, better across a wide range of devices since we are dealing with the camera layer. Please leave comments/bugs/suggestions here.

Thanks!

- destroy() releases the camera resource unlike close() which just stops the preview

Signed-off-by: Arka Prava Basu <[email protected]>
- setting LifecycleOwner will remove boilerplate around open(),
  close() and destroy()
- a possible fix for guardianproject#376 ?

Signed-off-by: Arka Prava Basu <[email protected]>
- holding this reference may lead to Context leak

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- keeping Context (Activity/Fragment) in a singleton will cause memory leaks
  Keep the global application object associated with it instead
- fix guardianproject#399

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- Among others this updates materialdatetimepicker to 4.2.0 which
  fixes a memory leak

Signed-off-by: Arka Prava Basu <[email protected]>
- keep this app dependency free; can be added in debug builds whenever
  another investigation is required

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- aims to solve guardianproject#395 (comment)
- According to https://developer.android.com/reference/androidx/lifecycle/DefaultLifecycleObserver
  [DefaultLifecycleObserver] should *always* be preferred over [androidx.lifecycle.LifecycleObserver]
  if we use Java 8. [CameraView] library targets Java 7 hence this implementation aims to ignore
  [androidx.lifecycle.OnLifecycleEvent] annotated methods in the super class and replace them with
  the callbacks implemeted in this sub class

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- due to camerax preview lib we need to make minSDK to 21 from 16; if
this is going to be a major problem we may need to go ahead without the
preview lib

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- Use PreviewView instead of CameraView
- MotionAnalyser as the image analysis Use Case to work with
MotionDetector to detct motion
- save image on motion detect and notify running service
- Expose LiveData from MotionDetector for result; alternative to
callbacks

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- current implementation does not allow both image and video capture
simultaneously

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- stop monitoring via a message to service instead of invoking from
outside
- close the activity after a delay of 3s to clean up video capture and
image capture

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- listen on a live data of result instead of events
- string formatting

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
- Keep switch in preference to toggle simultaneous video monitoring
- when on along with video monitoring both image and video will be
captured; this will stop camera preview
- when off but video monitoring on only video will be captured for the
camera events and we will have image preview
- video monitoring off will keep only image capture on

Signed-off-by: Arka Prava Basu <[email protected]>
- Copy image helper from cameraview lib

Signed-off-by: Arka Prava Basu <[email protected]>
@lukeswitz
Copy link
Collaborator

Awesome! This might solve more issues than those two, I'm seeing a stable experience on 5.1 and that's never happened, ever. I'll test this using legacy and flagship devices/ simulators & lend feedback. Testing some UI and onboarding changes yesterday, this library is what we needed:

  • Supports legacy camera API & fringe devices
  • lifecycle management more suited to use case
  • no crash when expected camera selection hits a fringe case (example: orientation sensor mounted such that delivers incorrect 0,1,2 camera values on Early Nexus & Galaxy).

@n8fr8
Copy link
Member

n8fr8 commented Jul 13, 2020

More fantastic work. This is the direction I had hoped to be able to head, and glad to see CameraX provides what we need.

@n8fr8
Copy link
Member

n8fr8 commented Jul 13, 2020

Getting this error on Pixel 3A, Android 10

E/CameraFragment: Use case binding failed
java.lang.IllegalArgumentException: No supported surface combination is found for camera device - Id : 1. May be attempting to bind too many use cases. Existing surfaces: [] New configs: [androidx.camera.core.impl.VideoCaptureConfig@fb85d12, androidx.camera.core.impl.ImageAnalysisConfig@7db16e0, androidx.camera.core.impl.PreviewConfig@93bf774]
at androidx.camera.camera2.internal.Camera2DeviceSurfaceManager.getSuggestedResolutions(Camera2DeviceSurfaceManager.java:192)
at androidx.camera.core.CameraX.calculateSuggestedResolutions(CameraX.java:966)
at androidx.camera.core.CameraX.bindToLifecycle(CameraX.java:357)
at androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle(ProcessCameraProvider.java:255)
at org.havenapp.main.ui.CameraFragment$initCamera$1.run(CameraFragment.kt:200)

@n8fr8
Copy link
Member

n8fr8 commented Jul 13, 2020

Seems like there was an issue reported here: android/camera-samples#38

@n8fr8
Copy link
Member

n8fr8 commented Jul 13, 2020

btw, this is with "Simultaneous Image Monitoring" disabled

@n8fr8
Copy link
Member

n8fr8 commented Jul 13, 2020

and found a blog post about this limitation here:
https://medium.com/androiddevelopers/using-multiple-camera-streams-simultaneously-bf9488a29482

@lukeswitz
Copy link
Collaborator

I think this can be fixed by removing lines 43 & 44 from MediaRecorderTask- unlocking the camera was causing this for me on lower builds. Give it a go if you please.

Removed:

 mCamera.unlock();
 mMediaRecorder.setCamera(mCamera);

@archie94
Copy link
Collaborator Author

Getting this error on Pixel 3A, Android 10

E/CameraFragment: Use case binding failed
java.lang.IllegalArgumentException: No supported surface combination is found for camera device - Id : 1. May be attempting to bind too many use cases. Existing surfaces: [] New configs: [androidx.camera.core.impl.VideoCaptureConfig@fb85d12, androidx.camera.core.impl.ImageAnalysisConfig@7db16e0, androidx.camera.core.impl.PreviewConfig@93bf774]
at androidx.camera.camera2.internal.Camera2DeviceSurfaceManager.getSuggestedResolutions(Camera2DeviceSurfaceManager.java:192)
at androidx.camera.core.CameraX.calculateSuggestedResolutions(CameraX.java:966)
at androidx.camera.core.CameraX.bindToLifecycle(CameraX.java:357)
at androidx.camera.lifecycle.ProcessCameraProvider.bindToLifecycle(ProcessCameraProvider.java:255)
at org.havenapp.main.ui.CameraFragment$initCamera$1.run(CameraFragment.kt:200)

Was able to reproduce. More as I debug.

@lukeswitz
Copy link
Collaborator

Checking in with new camera implementation. Some calls don't play well with CameraX, SDK dependent.

Performing only simple updates to deprecated camera methods has it working on AOS 5.1 and above.

  • AOS 5.1 and below: Samsung/LG: Preview window or captured image not correctly rotated

  • Preview > Activate > Camera Settings causes loss of surface view (grayed out, says Active, logging stops) Potentially rendering HB monitor useless & false sense of security. Monitor will report active, however no surface is being processed. I have not tested with Audio or sensor motion though logs fill with ambient light readings.

Thanks for pushing this, Archie. I appreciate your hard work.

@archie94
Copy link
Collaborator Author

Really appreciate all the testing. Will try hard to get into this the coming weekend.

@xloem
Copy link
Contributor

xloem commented Oct 8, 2020

We are using the camerax library instead of the camera view library we have been using. CameraX will use the native Hardware encoding to record videos which should provide us better quality of output and possible improvement to stability.

To reiterate. CameraView also uses native hardware encoding and #434 enables this. I'll look into this CameraX branch a little, too.

@xloem
Copy link
Contributor

xloem commented Oct 8, 2020

@archie94 are you available at all to rebase these changes in a way that would make it clearer and easier for others to take up the mantle of getting it stable? Or separate stable ones out so they can be merged?

@lukeswitz
Copy link
Collaborator

@xloem - Appreciate your work; the camera implementation was my first issue a few years back with this project.. so it seems we need some help getting it stable 🤝 - I’ll make a pseudo-master branch of your changes to see how they interact with updated Google/third party libs & deps

@archie94
Copy link
Collaborator Author

archie94 commented Oct 8, 2020

Sorry for the inactivity here, my job has kept me pretty busy!

@xloem I am open to resolving the conflict, which looks trivial. Do you need a rebase specifically? As such the commit history on this branch looks self explanatory to me.

Apart from that it looks like we need to have a few checks on the device camera characteristics before enabling Preview, analysis, image capture and video capture all at once. Initially I thought the first 3 would be always supported and the 4th can be optional but clearly some devices would not allow that.

A little busy right now to dig deeper at this moment but definitely will look into it as I have the time.
In the meantime please feel free to continue on the work (CameraX or CameraView, as the case may be) and if you have any questions regarding work on this branch feel free to reach out. :)

@archie94 archie94 marked this pull request as draft October 8, 2020 17:08
@xloem
Copy link
Contributor

xloem commented Oct 8, 2020

Sorry for the inactivity here, my job has kept me pretty busy!

@xloem I am open to resolving the conflict, which looks trivial. Do you need a rebase specifically? As such the commit history on this branch looks self explanatory to me.

Apart from that it looks like we need to have a few checks on the device camera characteristics before enabling Preview, analysis, image capture and video capture all at once. Initially I thought the first 3 would be always supported and the 4th can be optional but clearly some devices would not allow that.

A little busy right now to dig deeper at this moment but definitely will look into it as I have the time.
In the meantime please feel free to continue on the work (CameraX or CameraView, as the case may be) and if you have any questions regarding work on this branch feel free to reach out. :)

It's just a lot of changes to review to understand where the code is at. Some of them look squashable or technically unnecessary at a quick glance. The less noise there is to the information, the more visits and visitors pass the time thresholds to contribute. Any explanation is helpful. Thanks so much for your update.

Signed-off-by: Arka Prava Basu <[email protected]>

Update dependencies

Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
Signed-off-by: Arka Prava Basu <[email protected]>
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

Successfully merging this pull request may close these issues.

4 participants