From 38bbf5a90de93d90f2863195df5637fac5df4be4 Mon Sep 17 00:00:00 2001 From: davidjiagoogle Date: Thu, 2 Apr 2026 21:55:05 +0000 Subject: [PATCH 1/2] multipleEventCutter --- .../controller/impl/CaptureControllerImpl.kt | 197 +++++++++--------- .../controller/impl/MultipleEventsCutter.kt | 35 ++++ 2 files changed, 138 insertions(+), 94 deletions(-) create mode 100644 ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt index c6fb504e8..46e1b4062 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt @@ -89,6 +89,7 @@ class CaptureControllerImpl( private var recordingJob: Job? = null private val job = Job(parent = coroutineContext[Job]) private val scope = CoroutineScope(coroutineContext + job) + private val multipleEventsCutter = MultipleEventsCutter() override fun captureImage(contentResolver: ContentResolver) { if (externalCaptureMode == ExternalCaptureMode.VideoCapture) { @@ -102,56 +103,60 @@ class CaptureControllerImpl( ) return } - Log.d(TAG, "captureImage") - scope.launch { - val (saveLocation, progress) = nextSaveLocation( - saveMode, - externalCaptureMode, - externalCapturesCallback - ) - captureImageInternal( - saveLocation = saveLocation, - doTakePicture = { - cameraSystem.takePicture(contentResolver, saveLocation) { - trackedCaptureUiState.update { old -> - old.copy(lastBlinkTimeStamp = System.currentTimeMillis()) - } - }.savedUri - }, - onSuccess = { savedUri -> - val event = if (progress != null) { - ImageCaptureEvent.SequentialImageSaved(savedUri, progress) - } else { - if (saveLocation is SaveLocation.Cache) { - ImageCaptureEvent.SingleImageCached(savedUri) + multipleEventsCutter.processEvent { + Log.d(TAG, "captureImage") + scope.launch { + val (saveLocation, progress) = nextSaveLocation( + saveMode, + externalCaptureMode, + externalCapturesCallback + ) + captureImageInternal( + saveLocation = saveLocation, + doTakePicture = { + cameraSystem.takePicture(contentResolver, saveLocation) { + trackedCaptureUiState.update { old -> + old.copy(lastBlinkTimeStamp = System.currentTimeMillis()) + } + }.savedUri + }, + onSuccess = { savedUri -> + val event = if (progress != null) { + ImageCaptureEvent.SequentialImageSaved(savedUri, progress) } else { - ImageCaptureEvent.SingleImageSaved(savedUri) + if (saveLocation is SaveLocation.Cache) { + ImageCaptureEvent.SingleImageCached(savedUri) + } else { + ImageCaptureEvent.SingleImageSaved(savedUri) + } } - } - if (saveLocation !is SaveLocation.Cache) { - imageWellController.updateLastCapturedMedia() - } else { - savedUri?.let { - scope.launch { - postCurrentMediaToMediaRepository( - mediaRepository, - MediaDescriptor.Content.Image(it, null, true) - ) + if (saveLocation !is SaveLocation.Cache) { + imageWellController.updateLastCapturedMedia() + } else { + savedUri?.let { + scope.launch { + postCurrentMediaToMediaRepository( + mediaRepository, + MediaDescriptor.Content.Image(it, null, true) + ) + } } } - } - captureEvents.trySend(event) - }, - onFailure = { exception -> - val event = if (progress != null) { - ImageCaptureEvent.SequentialImageCaptureError(exception, progress) - } else { - ImageCaptureEvent.SingleImageCaptureError(exception) - } + captureEvents.trySend(event) + multipleEventsCutter.done() + }, + onFailure = { exception -> + val event = if (progress != null) { + ImageCaptureEvent.SequentialImageCaptureError(exception, progress) + } else { + ImageCaptureEvent.SingleImageCaptureError(exception) + } - captureEvents.trySend(event) - } - ) + captureEvents.trySend(event) + multipleEventsCutter.done() + } + ) + } } } @@ -169,69 +174,73 @@ class CaptureControllerImpl( return } Log.d(TAG, "startVideoRecording") - recordingJob = scope.launch { - val cookie = "Video-${videoCaptureStartedCount.incrementAndGet()}" - val (saveLocation, _) = nextSaveLocation( - saveMode, - externalCaptureMode, - externalCapturesCallback - ) - try { - cameraSystem.startVideoRecording(saveLocation) { - var snackbarToShow: SnackbarData? - when (it) { - is OnVideoRecordEvent.OnVideoRecorded -> { - Log.d(TAG, "cameraSystem.startRecording OnVideoRecorded") - val event = if (saveLocation is SaveLocation.Cache) { - VideoCaptureEvent.VideoCached(it.savedUri) - } else { - VideoCaptureEvent.VideoSaved(it.savedUri) - } + multipleEventsCutter.processEvent { + recordingJob = scope.launch { + val cookie = "Video-${videoCaptureStartedCount.incrementAndGet()}" + val (saveLocation, _) = nextSaveLocation( + saveMode, + externalCaptureMode, + externalCapturesCallback + ) + try { + cameraSystem.startVideoRecording(saveLocation) { + var snackbarToShow: SnackbarData? + when (it) { + is OnVideoRecordEvent.OnVideoRecorded -> { + Log.d(TAG, "cameraSystem.startRecording OnVideoRecorded") + val event = if (saveLocation is SaveLocation.Cache) { + VideoCaptureEvent.VideoCached(it.savedUri) + } else { + VideoCaptureEvent.VideoSaved(it.savedUri) + } - if (saveLocation !is SaveLocation.Cache) { - imageWellController.updateLastCapturedMedia() - } else { - scope.launch { - postCurrentMediaToMediaRepository( - mediaRepository, - MediaDescriptor.Content.Video(it.savedUri, null, true) + if (saveLocation !is SaveLocation.Cache) { + imageWellController.updateLastCapturedMedia() + } else { + scope.launch { + postCurrentMediaToMediaRepository( + mediaRepository, + MediaDescriptor.Content.Video(it.savedUri, null, true) + ) + } + } + + captureEvents.trySend(event) + // don't display snackbar for successful capture + snackbarToShow = if (saveLocation is SaveLocation.Cache) { + null + } else { + SnackbarData( + cookie = cookie, + stringResource = R.string.toast_video_capture_success, + withDismissAction = true, + testTag = VIDEO_CAPTURE_SUCCESS_TAG ) } } - captureEvents.trySend(event) - // don't display snackbar for successful capture - snackbarToShow = if (saveLocation is SaveLocation.Cache) { - null - } else { - SnackbarData( + is OnVideoRecordEvent.OnVideoRecordError -> { + Log.d(TAG, "cameraSystem.startRecording OnVideoRecordError") + captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(it.error)) + snackbarToShow = SnackbarData( cookie = cookie, - stringResource = R.string.toast_video_capture_success, + stringResource = R.string.toast_video_capture_failure, withDismissAction = true, - testTag = VIDEO_CAPTURE_SUCCESS_TAG + testTag = VIDEO_CAPTURE_FAILURE_TAG ) } } - is OnVideoRecordEvent.OnVideoRecordError -> { - Log.d(TAG, "cameraSystem.startRecording OnVideoRecordError") - captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(it.error)) - snackbarToShow = SnackbarData( - cookie = cookie, - stringResource = R.string.toast_video_capture_failure, - withDismissAction = true, - testTag = VIDEO_CAPTURE_FAILURE_TAG - ) + snackbarToShow?.let { data -> + snackBarController?.addSnackBarData(data) } + multipleEventsCutter.done() } - - snackbarToShow?.let { data -> - snackBarController?.addSnackBarData(data) - } + Log.d(TAG, "cameraSystem.startRecording success") + } catch (exception: IllegalStateException) { + Log.d(TAG, "cameraSystem.startVideoRecording error", exception) + multipleEventsCutter.done() } - Log.d(TAG, "cameraSystem.startRecording success") - } catch (exception: IllegalStateException) { - Log.d(TAG, "cameraSystem.startVideoRecording error", exception) } } } diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt new file mode 100644 index 000000000..19f092c92 --- /dev/null +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.ui.controller.impl + +import kotlinx.atomicfu.atomic + +/** + * A helper class that prevents multiple clicks. + */ +internal class MultipleEventsCutter { + private val isProcessing = atomic(false) + + fun processEvent(event: () -> Unit) { + if (isProcessing.compareAndSet(expect = false, update = true)) { + event() + } + } + + fun done() { + isProcessing.value = false + } +} From dcbd38850c94c56563f60a5f93870539e6bb1414 Mon Sep 17 00:00:00 2001 From: davidjiagoogle Date: Fri, 3 Apr 2026 18:26:57 +0000 Subject: [PATCH 2/2] Address PR #491 comments: improve error handling and add KDoc --- .../controller/impl/CaptureControllerImpl.kt | 114 ++++++++++-------- .../controller/impl/MultipleEventsCutter.kt | 6 + 2 files changed, 67 insertions(+), 53 deletions(-) diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt index 46e1b4062..deebd919b 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/CaptureControllerImpl.kt @@ -106,56 +106,62 @@ class CaptureControllerImpl( multipleEventsCutter.processEvent { Log.d(TAG, "captureImage") scope.launch { - val (saveLocation, progress) = nextSaveLocation( - saveMode, - externalCaptureMode, - externalCapturesCallback - ) - captureImageInternal( - saveLocation = saveLocation, - doTakePicture = { - cameraSystem.takePicture(contentResolver, saveLocation) { - trackedCaptureUiState.update { old -> - old.copy(lastBlinkTimeStamp = System.currentTimeMillis()) - } - }.savedUri - }, - onSuccess = { savedUri -> - val event = if (progress != null) { - ImageCaptureEvent.SequentialImageSaved(savedUri, progress) - } else { - if (saveLocation is SaveLocation.Cache) { - ImageCaptureEvent.SingleImageCached(savedUri) + try { + val (saveLocation, progress) = nextSaveLocation( + saveMode, + externalCaptureMode, + externalCapturesCallback + ) + captureImageInternal( + saveLocation = saveLocation, + doTakePicture = { + cameraSystem.takePicture(contentResolver, saveLocation) { + trackedCaptureUiState.update { old -> + old.copy(lastBlinkTimeStamp = System.currentTimeMillis()) + } + }.savedUri + }, + onSuccess = { savedUri -> + val event = if (progress != null) { + ImageCaptureEvent.SequentialImageSaved(savedUri, progress) } else { - ImageCaptureEvent.SingleImageSaved(savedUri) + if (saveLocation is SaveLocation.Cache) { + ImageCaptureEvent.SingleImageCached(savedUri) + } else { + ImageCaptureEvent.SingleImageSaved(savedUri) + } } - } - if (saveLocation !is SaveLocation.Cache) { - imageWellController.updateLastCapturedMedia() - } else { - savedUri?.let { - scope.launch { - postCurrentMediaToMediaRepository( - mediaRepository, - MediaDescriptor.Content.Image(it, null, true) - ) + if (saveLocation !is SaveLocation.Cache) { + imageWellController.updateLastCapturedMedia() + } else { + savedUri?.let { + scope.launch { + postCurrentMediaToMediaRepository( + mediaRepository, + MediaDescriptor.Content.Image(it, null, true) + ) + } } } - } - captureEvents.trySend(event) - multipleEventsCutter.done() - }, - onFailure = { exception -> - val event = if (progress != null) { - ImageCaptureEvent.SequentialImageCaptureError(exception, progress) - } else { - ImageCaptureEvent.SingleImageCaptureError(exception) - } + captureEvents.trySend(event) + multipleEventsCutter.done() + }, + onFailure = { exception -> + val event = if (progress != null) { + ImageCaptureEvent.SequentialImageCaptureError(exception, progress) + } else { + ImageCaptureEvent.SingleImageCaptureError(exception) + } - captureEvents.trySend(event) - multipleEventsCutter.done() - } - ) + captureEvents.trySend(event) + multipleEventsCutter.done() + } + ) + } catch (exception: Exception) { + Log.e(TAG, "captureImage error: $exception") + multipleEventsCutter.done() + captureEvents.trySend(ImageCaptureEvent.SingleImageCaptureError(exception)) + } } } } @@ -177,12 +183,12 @@ class CaptureControllerImpl( multipleEventsCutter.processEvent { recordingJob = scope.launch { val cookie = "Video-${videoCaptureStartedCount.incrementAndGet()}" - val (saveLocation, _) = nextSaveLocation( - saveMode, - externalCaptureMode, - externalCapturesCallback - ) try { + val (saveLocation, _) = nextSaveLocation( + saveMode, + externalCaptureMode, + externalCapturesCallback + ) cameraSystem.startVideoRecording(saveLocation) { var snackbarToShow: SnackbarData? when (it) { @@ -217,6 +223,7 @@ class CaptureControllerImpl( testTag = VIDEO_CAPTURE_SUCCESS_TAG ) } + multipleEventsCutter.done() } is OnVideoRecordEvent.OnVideoRecordError -> { @@ -228,18 +235,19 @@ class CaptureControllerImpl( withDismissAction = true, testTag = VIDEO_CAPTURE_FAILURE_TAG ) + multipleEventsCutter.done() } } snackbarToShow?.let { data -> snackBarController?.addSnackBarData(data) } - multipleEventsCutter.done() } Log.d(TAG, "cameraSystem.startRecording success") - } catch (exception: IllegalStateException) { - Log.d(TAG, "cameraSystem.startVideoRecording error", exception) + } catch (exception: Exception) { + Log.e(TAG, "cameraSystem.startVideoRecording error", exception) multipleEventsCutter.done() + captureEvents.trySend(VideoCaptureEvent.VideoCaptureError(exception)) } } } diff --git a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt index 19f092c92..c8c92a47d 100644 --- a/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt +++ b/ui/controller/impl/src/main/java/com/google/jetpackcamera/ui/controller/impl/MultipleEventsCutter.kt @@ -23,12 +23,18 @@ import kotlinx.atomicfu.atomic internal class MultipleEventsCutter { private val isProcessing = atomic(false) + /** + * Executes the [event] if no other event is currently being processed. + */ fun processEvent(event: () -> Unit) { if (isProcessing.compareAndSet(expect = false, update = true)) { event() } } + /** + * Signals that the current event processing is complete. + */ fun done() { isProcessing.value = false }