Fragment to control and show camera images

Question

I have never tried this before. Just started with Kotlin and Android CameraX. Here is a fragment I would like some feedback on so I can writer safer and better Kotlin code.

package com.name.app.maincontroller

import android.Manifest
import android.annotation.SuppressLint
import android.content.Context
import android.content.pm.PackageManager
import android.os.Bundle
import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.ImageButton
import androidx.camera.core.*
import androidx.camera.lifecycle.ProcessCameraProvider
import androidx.camera.view.PreviewView
import androidx.core.app.ActivityCompat
import androidx.core.content.ContextCompat
import androidx.fragment.app.Fragment
import androidx.lifecycle.LifecycleOwner
import com.name.app.persistentdata.getOutputDirectory
import java.io.*
import java.text.SimpleDateFormat
import java.util.*
import java.util.concurrent.Executors
import kotlin.random.Random

const val height: Int = 175
const val width: Int = 100

class CameraActivityFragment : Fragment() {

    private val permissions = listOf(Manifest.permission.CAMERA, Manifest.permission.RECORD_AUDIO)
    private val permissionsRequestCode = Random.nextInt(0, 10000)


    private var isRecording = false

    private val videoCapture = VideoCapture.Builder().build()
    private lateinit var cameraCaptureButton: ImageButton
    private lateinit var cameraPreview: PreviewView
    private lateinit var cameraProvider: ProcessCameraProvider
    private lateinit var preview: Preview
    private val cameraSelector = CameraSelector.DEFAULT_BACK_CAMERA

    private fun setRecordVideoListener() {
        // Disable all camera controls
        cameraCaptureButton.setOnClickListener {
            it.isEnabled = false
            if (!isRecording) {
                isRecording = true
                cameraCaptureButton.setBackgroundResource(R.drawable.ic_shutter_pressed)
                it.invalidate()
                startRecording()

            } else {
                isRecording = false
                cameraCaptureButton.setBackgroundResource(R.drawable.ic_shutter_normal)
                it.invalidate()
                stopRecording()
            }
            // Re-enable camera controls
            it.isEnabled = true
        }
    }

    @SuppressLint("RestrictedApi")
    private fun startRecording() {
        val appName = resources.getString(R.string.app_name)
        val currentTime = System.currentTimeMillis()
        val videoFile = File(
            getOutputDirectory(requireActivity(), appName),
            SimpleDateFormat(
                "yyyy-MM-dd-HH-mm-ss-SSS", Locale.US
            ).format(currentTime) + ".mp4"
        )

        val outputOptions = VideoCapture.OutputFileOptions.Builder(videoFile).build()
        if (ActivityCompat.checkSelfPermission(
                requireContext(),
                Manifest.permission.RECORD_AUDIO
            ) != PackageManager.PERMISSION_GRANTED
        ) {
            return
        }
        videoCapture.startRecording(outputOptions, ContextCompat.getMainExecutor(activity),
            object : VideoCapture.OnVideoSavedCallback {
                override fun onError(videoCaptureError: Int, message: String, cause: Throwable?) {
                    Log.e(TAG, "Video capture failed: $message")
                }
                override fun onVideoSaved(outputFileResults: VideoCapture.OutputFileResults) {
                }
            })
    }

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

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        cameraCaptureButton = view.findViewById(R.id.camera_capture_button)
        cameraPreview = view.findViewById(R.id.camera_preview)
        setRecordVideoListener()
    }

    override fun onCreateView(
        inflater: LayoutInflater, container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View? {
        // Inflate the layout for this fragment
        return inflater.inflate(R.layout.fragment_camera_activity, container, false)
    }

    @SuppressLint("UnsafeExperimentalUsageError")
    private fun bindVideoCapture() {
        // Create a new camera selector each time, enforcing lens facing
        try {
            // Apply declared configs to CameraX using the same lifecycle owner
            cameraProvider.bindToLifecycle(
                this as LifecycleOwner, cameraSelector, preview, videoCapture
            )

        } catch (exc: Exception) {
            Log.e("BindVideoCapture", "Use case binding failed", exc)
        }
    }

    private fun startCamera() {
        val cameraProviderFuture = ProcessCameraProvider.getInstance(requireContext())
        cameraProviderFuture.addListener({
            // Used to bind the lifecycle of cameras to the lifecycle owner
            cameraProvider = cameraProviderFuture.get()
            cameraProvider.unbindAll()
            // Set up the view finder use case to display camera preview
            preview = Preview.Builder()
                .build()
            try {
                bindVideoCapture()
            } catch (exc: Exception) {
                Log.e("TestCameraPytorch", "Use case binding failed", exc)
            }
            preview.setSurfaceProvider(cameraPreview.surfaceProvider)
        }, ContextCompat.getMainExecutor(requireContext()))
    }

    override fun onResume() {
        super.onResume()
        // Request permissions each time the app resumes, since they can be revoked at any time
        if (!context?.let { hasPermissions(it) }!!) {
            activity?.let {
                ActivityCompat.requestPermissions(
                    it, permissions.toTypedArray(), permissionsRequestCode
                )
            }
        } else {
            startCamera()
        }
    }

    override fun onRequestPermissionsResult(
        requestCode: Int,
        permissions: Array<out String>,
        grantResults: IntArray
    ) {
        super.onRequestPermissionsResult(requestCode, permissions, grantResults)
        if (requestCode == permissionsRequestCode && hasPermissions(requireContext())) {
            startCamera()
        } else {
            activity?.finish() // If we don't have the required permissions, we can't run
        }
    }

    /** Convenience method used to check if all permissions required by this app are granted */
    private fun hasPermissions(context: Context) = permissions.all {
        ContextCompat.checkSelfPermission(context, it) == PackageManager.PERMISSION_GRANTED
    }

    companion object {
        private val TAG = MainActivity::class.java.simpleName
    }
}

