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 foundation: introducing CameraX #23

Merged
merged 16 commits into from
Aug 14, 2019
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,40 @@ dependencies {
androidTestImplementation 'androidx.test:runner:1.2.0'
androidTestImplementation 'androidx.test.espresso:espresso-core:3.2.0'
}

android.buildTypes.all { buildType ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of this looks good, though I think we should take care of adding gradle.properties to .gitignore sooner than later and add a gradle.properties-example file. I'll open an issue to track that.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opened: #27.

// Add properties named "portkey.xxx" to our BuildConfig
def inputFile = checkGradlePropertiesFile()
def properties = loadPropertiesFromFile(inputFile)
properties.any { property ->
if (property.key.toLowerCase().startsWith("portkey.use.")) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting - in WPAndroid we don't have this handling and instead just use BuildConfig.VALUE.toBoolean() when accessing it. This is probably better 😀

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH that was a TIL for me as well when I introduced / borrowed these scripts :D

buildType.buildConfigField "boolean", property.key.replace("portkey.", "").replace(".", "_").toUpperCase(),
"${property.value}"
}
else if (property.key.toLowerCase().startsWith("portkey.")) {
buildType.buildConfigField "String", property.key.replace("portkey.", "").replace(".", "_").toUpperCase(),
"\"${property.value}\""
}
else if (property.key.toLowerCase().startsWith("portkey.res.")) {
buildType.resValue "string", property.key.replace("portkey.res.", "").replace(".", "_").toLowerCase(),
"${property.value}"
}
}
}

def checkGradlePropertiesFile() {
def inputFile = file("${rootDir}/gradle.properties")
if (!inputFile.exists()) {
throw new StopActionException("Build configuration file gradle.properties doesn't exist, follow README instructions")
}
return inputFile
}

static def loadPropertiesFromFile(inputFile) {
def properties = new Properties()
inputFile.withInputStream { stream ->
properties.load(stream)
}
return properties
}

Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import com.automattic.photoeditor.state.BackgroundSurfaceManager
import com.automattic.photoeditor.util.FileUtils.Companion.getLoopFrameFile
import com.automattic.photoeditor.util.PermissionUtils
import com.automattic.photoeditor.views.ViewType
import com.automattic.portkey.BuildConfig
import com.automattic.portkey.R
import com.automattic.portkey.R.color
import com.automattic.portkey.R.id
Expand Down Expand Up @@ -86,7 +87,9 @@ class ComposeLoopFrameActivity : AppCompatActivity() {
savedInstanceState,
lifecycle,
photoEditorView,
supportFragmentManager)
supportFragmentManager,
BuildConfig.USE_CAMERAX)

lifecycle.addObserver(backgroundSurfaceManager)
}

Expand Down
2 changes: 2 additions & 0 deletions gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,5 @@ android.useAndroidX=true
android.enableJetifier=true
# Kotlin code style for this project: "official" or "obsolete":
kotlin.code.style=official

