diff --git a/CHANGELOG.md b/CHANGELOG.md index 893779a9a3..c71f589a2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixes - Fix warm start detection ([#3937](https://github.com/getsentry/sentry-java/pull/3937)) +- Session Replay: Reduce memory allocations, disk space consumption, and payload size ([#4016](https://github.com/getsentry/sentry-java/pull/4016)) +- Session Replay: Do not try to encode corrupted frames multiple times ([#4016](https://github.com/getsentry/sentry-java/pull/4016)) ### Internal diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt index a757c4b455..8031455c68 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt @@ -85,7 +85,7 @@ public class ReplayCache( it.createNewFile() } screenshot.outputStream().use { - bitmap.compress(JPEG, 80, it) + bitmap.compress(JPEG, options.experimental.sessionReplay.quality.screenshotQuality, it) it.flush() } @@ -162,7 +162,7 @@ public class ReplayCache( val step = 1000 / frameRate.toLong() var frameCount = 0 - var lastFrame: ReplayFrame = frames.first() + var lastFrame: ReplayFrame? = frames.first() for (timestamp in from until (from + (duration)) step step) { val iter = frames.iterator() while (iter.hasNext()) { @@ -182,6 +182,12 @@ public class ReplayCache( // to respect the video duration if (encode(lastFrame)) { frameCount++ + } else if (lastFrame != null) { + // if we failed to encode the frame, we delete the screenshot right away as the + // likelihood of it being able to be encoded later is low + deleteFile(lastFrame.screenshot) + frames.remove(lastFrame) + lastFrame = null } } @@ -206,7 +212,10 @@ public class ReplayCache( return GeneratedVideo(videoFile, frameCount, videoDuration) } - private fun encode(frame: ReplayFrame): Boolean { + private fun encode(frame: ReplayFrame?): Boolean { + if (frame == null) { + return false + } return try { val bitmap = BitmapFactory.decodeFile(frame.screenshot.absolutePath) synchronized(encoderLock) { 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 6f588fe779..ba3cfc7115 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 @@ -3,7 +3,6 @@ package io.sentry.android.replay import android.annotation.TargetApi import android.content.Context import android.graphics.Bitmap -import android.graphics.Bitmap.Config.ARGB_8888 import android.graphics.Canvas import android.graphics.Color import android.graphics.Matrix @@ -54,9 +53,14 @@ internal class ScreenshotRecorder( Bitmap.createBitmap( 1, 1, - Bitmap.Config.ARGB_8888 + Bitmap.Config.RGB_565 ) } + private val screenshot = Bitmap.createBitmap( + config.recordingWidth, + config.recordingHeight, + Bitmap.Config.RGB_565 + ) private val singlePixelBitmapCanvas: Canvas by lazy(NONE) { Canvas(singlePixelBitmap) } private val prescaledMatrix by lazy(NONE) { Matrix().apply { @@ -65,7 +69,7 @@ internal class ScreenshotRecorder( } private val contentChanged = AtomicBoolean(false) private val isCapturing = AtomicBoolean(true) - private var lastScreenshot: Bitmap? = null + private val lastCaptureSuccessful = AtomicBoolean(false) fun capture() { if (!isCapturing.get()) { @@ -73,14 +77,10 @@ internal class ScreenshotRecorder( return } - if (!contentChanged.get() && lastScreenshot != null && !lastScreenshot!!.isRecycled) { + if (!contentChanged.get() && lastCaptureSuccessful.get()) { options.logger.log(DEBUG, "Content hasn't changed, repeating last known frame") - lastScreenshot?.let { - screenshotRecorderCallback?.onScreenshotRecorded( - it.copy(ARGB_8888, false) - ) - } + screenshotRecorderCallback?.onScreenshotRecorded(screenshot) return } @@ -96,38 +96,33 @@ internal class ScreenshotRecorder( return } - val bitmap = Bitmap.createBitmap( - config.recordingWidth, - config.recordingHeight, - Bitmap.Config.ARGB_8888 - ) - // postAtFrontOfQueue to ensure the view hierarchy and bitmap are ase close in-sync as possible mainLooperHandler.post { try { contentChanged.set(false) PixelCopy.request( window, - bitmap, + screenshot, { copyResult: Int -> if (copyResult != PixelCopy.SUCCESS) { options.logger.log(INFO, "Failed to capture replay recording: %d", copyResult) - bitmap.recycle() + lastCaptureSuccessful.set(false) return@request } // TODO: handle animations with heuristics (e.g. if we fall under this condition 2 times in a row, we should capture) if (contentChanged.get()) { options.logger.log(INFO, "Failed to determine view hierarchy, not capturing") - bitmap.recycle() + lastCaptureSuccessful.set(false) return@request } + // TODO: disableAllMasking here and dont traverse? val viewHierarchy = ViewHierarchyNode.fromView(root, null, 0, options) root.traverse(viewHierarchy, options) recorder.submitSafely(options, "screenshot_recorder.mask") { - val canvas = Canvas(bitmap) + val canvas = Canvas(screenshot) canvas.setMatrix(prescaledMatrix) viewHierarchy.traverse { node -> if (node.shouldMask && (node.width > 0 && node.height > 0)) { @@ -141,7 +136,7 @@ internal class ScreenshotRecorder( val (visibleRects, color) = when (node) { is ImageViewHierarchyNode -> { listOf(node.visibleRect) to - bitmap.dominantColorForRect(node.visibleRect) + screenshot.dominantColorForRect(node.visibleRect) } is TextViewHierarchyNode -> { @@ -168,20 +163,16 @@ internal class ScreenshotRecorder( return@traverse true } - val screenshot = bitmap.copy(ARGB_8888, false) screenshotRecorderCallback?.onScreenshotRecorded(screenshot) - lastScreenshot?.recycle() - lastScreenshot = screenshot + lastCaptureSuccessful.set(true) contentChanged.set(false) - - bitmap.recycle() } }, mainLooperHandler.handler ) } catch (e: Throwable) { options.logger.log(WARNING, "Failed to capture replay recording", e) - bitmap.recycle() + lastCaptureSuccessful.set(false) } } } @@ -226,7 +217,7 @@ internal class ScreenshotRecorder( fun close() { unbind(rootView?.get()) rootView?.clear() - lastScreenshot?.recycle() + screenshot.recycle() isCapturing.set(false) } 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 c7529ac1f6..b2c8836d40 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 @@ -25,6 +25,7 @@ import org.junit.Rule import org.junit.rules.TemporaryFolder import org.junit.runner.RunWith import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowBitmapFactory import java.io.File import kotlin.test.BeforeTest import kotlin.test.Test @@ -61,6 +62,7 @@ class ReplayCacheTest { @BeforeTest fun `set up`() { ReplayShadowMediaCodec.framesToEncode = 5 + ShadowBitmapFactory.setAllowInvalidImageData(true) } @Test @@ -500,4 +502,28 @@ class ReplayCacheTest { assertEquals(0, lastSegment.id) } + + @Test + fun `when screenshot is corrupted, deletes it immediately`() { + ShadowBitmapFactory.setAllowInvalidImageData(false) + ReplayShadowMediaCodec.framesToEncode = 1 + val replayCache = fixture.getSut( + tmpDir + ) + + val bitmap = Bitmap.createBitmap(1, 1, ARGB_8888) + replayCache.addFrame(bitmap, 1) + + // corrupt the image + File(replayCache.replayCacheDir, "1.jpg").outputStream().use { + it.write(Int.MIN_VALUE) + it.flush() + } + + val segment0 = replayCache.createVideoOf(3000L, 0, 0, 100, 200, 1, 20_000) + assertNull(segment0) + + assertTrue(replayCache.frames.isEmpty()) + assertTrue(replayCache.replayCacheDir!!.listFiles()!!.none { it.extension == "jpg" }) + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 26f77fe3ac..fa303ba61a 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2759,6 +2759,7 @@ public final class io/sentry/SentryReplayOptions$SentryReplayQuality : java/lang public static final field LOW Lio/sentry/SentryReplayOptions$SentryReplayQuality; public static final field MEDIUM Lio/sentry/SentryReplayOptions$SentryReplayQuality; public final field bitRate I + public final field screenshotQuality I public final field sizeScale F public fun serializedName ()Ljava/lang/String; public static fun valueOf (Ljava/lang/String;)Lio/sentry/SentryReplayOptions$SentryReplayQuality; diff --git a/sentry/src/main/java/io/sentry/SentryReplayOptions.java b/sentry/src/main/java/io/sentry/SentryReplayOptions.java index 396c733815..36afd60879 100644 --- a/sentry/src/main/java/io/sentry/SentryReplayOptions.java +++ b/sentry/src/main/java/io/sentry/SentryReplayOptions.java @@ -21,14 +21,14 @@ public final class SentryReplayOptions { "com.google.android.exoplayer2.ui.StyledPlayerView"; public enum SentryReplayQuality { - /** Video Scale: 80% Bit Rate: 50.000 */ - LOW(0.8f, 50_000), + /** Video Scale: 80% Bit Rate: 50.000 JPEG Compression: 10 */ + LOW(0.8f, 50_000, 10), - /** Video Scale: 100% Bit Rate: 75.000 */ - MEDIUM(1.0f, 75_000), + /** Video Scale: 100% Bit Rate: 75.000 JPEG Compression: 30 */ + MEDIUM(1.0f, 75_000, 30), - /** Video Scale: 100% Bit Rate: 100.000 */ - HIGH(1.0f, 100_000); + /** Video Scale: 100% Bit Rate: 100.000 JPEG Compression: 50 */ + HIGH(1.0f, 100_000, 50); /** The scale related to the window size (in dp) at which the replay will be created. */ public final float sizeScale; @@ -39,9 +39,13 @@ public enum SentryReplayQuality { */ public final int bitRate; - SentryReplayQuality(final float sizeScale, final int bitRate) { + /** Defines the compression quality with which the screenshots are stored to disk. */ + public final int screenshotQuality; + + SentryReplayQuality(final float sizeScale, final int bitRate, final int screenshotQuality) { this.sizeScale = sizeScale; this.bitRate = bitRate; + this.screenshotQuality = screenshotQuality; } public @NotNull String serializedName() {