From 0c5e4b0370aab4196ac7bf331288ad4dd9a01f30 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 5 Apr 2024 10:44:56 +0200 Subject: [PATCH 1/5] Fix crashing MediaCodec and use density to determine recording resolution --- .../api/sentry-android-replay.api | 14 ++-- .../android/replay/ReplayIntegration.kt | 1 - .../android/replay/ScreenshotRecorder.kt | 52 ++++++++++----- .../sentry/android/replay/WindowRecorder.kt | 5 -- .../replay/video/SimpleVideoEncoder.kt | 65 +++++++++++++++---- .../java/io/sentry/SentryReplayOptions.java | 2 +- 6 files changed, 100 insertions(+), 39 deletions(-) diff --git a/sentry-android-replay/api/sentry-android-replay.api b/sentry-android-replay/api/sentry-android-replay.api index cda49e0fd1..32f1891af6 100644 --- a/sentry-android-replay/api/sentry-android-replay.api +++ b/sentry-android-replay/api/sentry-android-replay.api @@ -49,26 +49,28 @@ public abstract interface class io/sentry/android/replay/ScreenshotRecorderCallb public final class io/sentry/android/replay/ScreenshotRecorderConfig { public static final field Companion Lio/sentry/android/replay/ScreenshotRecorderConfig$Companion; - public fun (IIFII)V + public fun (IIFFII)V public final fun component1 ()I public final fun component2 ()I public final fun component3 ()F - public final fun component4 ()I + public final fun component4 ()F public final fun component5 ()I - public final fun copy (IIFII)Lio/sentry/android/replay/ScreenshotRecorderConfig; - public static synthetic fun copy$default (Lio/sentry/android/replay/ScreenshotRecorderConfig;IIFIIILjava/lang/Object;)Lio/sentry/android/replay/ScreenshotRecorderConfig; + public final fun component6 ()I + public final fun copy (IIFFII)Lio/sentry/android/replay/ScreenshotRecorderConfig; + public static synthetic fun copy$default (Lio/sentry/android/replay/ScreenshotRecorderConfig;IIFFIIILjava/lang/Object;)Lio/sentry/android/replay/ScreenshotRecorderConfig; public fun equals (Ljava/lang/Object;)Z public final fun getBitRate ()I public final fun getFrameRate ()I public final fun getRecordingHeight ()I public final fun getRecordingWidth ()I - public final fun getScaleFactor ()F + public final fun getScaleFactorX ()F + public final fun getScaleFactorY ()F public fun hashCode ()I public fun toString ()Ljava/lang/String; } public final class io/sentry/android/replay/ScreenshotRecorderConfig$Companion { - public final fun from (Landroid/content/Context;ILio/sentry/SentryReplayOptions;)Lio/sentry/android/replay/ScreenshotRecorderConfig; + public final fun from (Landroid/content/Context;Lio/sentry/SentryReplayOptions;)Lio/sentry/android/replay/ScreenshotRecorderConfig; } public abstract interface class io/sentry/android/replay/video/SimpleFrameMuxer { diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt index 3079cb47fd..94a6ccf1a4 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayIntegration.kt @@ -70,7 +70,6 @@ class ReplayIntegration( private val recorderConfig by lazy(NONE) { ScreenshotRecorderConfig.from( context, - targetHeight = 720, options.experimental.sessionReplayOptions ) } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt index e92f2a0574..7cba3bf54a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ScreenshotRecorder.kt @@ -30,7 +30,6 @@ import java.lang.ref.WeakReference import java.util.concurrent.atomic.AtomicBoolean import java.util.concurrent.atomic.AtomicReference import kotlin.math.roundToInt -import kotlin.system.measureTimeMillis @TargetApi(26) internal class ScreenshotRecorder( @@ -51,14 +50,14 @@ internal class ScreenshotRecorder( ) private val singlePixelBitmapCanvas: Canvas = Canvas(singlePixelBitmap) private val prescaledMatrix = Matrix().apply { - preScale(config.scaleFactor, config.scaleFactor) + preScale(config.scaleFactorX, config.scaleFactorY) } private val contentChanged = AtomicBoolean(false) private val isCapturing = AtomicBoolean(true) private var lastScreenshot: Bitmap? = null fun capture() { - val viewHierarchy = pendingViewHierarchy.get() + val viewHierarchy = pendingViewHierarchy.getAndSet(null) if (!isCapturing.get()) { options.logger.log(DEBUG, "ScreenshotRecorder is paused, not capturing screenshot") @@ -165,12 +164,9 @@ internal class ScreenshotRecorder( return } - val time = measureTimeMillis { - val rootNode = ViewHierarchyNode.fromView(root) - root.traverse(rootNode) - pendingViewHierarchy.set(rootNode) - } - options.logger.log(DEBUG, "Took %d ms to capture view hierarchy", time) + val rootNode = ViewHierarchyNode.fromView(root) + root.traverse(rootNode) + pendingViewHierarchy.set(rootNode) contentChanged.set(true) } @@ -243,12 +239,29 @@ internal class ScreenshotRecorder( public data class ScreenshotRecorderConfig( val recordingWidth: Int, val recordingHeight: Int, - val scaleFactor: Float, + val scaleFactorX: Float, + val scaleFactorY: Float, val frameRate: Int, val bitRate: Int ) { companion object { - fun from(context: Context, targetHeight: Int, sentryReplayOptions: SentryReplayOptions): ScreenshotRecorderConfig { + /** + * Since codec block size is 16, so we have to adjust the width and height to it, otherwise + * the codec might fail to configure on some devices, see https://cs.android.com/android/platform/superproject/+/master:frameworks/base/media/java/android/media/MediaCodecInfo.java;l=1999-2001 + */ + private fun Int.adjustToBlockSize(): Int { + val remainder = this % 16 + return if (remainder <= 8) { + this - remainder + } else { + this + (16 - remainder) + } + } + + fun from( + context: Context, + sentryReplayOptions: SentryReplayOptions + ): ScreenshotRecorderConfig { // PixelCopy takes screenshots including system bars, so we have to get the real size here val wm = context.getSystemService(Context.WINDOW_SERVICE) as WindowManager val screenBounds = if (VERSION.SDK_INT >= VERSION_CODES.R) { @@ -259,12 +272,21 @@ public data class ScreenshotRecorderConfig( wm.defaultDisplay.getRealSize(screenBounds) Rect(0, 0, screenBounds.x, screenBounds.y) } - val aspectRatio = screenBounds.height().toFloat() / screenBounds.width().toFloat() + + // use the baseline density of 1x (mdpi) + val (height, width) = + (screenBounds.height() / context.resources.displayMetrics.density) + .roundToInt() + .adjustToBlockSize() to + (screenBounds.width() / context.resources.displayMetrics.density) + .roundToInt() + .adjustToBlockSize() return ScreenshotRecorderConfig( - recordingWidth = (targetHeight / aspectRatio).roundToInt(), - recordingHeight = targetHeight, - scaleFactor = targetHeight.toFloat() / screenBounds.height(), + recordingWidth = width, + recordingHeight = height, + scaleFactorX = width.toFloat() / screenBounds.width(), + scaleFactorY = height.toFloat() / screenBounds.height(), frameRate = sentryReplayOptions.frameRate, bitRate = sentryReplayOptions.bitRate ) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt index 743b5f5d89..d5e11936de 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/WindowRecorder.kt @@ -55,11 +55,6 @@ internal class WindowRecorder( return } -// val (height, width) = (wm.currentWindowMetrics.bounds.bottom / -// context.resources.displayMetrics.density).roundToInt() to -// (wm.currentWindowMetrics.bounds.right / -// context.resources.displayMetrics.density).roundToInt() - recorder = ScreenshotRecorder(recorderConfig, options, screenshotRecorderCallback) rootViewsSpy.listeners += onRootViewsChangedListener capturingTask = capturer.scheduleAtFixedRateSafely( diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 73b8862434..630637bfbf 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -40,6 +40,7 @@ import io.sentry.SentryOptions import io.sentry.android.replay.ScreenshotRecorderConfig import java.io.File import java.nio.ByteBuffer +import kotlin.LazyThreadSafetyMode.NONE private const val TIMEOUT_USEC = 100_000L @@ -49,35 +50,77 @@ internal class SimpleVideoEncoder( val muxerConfig: MuxerConfig, val onClose: (() -> Unit)? = null ) { - private val mediaFormat: MediaFormat = run { + + internal val mediaCodec: MediaCodec = run { + val codec = MediaCodec.createEncoderByType(muxerConfig.mimeType) + + codec + } + + private val mediaFormat: MediaFormat by lazy(NONE) { + val videoCapabilities = mediaCodec.codecInfo + .getCapabilitiesForType(muxerConfig.mimeType) + .videoCapabilities + + var bitRate = muxerConfig.recorderConfig.bitRate + if (!videoCapabilities.bitrateRange.contains(bitRate)) { + options.logger.log( + DEBUG, + "Encoder doesn't support the provided bitRate: $bitRate, the value will be clamped to the closest one" + ) + bitRate = videoCapabilities.bitrateRange.clamp(bitRate) + } + + // TODO: if this ever becomes a problem, move this to ScreenshotRecorderConfig.from() + // TODO: because the screenshot config has to match the video config + +// var frameRate = muxerConfig.recorderConfig.frameRate +// if (!videoCapabilities.supportedFrameRates.contains(frameRate)) { +// options.logger.log(DEBUG, "Encoder doesn't support the provided frameRate: $frameRate, the value will be clamped to the closest one") +// frameRate = videoCapabilities.supportedFrameRates.clamp(frameRate) +// } + +// var height = muxerConfig.recorderConfig.recordingHeight +// var width = muxerConfig.recorderConfig.recordingWidth +// val aspectRatio = height.toFloat() / width.toFloat() +// while (!videoCapabilities.supportedHeights.contains(height) || !videoCapabilities.supportedWidths.contains(width)) { +// options.logger.log(DEBUG, "Encoder doesn't support the provided height x width: ${height}x${width}, the values will be clamped to the closest ones") +// if (!videoCapabilities.supportedHeights.contains(height)) { +// height = videoCapabilities.supportedHeights.clamp(height) +// width = (height / aspectRatio).roundToInt() +// } else if (!videoCapabilities.supportedWidths.contains(width)) { +// width = videoCapabilities.supportedWidths.clamp(width) +// height = (width * aspectRatio).roundToInt() +// } +// } + val format = MediaFormat.createVideoFormat( muxerConfig.mimeType, muxerConfig.recorderConfig.recordingWidth, muxerConfig.recorderConfig.recordingHeight ) + // this allows reducing bitrate on newer devices, where they enforce higher quality in VBR + // mode, see https://developer.android.com/reference/android/media/MediaCodec#qualityFloor + // TODO: maybe enable this back later, for now variable bitrate seems to provide much better + // TODO: quality with almost no overhead in terms of video size, let's monitor that +// format.setInteger( +// MediaFormat.KEY_BITRATE_MODE, +// MediaCodecInfo.EncoderCapabilities.BITRATE_MODE_CBR +// ) // Set some properties. Failing to specify some of these can cause the MediaCodec // configure() call to throw an unhelpful exception. format.setInteger( MediaFormat.KEY_COLOR_FORMAT, MediaCodecInfo.CodecCapabilities.COLOR_FormatSurface ) - format.setInteger(MediaFormat.KEY_BIT_RATE, muxerConfig.recorderConfig.bitRate) + format.setInteger(MediaFormat.KEY_BIT_RATE, bitRate) format.setFloat(MediaFormat.KEY_FRAME_RATE, muxerConfig.recorderConfig.frameRate.toFloat()) format.setInteger(MediaFormat.KEY_I_FRAME_INTERVAL, 10) format } - internal val mediaCodec: MediaCodec = run { -// val codecs = MediaCodecList(REGULAR_CODECS) -// val codecName = codecs.findEncoderForFormat(mediaFormat) -// val codec = MediaCodec.createByCodecName(codecName) - val codec = MediaCodec.createEncoderByType(muxerConfig.mimeType) - - codec - } - private val bufferInfo: MediaCodec.BufferInfo = MediaCodec.BufferInfo() private val frameMuxer = SimpleMp4FrameMuxer(muxerConfig.file.absolutePath, muxerConfig.recorderConfig.frameRate.toFloat()) val duration get() = frameMuxer.getVideoTime() diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index 51ed1a3a72..c8dc7df7a2 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -24,7 +24,7 @@ public final class SentryReplayOptions { * Defines the quality of the session replay. Higher bit rates have better replay quality, but * also affect the final payload size to transfer, defaults to 20kbps. */ - private int bitRate = 20_000; + private int bitRate = 100_000; /** * Number of frames per second of the replay. The bigger the number, the more accurate the replay From 1fc9aa22e31eaffaf8b48dfac8fb73672276894a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 5 Apr 2024 15:53:25 +0200 Subject: [PATCH 2/5] Fix tests --- .../src/test/java/io/sentry/android/replay/ReplayCacheTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt index 1290689d53..91addc206a 100644 --- a/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt +++ b/sentry-android-replay/src/test/java/io/sentry/android/replay/ReplayCacheTest.kt @@ -37,7 +37,7 @@ class ReplayCacheTest { frameRate: Int, framesToEncode: Int = 0 ): ReplayCache { - val recorderConfig = ScreenshotRecorderConfig(100, 200, 1f, frameRate = frameRate, bitRate = 20_000) + val recorderConfig = ScreenshotRecorderConfig(100, 200, 1f, 1f, frameRate = frameRate, bitRate = 20_000) options.run { cacheDirPath = dir?.newFolder()?.absolutePath } From ad98acd0e7cc31e2e913854a269ffa8f7709d08c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 5 Apr 2024 16:19:04 +0200 Subject: [PATCH 3/5] Fix tests --- .../replay/video/SimpleVideoEncoder.kt | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 630637bfbf..35d3c90541 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -58,17 +58,22 @@ internal class SimpleVideoEncoder( } private val mediaFormat: MediaFormat by lazy(NONE) { - val videoCapabilities = mediaCodec.codecInfo - .getCapabilitiesForType(muxerConfig.mimeType) - .videoCapabilities - var bitRate = muxerConfig.recorderConfig.bitRate - if (!videoCapabilities.bitrateRange.contains(bitRate)) { - options.logger.log( - DEBUG, - "Encoder doesn't support the provided bitRate: $bitRate, the value will be clamped to the closest one" - ) - bitRate = videoCapabilities.bitrateRange.clamp(bitRate) + + try { + val videoCapabilities = mediaCodec.codecInfo + .getCapabilitiesForType(muxerConfig.mimeType) + .videoCapabilities + + if (!videoCapabilities.bitrateRange.contains(bitRate)) { + options.logger.log( + DEBUG, + "Encoder doesn't support the provided bitRate: $bitRate, the value will be clamped to the closest one" + ) + bitRate = videoCapabilities.bitrateRange.clamp(bitRate) + } + } catch (e: Throwable) { + options.logger.log(DEBUG, "Could not retrieve MediaCodec info", e) } // TODO: if this ever becomes a problem, move this to ScreenshotRecorderConfig.from() From 0fca8ad26641c251aa92ba172fd79ef40caa509c Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 5 Apr 2024 22:24:29 +0200 Subject: [PATCH 4/5] Try-catch release of encoder --- .../android/replay/video/SimpleVideoEncoder.kt | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt index 35d3c90541..7d662b7231 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/video/SimpleVideoEncoder.kt @@ -221,13 +221,17 @@ internal class SimpleVideoEncoder( } fun release() { - onClose?.invoke() - drainCodec(true) - mediaCodec.stop() - mediaCodec.release() - surface?.release() + try { + onClose?.invoke() + drainCodec(true) + mediaCodec.stop() + mediaCodec.release() + surface?.release() - frameMuxer.release() + frameMuxer.release() + } catch (e: Throwable) { + options.logger.log(DEBUG, "Failed to properly release video encoder", e) + } } } From 71837c18d77c90be4f91f2f81f949d7961a784e7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 9 Apr 2024 12:04:19 +0200 Subject: [PATCH 5/5] Update sentry/src/main/java/io/sentry/SentryReplayOptions.java Co-authored-by: Markus Hintersteiner --- sentry/src/main/java/io/sentry/SentryReplayOptions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index c8dc7df7a2..3613061572 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -22,7 +22,7 @@ public final class SentryReplayOptions { /** * Defines the quality of the session replay. Higher bit rates have better replay quality, but - * also affect the final payload size to transfer, defaults to 20kbps. + * also affect the final payload size to transfer, defaults to 100kbps. */ private int bitRate = 100_000;