portkey.use.cameraX = true
13 changes: 13 additions & 0 deletions photoeditor/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,19 @@ dependencies {

implementation 'androidx.appcompat:appcompat:1.1.0-beta01'
implementation "androidx.core:core-ktx:+"

// jetpack camera library versions
// check https://developer.android.com/jetpack/androidx/releases/camera for updates

// CameraX core library
def camerax_version = "1.0.0-alpha04"
// CameraX view library
def camerax_view_version = "1.0.0-alpha01"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one and the one below seems to be unused.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addressed in 26767fc

// CameraX extensions library
def camerax_ext_version = "1.0.0-alpha01"
implementation "androidx.camera:camera-core:${camerax_version}"
implementation "androidx.camera:camera-camera2:${camerax_version}"

implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk7:$kotlin_version"
implementation 'com.github.bumptech.glide:glide:4.9.0'
kapt 'com.github.bumptech.glide:compiler:4.9.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ import android.view.TextureView
import android.view.View
import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat
import androidx.fragment.app.Fragment
import com.automattic.photoeditor.util.FileUtils
import com.automattic.photoeditor.R
import com.automattic.photoeditor.camera.interfaces.VideoRecorderFragment
import com.automattic.photoeditor.util.PermissionUtils
import com.automattic.photoeditor.views.background.video.AutoFitTextureView
import java.io.File
Expand All @@ -64,8 +64,8 @@ import kotlin.collections.ArrayList

@JvmField val PIC_FILE_NAME = "pic.jpg"

class Camera2BasicHandling : Fragment(), View.OnClickListener,
ActivityCompat.OnRequestPermissionsResultCallback, SurfaceFragmentHandler {
class Camera2BasicHandling : VideoRecorderFragment(), View.OnClickListener,
ActivityCompat.OnRequestPermissionsResultCallback {
/**
* [TextureView.SurfaceTextureListener] handles several lifecycle events on a
* [TextureView].
Expand All @@ -91,11 +91,6 @@ class Camera2BasicHandling : Fragment(), View.OnClickListener,
*/
private lateinit var cameraId: String

/**
* An [AutoFitTextureView] for camera preview.
*/
lateinit var textureView: AutoFitTextureView

private var active: Boolean = false

/**
Expand Down Expand Up @@ -201,8 +196,6 @@ class Camera2BasicHandling : Fragment(), View.OnClickListener,
* */
private var mediaRecorder: MediaRecorder = MediaRecorder()

var currentFile: File? = null

/**
* A [CameraCaptureSession.CaptureCallback] that handles events related to JPEG capture.
*/
Expand Down Expand Up @@ -768,7 +761,7 @@ class Camera2BasicHandling : Fragment(), View.OnClickListener,
/**
* Creates a new [CameraCaptureSession] for actual video recording.
*/
fun createCameraRecordingSession() {
override fun startRecordingVideo() {
try {
closePreviewSession()
setUpMediaRecorder()
Expand Down Expand Up @@ -826,7 +819,7 @@ class Camera2BasicHandling : Fragment(), View.OnClickListener,
}
}

fun stopRecordingVideo() {
override fun stopRecordingVideo() {
captureSession?.apply {
stopRepeating()
abortCaptures()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
package com.automattic.photoeditor.camera

import android.annotation.SuppressLint
import android.os.Bundle
import android.util.Log
import androidx.camera.core.CameraX
import androidx.camera.core.Preview
import androidx.camera.core.PreviewConfig
import androidx.camera.core.VideoCapture
import androidx.camera.core.VideoCaptureConfig
import androidx.core.app.ActivityCompat
import com.automattic.photoeditor.R
import com.automattic.photoeditor.camera.interfaces.VideoRecorderFragment
import com.automattic.photoeditor.util.FileUtils
import com.automattic.photoeditor.util.PermissionUtils
import com.automattic.photoeditor.views.background.video.AutoFitTextureView
import java.io.File

class CameraXBasicHandling : VideoRecorderFragment(),
ActivityCompat.OnRequestPermissionsResultCallback {
private lateinit var videoCapture: VideoCapture

private var active: Boolean = false

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
retainInstance = true
}

override fun activate() {
active = true
startUp()
}

override fun deactivate() {
if (active) {
active = false
windDown()
}
}

private fun startUp() {
if (active) {
startCamera()
}
}

private fun windDown() {
if (CameraX.isBound(videoCapture)) {
CameraX.unbind(videoCapture)
}
}

override fun onRequestPermissionsResult(
requestCode: Int,
permissions: Array<String>,
grantResults: IntArray
) {
if (!PermissionUtils.allRequiredPermissionsGranted(activity!!)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove the !! force unwrap here by wrapping the whole function body in a null check:

activity?.let { activity ->
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ended up removing this entirely in bbcf4df, to match the same thing we did in e2fe8ee , related comment here: #22 (comment)

ErrorDialog.newInstance(getString(R.string.request_permissions))
.show(childFragmentManager,
FRAGMENT_DIALOG
)
} else {
super.onRequestPermissionsResult(requestCode, permissions, grantResults)
}
}

// TODO remove this RestrictedApi annotation once androidx.camera:camera moves out of alpha
@SuppressLint("RestrictedApi")
private fun startCamera() {
// Create configuration object for the preview use case
val previewConfig = PreviewConfig.Builder().build()
val preview = Preview(previewConfig)

// Create a configuration object for the video capture use case
val videoCaptureConfig = VideoCaptureConfig.Builder().apply {
setTargetRotation(textureView.display.rotation)
}.build()
videoCapture = VideoCapture(videoCaptureConfig)

preview.setOnPreviewOutputUpdateListener {
textureView.surfaceTexture = it.surfaceTexture
}

// Bind use cases to lifecycle
CameraX.bindToLifecycle(activity, preview, videoCapture)
}

@SuppressLint("RestrictedApi")
override fun startRecordingVideo() {
currentFile = FileUtils.getLoopFrameFile(true, "orig_")
currentFile?.createNewFile()

videoCapture.startRecording(currentFile, object : VideoCapture.OnVideoSavedListener {
override fun onVideoSaved(file: File?) {
Log.i(tag, "Video File : $file")
}
override fun onError(useCaseError: VideoCapture.UseCaseError?, message: String?, cause: Throwable?) {
Log.i(tag, "Video Error: $message")
}
})
}

@SuppressLint("RestrictedApi")
override fun stopRecordingVideo() {
videoCapture.stopRecording()
}

companion object {
private val instance = CameraXBasicHandling()

private val FRAGMENT_DIALOG = "dialog"
/**
* Tag for the [Log].
*/
private val TAG = "CameraXBasicHandling"

@JvmStatic fun getInstance(textureView: AutoFitTextureView): CameraXBasicHandling {
instance.textureView = textureView
return instance
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,15 @@ import java.io.File
import android.media.AudioManager
import android.media.MediaPlayer
import androidx.fragment.app.Fragment
import com.automattic.photoeditor.camera.interfaces.SurfaceFragmentHandler
import com.automattic.photoeditor.views.background.video.AutoFitTextureView
import java.io.FileInputStream
import java.io.IOException

class VideoPlayingBasicHandling : Fragment(), SurfaceFragmentHandler {
// holds the File handle to the current video file to be played
var currentFile: File? = null

/**
* [TextureView.SurfaceTextureListener] handles several lifecycle events on a
* [TextureView].
Expand Down Expand Up @@ -131,22 +136,26 @@ class VideoPlayingBasicHandling : Fragment(), SurfaceFragmentHandler {
stopVideoPlay()
}

val assetManager = context?.assets
val descriptor = assetManager!!.openFd("small.mp4")
mediaPlayer = MediaPlayer().apply {
// setDataSource("http://techslides.com/demos/sample-videos/small.mp4")
setDataSource(descriptor?.getFileDescriptor(), descriptor.getStartOffset(), descriptor.getLength())
setSurface(s)
prepare()
// TODO check whether we want fine grained error handling by setting these listeners
// setOnBufferingUpdateListener(this)
// setOnCompletionListener(this)
// setOnPreparedListener(this)
// setOnVideoSizeChangedListener(this)
setAudioStreamType(AudioManager.STREAM_MUSIC)
start()
if (currentFile != null && currentFile!!.exists()) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can Kotlinify this a bit and fix a nullability warning in the code (the call is actually safe but the IDE doesn't know that):

currentFile?.takeIf { it.exists() }?.let { file ->
    ...
    val inputStream = FileInputStream(file)
    ...
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice! addressed in e0f25fa (plus removed some commented code)

Also TIL:

the call is actually safe but the IDE doesn't know that

// val assetManager = context?.assets
// val descriptor = assetManager!!.openFd("small.mp4")
val inputStream = FileInputStream(currentFile)
mediaPlayer = MediaPlayer().apply {
setDataSource(inputStream.getFD())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Kotlinification note, not a blocker)

This can use property access syntax: inputStream.fd.

// setDataSource("http://techslides.com/demos/sample-videos/small.mp4")
// setDataSource(descriptor?.getFileDescriptor(), descriptor.getStartOffset(), descriptor.getLength())
setSurface(s)
prepare()
// TODO check whether we want fine grained error handling by setting these listeners
// setOnBufferingUpdateListener(this)
// setOnCompletionListener(this)
// setOnPreparedListener(this)
// setOnVideoSizeChangedListener(this)
setAudioStreamType(AudioManager.STREAM_MUSIC)
start()
}
// descriptor?.close()
}
descriptor?.close()
} catch (e: IllegalArgumentException) {
// TODO Auto-generated catch block
e.printStackTrace()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

package com.automattic.photoeditor.camera
package com.automattic.photoeditor.camera.interfaces

interface SurfaceFragmentHandler {
fun activate()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@

package com.automattic.photoeditor.camera.interfaces

import androidx.fragment.app.Fragment
import com.automattic.photoeditor.views.background.video.AutoFitTextureView
import java.io.File

abstract class VideoRecorderFragment : Fragment(), VideoRecorderHandler, SurfaceFragmentHandler {
/**
* An [AutoFitTextureView] for camera preview.
*/
lateinit var textureView: AutoFitTextureView
var currentFile: File? = null
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@

package com.automattic.photoeditor.camera.interfaces

interface VideoRecorderHandler {
fun startRecordingVideo()
fun stopRecordingVideo()
}
Loading