Answer 1

I haven't worked with CameraX, so I'll just point out other things I noticed.

---

In this code:

        cameraCaptureButton.setOnClickListener {
            it.isEnabled = false
            if (!isRecording) {
                isRecording = true
                cameraCaptureButton.setBackgroundResource(R.drawable.ic_shutter_pressed)
                it.invalidate()
                startRecording()

            } else {
                isRecording = false
                cameraCaptureButton.setBackgroundResource(R.drawable.ic_shutter_normal)
                it.invalidate()
                stopRecording()
            }
            // Re-enable camera controls
            it.isEnabled = true
        }

It is poor practice for code clarity to use the implicit parameter name it in a multi-line lambda. In this case, you don't need to use it at all, so you don't need to name it, since it refers to the same object as the already-named property cameraCaptureButton.

Also, disabling it and re-enabling it here in the listener won't accomplish anything because input processing is not handled concurrently with this code run on the main thread. Multiple taps that happen rapidly between two frames of the main thread will be queued and then all submitted in succession on the main thread. If your goal is to avoid the possibility of someone double-tapping the button faster than the state can be changed, you must use some sort of debouncing. (See here for example.)

The invalidate calls are redundant because changing anything about the view's appearance (like setting the background) will invalidate it.

---

If the only thing you're doing in onCreateView is inflating and returning a Layout, you can omit it completely and simply pass the layout to the super-class constructor:

class CameraActivityFragment : Fragment(R.layout.fragment_camera_activity)

---

This code in onResume() is self-contradictory:

!context?.let { hasPermissions(it) }!!

You use a null-safe call followed by a non-null assertion. Either you know the object is not null or you don't, but this code simultaneously communicates that you're not sure (?.) and then says that you are sure (!!). If you know it's not null, you should be bold and use it at the source like this:

!hasPermissions(context!!)

Incidentally, we know from the documentation of the Fragment lifecycle that context will not be null during lifecycle functions like onResume, so context!! is safe. However, Android provides requireContext() and requireActivity() functions that are preferable to context!! and activity!!, because they will provide better error messages if you use them in the wrong place. Also, since !! is so often a code smell, these functions help show your intent more deliberately without alarming anyone that's reviewing your code.

In this related code:

        activity?.let {
            ActivityCompat.requestPermissions(
                it, permissions.toTypedArray(), permissionsRequestCode
            )
        }

You demonstrate with ?.let that you are unsure if activity might be null, and you don't want to do anything if it is null. If that were actually possible, your code would not start the camera and would not check permissions, so you'd just have kind of blank screen with no feedback to the user why nothing is happening indefinitely.

If you were actually unsure of whether activity could be null, you would have a fallback action to take in the flow of how your app behaves to the user.

But in actuality, we know the activity is not null during onResume, so you can use requireActivity().