Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR] Fix crashing MediaCodec and use density to determine recording resolution #3327

Merged
merged 5 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions sentry-android-replay/api/sentry-android-replay.api
Original file line number Diff line number Diff line change
Expand Up @@ -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 <init> (IIFII)V
public fun <init> (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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ class ReplayIntegration(
private val recorderConfig by lazy(NONE) {
ScreenshotRecorderConfig.from(
context,
targetHeight = 720,
options.experimental.sessionReplayOptions
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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")
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,6 @@ internal class WindowRecorder(
return
}

// val (height, width) = (wm.currentWindowMetrics.bounds.bottom /
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be safely removed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah red lines means these have been removed, haha ;)

// 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -49,35 +50,82 @@ 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) {
var bitRate = muxerConfig.recorderConfig.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()
// 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()
Expand Down Expand Up @@ -173,13 +221,17 @@ internal class SimpleVideoEncoder(
}

fun release() {
onClose?.invoke()
drainCodec(true)
mediaCodec.stop()
mediaCodec.release()
surface?.release()

frameMuxer.release()
try {
onClose?.invoke()
drainCodec(true)
mediaCodec.stop()
mediaCodec.release()
surface?.release()

frameMuxer.release()
} catch (e: Throwable) {
options.logger.log(DEBUG, "Failed to properly release video encoder", e)
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
4 changes: 2 additions & 2 deletions sentry/src/main/java/io/sentry/SentryReplayOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ 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 = 20_000;
private int bitRate = 100_000;

/**
* Number of frames per second of the replay. The bigger the number, the more accurate the replay
Expand Down
Loading