Skip to content

Commit

Permalink
Split pure silence warning notification to separate channel
Browse files Browse the repository at this point in the history 
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 <accounts+github@chiller3.com>
  • Loading branch information
@chenxiaolong
chenxiaolong committed Aug 10, 2024
1 parent ed8ee27 commit d5aba1e
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 80 deletions.
31 changes: 31 additions & 0 deletions app/src/main/java/com/chiller3/bcr/Notifications.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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.
*
Expand All @@ -116,6 +128,7 @@ class Notifications(
createPersistentChannel(),
createFailureAlertsChannel(),
createSuccessAlertsChannel(),
createPureSilenceWarningsChannel(),
))
LEGACY_CHANNEL_IDS.forEach { notificationManager.deleteNotificationChannel(it) }
}
Expand Down Expand Up @@ -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
Expand Down
68 changes: 37 additions & 31 deletions app/src/main/java/com/chiller3/bcr/RecorderInCallService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -410,18 +410,6 @@ class RecorderInCallService : InCallService(), RecorderThread.OnRecordingComplet
}
}

private fun notifySuccess(file: OutputFile, additionalFiles: List<OutputFile>) {
notifications.notifyRecordingSuccess(file, additionalFiles)
}

private fun notifyFailure(
errorMsg: String?,
file: OutputFile?,
additionalFiles: List<OutputFile>,
) {
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.
Expand Down Expand Up @@ -449,30 +437,48 @@ class RecorderInCallService : InCallService(), RecorderThread.OnRecordingComplet
thread: RecorderThread,
file: OutputFile?,
additionalFiles: List<OutputFile>,
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<OutputFile>,
) {
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 -> {}
}
}
}
}
112 changes: 63 additions & 49 deletions app/src/main/java/com/chiller3/bcr/RecorderThread.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

/**
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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<OutputFile>()

Expand Down Expand Up @@ -208,6 +205,8 @@ class RecorderThread(
recordingInfo = recordUntilCancelled(it)
Os.fsync(it.fileDescriptor)
}

status = Status.Succeeded
} finally {
state = State.FINALIZING
listener.onRecordingStateChanged(this)
Expand All @@ -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")
Expand All @@ -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")
Expand All @@ -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)
}
}

Expand All @@ -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")
Expand Down Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -655,7 +648,9 @@ class RecorderThread(
}
}

if (wasPureSilence) {
if (wasReadSamplesError) {
throw ReadSamplesException()
} else if (wasPureSilence) {
throw PureSilenceException()
}

Expand Down Expand Up @@ -713,42 +708,61 @@ 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.
*/
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<OutputFile>,
)

/**
* 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<OutputFile>,
status: Status,
)
}
}
2 changes: 2 additions & 0 deletions app/src/main/res/values/strings.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@
<string name="notification_channel_failure_desc">Alerts for errors during call recording</string>
<string name="notification_channel_success_name">Success alerts</string>
<string name="notification_channel_success_desc">Alerts for successful call recordings</string>
<string name="notification_channel_silence_name">Pure silence warnings</string>
<string name="notification_channel_silence_desc">Warn if call recordings contained pure silence</string>
<string name="notification_direct_boot_migration_failed">Failed to migrate recordings</string>
<string name="notification_direct_boot_migration_error">Some recordings saved before the device was initially unlocked could not be moved to the output directory.</string>
<string name="notification_recording_initializing">Call recording initializing</string>
Expand Down

0 comments on commit d5aba1e

Please sign in to comment.