From d5aba1e1ed11070617f07637f77aa659cb92fe23 Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Sat, 10 Aug 2024 18:51:19 -0400 Subject: [PATCH] Split pure silence warning notification to separate channel The warning can sometimes be triggered when hanging up before the other party is able to answer, so allow the user to disable the notification if desired. This commit also changes the recorder thread status reporting to be more explicit and type-safe. All user-facing error messages are now computed in the UI layers instead of the recorder thread. Closes: #579 Signed-off-by: Andrew Gunnerson --- .../java/com/chiller3/bcr/Notifications.kt | 31 +++++ .../com/chiller3/bcr/RecorderInCallService.kt | 68 ++++++----- .../java/com/chiller3/bcr/RecorderThread.kt | 112 ++++++++++-------- app/src/main/res/values/strings.xml | 2 + 4 files changed, 133 insertions(+), 80 deletions(-) diff --git a/app/src/main/java/com/chiller3/bcr/Notifications.kt b/app/src/main/java/com/chiller3/bcr/Notifications.kt index 0a32ce047..8461a96ae 100644 --- a/app/src/main/java/com/chiller3/bcr/Notifications.kt +++ b/app/src/main/java/com/chiller3/bcr/Notifications.kt @@ -29,6 +29,7 @@ class Notifications( const val CHANNEL_ID_PERSISTENT = "persistent" const val CHANNEL_ID_FAILURE = "failure" const val CHANNEL_ID_SUCCESS = "success" + const val CHANNEL_ID_SILENCE = "silence" private val LEGACY_CHANNEL_IDS = arrayOf("alerts") @@ -106,6 +107,17 @@ class Notifications( description = context.getString(R.string.notification_channel_success_desc) } + /** + * Create a high priority notification channel for pure silence warnings. + */ + private fun createPureSilenceWarningsChannel() = NotificationChannel( + CHANNEL_ID_SILENCE, + context.getString(R.string.notification_channel_silence_name), + NotificationManager.IMPORTANCE_HIGH, + ).apply { + description = context.getString(R.string.notification_channel_silence_desc) + } + /** * Ensure notification channels are up-to-date. * @@ -116,6 +128,7 @@ class Notifications( createPersistentChannel(), createFailureAlertsChannel(), createSuccessAlertsChannel(), + createPureSilenceWarningsChannel(), )) LEGACY_CHANNEL_IDS.forEach { notificationManager.deleteNotificationChannel(it) } } @@ -341,6 +354,24 @@ class Notifications( vibrateIfEnabled(CHANNEL_ID_FAILURE) } + /** + * Send a pure silence warning notification. + * + * This will explicitly vibrate the device if the user enabled vibration for + * [CHANNEL_ID_SILENCE]. This is necessary because Android itself will not vibrate for a + * notification during a phone call. + */ + fun notifyRecordingPureSilence(packageName: String) { + sendRecordingNotification( + CHANNEL_ID_SILENCE, + R.string.notification_recording_failed, + context.getString(R.string.notification_pure_silence_error, packageName), + null, + emptyList(), + ) + vibrateIfEnabled(CHANNEL_ID_SILENCE) + } + /** Send a direct boot file migration failure alert notification. */ fun notifyMigrationFailure(errorMsg: String?) { val notificationId = prefs.nextNotificationId diff --git a/app/src/main/java/com/chiller3/bcr/RecorderInCallService.kt b/app/src/main/java/com/chiller3/bcr/RecorderInCallService.kt index 120889e6e..f29bbb139 100644 --- a/app/src/main/java/com/chiller3/bcr/RecorderInCallService.kt +++ b/app/src/main/java/com/chiller3/bcr/RecorderInCallService.kt @@ -243,7 +243,7 @@ class RecorderInCallService : InCallService(), RecorderThread.OnRecordingComplet val recorder = try { RecorderThread(this, this, call) } catch (e: Exception) { - notifyFailure(e.message, null, emptyList()) + notifications.notifyRecordingFailure(e.message, null, emptyList()) throw e } callsToRecorders[call] = recorder @@ -410,18 +410,6 @@ class RecorderInCallService : InCallService(), RecorderThread.OnRecordingComplet } } - private fun notifySuccess(file: OutputFile, additionalFiles: List) { - notifications.notifyRecordingSuccess(file, additionalFiles) - } - - private fun notifyFailure( - errorMsg: String?, - file: OutputFile?, - additionalFiles: List, - ) { - notifications.notifyRecordingFailure(errorMsg, file, additionalFiles) - } - private fun onRecorderExited(recorder: RecorderThread) { // This may be an early exit if an error occurred while recording. Remove from the map to // make sure the thread doesn't receive any more call-related callbacks. @@ -449,30 +437,48 @@ class RecorderInCallService : InCallService(), RecorderThread.OnRecordingComplet thread: RecorderThread, file: OutputFile?, additionalFiles: List, + status: RecorderThread.Status, ) { - Log.i(TAG, "Recording completed: ${thread.id}: ${file?.redacted}") + Log.i(TAG, "Recording completed: ${thread.id}: ${file?.redacted}: $status") handler.post { onRecorderExited(thread) - // If the recording was initially paused and the user never resumed it, there's no - // output file, so nothing needs to be shown. - if (file != null) { - notifySuccess(file, additionalFiles) - } - } - } + when (status) { + RecorderThread.Status.Succeeded -> { + notifications.notifyRecordingSuccess(file!!, additionalFiles) + } + is RecorderThread.Status.Failed -> { + val message = buildString { + when (status.component) { + is RecorderThread.FailureComponent.AndroidMedia -> { + val frame = status.component.stackFrame + + append(getString(R.string.notification_internal_android_error, + "${frame.className}.${frame.methodName}")) + } + RecorderThread.FailureComponent.Other -> {} + } - override fun onRecordingFailed( - thread: RecorderThread, - errorMsg: String?, - file: OutputFile?, - additionalFiles: List, - ) { - Log.w(TAG, "Recording failed: ${thread.id}: ${file?.redacted}") - handler.post { - onRecorderExited(thread) + status.exception?.localizedMessage?.let { + if (isNotEmpty()) { + append("\n\n") + } + append(it) + } + } - notifyFailure(errorMsg, file, additionalFiles) + notifications.notifyRecordingFailure(message, file, additionalFiles) + } + is RecorderThread.Status.Discarded -> { + when (status.reason) { + RecorderThread.DiscardReason.Intentional -> {} + is RecorderThread.DiscardReason.Silence -> { + notifications.notifyRecordingPureSilence(status.reason.callPackage) + } + } + } + RecorderThread.Status.Cancelled -> {} + } } } } diff --git a/app/src/main/java/com/chiller3/bcr/RecorderThread.kt b/app/src/main/java/com/chiller3/bcr/RecorderThread.kt index 17d6a5fb8..646d54876 100644 --- a/app/src/main/java/com/chiller3/bcr/RecorderThread.kt +++ b/app/src/main/java/com/chiller3/bcr/RecorderThread.kt @@ -5,7 +5,6 @@ import android.content.Context import android.media.AudioFormat import android.media.AudioRecord import android.media.MediaRecorder -import android.net.Uri import android.os.ParcelFileDescriptor import android.system.Os import android.telecom.Call @@ -36,7 +35,7 @@ import com.chiller3.bcr.rule.RecordRule import org.json.JSONObject import java.lang.Process import java.nio.ByteBuffer -import java.time.* +import java.time.Duration import android.os.Process as AndroidProcess /** @@ -72,7 +71,6 @@ class RecorderThread( @Volatile var state = State.NOT_STARTED private set @Volatile private var isCancelled = false - private var captureFailed = false /** * Whether to preserve the recording. @@ -179,8 +177,7 @@ class RecorderThread( } override fun run() { - var success = false - var errorMsg: String? = null + var status: Status = Status.Cancelled var outputDocFile: DocumentFile? = null val additionalFiles = ArrayList() @@ -208,6 +205,8 @@ class RecorderThread( recordingInfo = recordUntilCancelled(it) Os.fsync(it.fileDescriptor) } + + status = Status.Succeeded } finally { state = State.FINALIZING listener.onRecordingStateChanged(this) @@ -231,37 +230,36 @@ class RecorderThread( Log.i(tag, "Deleting recording: $finalPath") outputDocFile!!.delete() outputDocFile = null + + status = Status.Discarded(DiscardReason.Intentional) } processRetention() } - - success = !captureFailed } } catch (e: Exception) { Log.e(tag, "Error during recording", e) - errorMsg = if (e is PureSilenceException) { + if (e is PureSilenceException) { outputDocFile?.delete() outputDocFile = null additionalFiles.forEach { it.toDocumentFile(context).delete() } + additionalFiles.clear() - context.getString(R.string.notification_pure_silence_error, - parentCall.details.accountHandle.componentName.packageName) + val packageName = parentCall.details.accountHandle.componentName.packageName + status = Status.Discarded(DiscardReason.Silence(packageName)) } else { - buildString { - val elem = e.stackTrace.find { it.className.startsWith("android.media.") } - if (elem != null) { - append(context.getString(R.string.notification_internal_android_error, - "${elem.className}.${elem.methodName}")) - append("\n\n") - } - - append(e.localizedMessage) + val mediaFrame = e.stackTrace.find { it.className.startsWith("android.media.") } + val component = if (mediaFrame != null) { + FailureComponent.AndroidMedia(mediaFrame) + } else { + FailureComponent.Other } + + status = Status.Failed(component, e) } } finally { Log.i(tag, "Recording thread completed") @@ -271,7 +269,7 @@ class RecorderThread( // Log files are always kept when an error occurs to avoid the hassle of having the // user manually enable debug mode and needing to reproduce the problem. - if (prefs.isDebugMode || errorMsg != null) { + if (prefs.isDebugMode || status is Status.Failed) { additionalFiles.add(logcatOutput) } else { Log.d(tag, "No need to preserve logcat") @@ -291,12 +289,7 @@ class RecorderThread( state = State.COMPLETED listener.onRecordingStateChanged(this) - - if (success) { - listener.onRecordingCompleted(this, outputFile, additionalFiles) - } else { - listener.onRecordingFailed(this, errorMsg, outputFile, additionalFiles) - } + listener.onRecordingCompleted(this, outputFile, additionalFiles, status) } } @@ -306,8 +299,7 @@ class RecorderThread( * output file. * * If called before [start], the thread will not record any audio not create an output file. In - * this scenario, [OnRecordingCompletedListener.onRecordingFailed] will be called with a null - * [Uri]. + * this scenario, the status will be reported as [Status.Cancelled]. */ fun cancel() { Log.d(tag, "Requested cancellation") @@ -581,6 +573,7 @@ class RecorderThread( var bufferOverruns = 0 var wasEverPaused = false var wasEverHolding = false + var wasReadSamplesError = false var wasPureSilence = true val frameSize = audioRecord.format.frameSizeInBytesCompat @@ -605,7 +598,7 @@ class RecorderThread( if (n < 0) { Log.e(tag, "Error when reading samples from $audioRecord: $n") isCancelled = true - captureFailed = true + wasReadSamplesError = true } else if (n == 0) { // Wait for the wall clock equivalent of the minimum buffer size sleep(bufferNs / 1_000_000L / factor) @@ -655,7 +648,9 @@ class RecorderThread( } } - if (wasPureSilence) { + if (wasReadSamplesError) { + throw ReadSamplesException() + } else if (wasPureSilence) { throw PureSilenceException() } @@ -713,9 +708,34 @@ class RecorderThread( } } + private class ReadSamplesException(cause: Throwable? = null) + : Exception("Failed to read audio samples", cause) + private class PureSilenceException(cause: Throwable? = null) : Exception("Audio contained pure silence", cause) + sealed interface FailureComponent { + data class AndroidMedia(val stackFrame: StackTraceElement) : FailureComponent + + data object Other : FailureComponent + } + + sealed interface DiscardReason { + data object Intentional : DiscardReason + + data class Silence(val callPackage: String) : DiscardReason + } + + sealed interface Status { + data object Succeeded : Status + + data class Failed(val component: FailureComponent, val exception: Exception?) : Status + + data class Discarded(val reason: DiscardReason) : Status + + data object Cancelled : Status + } + interface OnRecordingCompletedListener { /** * Called when the pause state, keep state, or output filename are changed. @@ -723,32 +743,26 @@ class RecorderThread( fun onRecordingStateChanged(thread: RecorderThread) /** - * Called when the recording completes successfully. [file] is the output file. If [file] is - * null, then the recording was started in the paused state and the output file was deleted - * because the user never resumed it. + * Called when the recording completes, successfully or otherwise. [file] is the output + * file. * - * [additionalFiles] are additional files associated with the main output file and should be - * deleted along with the main file. - */ - fun onRecordingCompleted( - thread: RecorderThread, - file: OutputFile?, - additionalFiles: List, - ) - - /** - * Called when an error occurs during recording. If [file] is not null, it points to the - * output file containing partially recorded audio. If [file] is null, then either the - * output file could not be created or the thread was cancelled before it was started. + * [file] may be null in several scenarios: + * * The call matched a record rule that defaults to discarding the recording + * * The user intentionally chose to discard the recording via the notification + * * The output file could not be created + * * The thread was cancelled before it started + * + * Note that for most errors, the output file is *not* deleted. * * [additionalFiles] are additional files associated with the main output file and should be - * deleted along with the main file. + * deleted along with the main file if the user chooses to do so via the notification. These + * files may exist even if [file] is null (eg. the log file). */ - fun onRecordingFailed( + fun onRecordingCompleted( thread: RecorderThread, - errorMsg: String?, file: OutputFile?, additionalFiles: List, + status: Status, ) } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index ba441c66a..95c3de514 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -120,6 +120,8 @@ Alerts for errors during call recording Success alerts Alerts for successful call recordings + Pure silence warnings + Warn if call recordings contained pure silence Failed to migrate recordings Some recordings saved before the device was initially unlocked could not be moved to the output directory. Call recording initializing