From 1be87994b13ffece5eb7a0790303ff57e9da2de6 Mon Sep 17 00:00:00 2001
From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com>
Date: Sun, 9 Aug 2026 04:08:56 +0300
Subject: [PATCH] Never re-encode a video at a higher bitrate than the source

The compression config computed a target bitrate purely from the output
resolution and frame rate, ignoring the bitrate already extracted from the
source. A clip that is small because it was efficiently encoded, rather
than because it is short, was therefore re-encoded upwards: the upload got
slower and the resulting file was larger than the original.

The target is now clamped to the source bitrate when that is known.
METADATA_KEY_BITRATE is the container total, video plus audio, so it is a
safe upper bound for the video track alone. An unknown or zero bitrate
still falls back to the calculated value.

Relates to https://github.com/element-hq/element-x-android/issues/4191
---
 .../mediaupload/impl/VideoCompressorConfig.kt | 12 +++++-
 .../impl/VideoCompressorConfigFactoryTest.kt  | 42 +++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfig.kt b/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfig.kt
index 152678261c..fdde89af77 100644
--- a/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfig.kt
+++ b/libraries/mediaupload/impl/src/main/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfig.kt
@@ -34,11 +34,19 @@ internal object VideoCompressorConfigFactory {
         val newFrameRate = min(originalFrameRate, DEFAULT_FRAME_RATE)
 
         // If we need to resize the video, we also want to recalculate the bitrate
-        val newBitrate = resizer.calculateOptimalBitrate(Size(width, height), newFrameRate)
+        val optimalBitrate = resizer.calculateOptimalBitrate(Size(width, height), newFrameRate).toInt()
+
+        // Never ask the encoder for more than the source already carries: re-encoding a video that is
+        // already efficiently encoded at a low bitrate otherwise inflates it, making the upload both
+        // slower and bigger than sending the original. METADATA_KEY_BITRATE is the container total,
+        // video plus audio, so it is a safe upper bound for the video track alone.
+        // See https://github.com/element-hq/element-x-android/issues/4191
+        val sourceBitrate = metadata?.bitrate?.takeIf { it > 0 }
+        val newBitrate = sourceBitrate?.let { min(optimalBitrate.toLong(), it).toInt() } ?: optimalBitrate
 
         return VideoCompressorConfig(
             videoCompressorHelper = resizer,
-            newBitRate = newBitrate.toInt(),
+            newBitRate = newBitrate,
             newFrameRate = newFrameRate,
         )
     }
diff --git a/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfigFactoryTest.kt b/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfigFactoryTest.kt
index 6af0956541..52e6b6f0c3 100644
--- a/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfigFactoryTest.kt
+++ b/libraries/mediaupload/impl/src/test/kotlin/io/element/android/libraries/mediaupload/impl/VideoCompressorConfigFactoryTest.kt
@@ -33,6 +33,48 @@ class VideoCompressorConfigFactoryTest : RobolectricTest() {
         assertThat(videoCompressorConfig.newBitRate).isNotEqualTo(VideoEncoderSettings.NO_VALUE)
     }
 
+    @Test
+    fun `the bitrate is clamped to the source bitrate when the source is already low`() {
+        // A 720x720 clip encoded at 615 kbps: the calculated optimal is far higher, which would
+        // re-encode it into a much bigger file.
+        val metadata = VideoFileMetadata(width = 720, height = 720, bitrate = 615_000, frameRate = 25, rotation = 0)
+
+        val videoCompressorConfig = VideoCompressorConfigFactory.create(
+            metadata = metadata,
+            preset = VideoCompressionPreset.STANDARD,
+        )
+
+        assertThat(videoCompressorConfig.newBitRate).isEqualTo(615_000)
+    }
+
+    @Test
+    fun `the bitrate is not raised for a high bitrate source`() {
+        val metadata = VideoFileMetadata(width = 1920, height = 1080, bitrate = 20_000_000, frameRate = 30, rotation = 0)
+
+        val videoCompressorConfig = VideoCompressorConfigFactory.create(
+            metadata = metadata,
+            preset = VideoCompressionPreset.STANDARD,
+        )
+
+        assertThat(videoCompressorConfig.newBitRate).isLessThan(20_000_000)
+        assertThat(videoCompressorConfig.newBitRate).isGreaterThan(0)
+    }
+
+    @Test
+    fun `an unknown bitrate falls back to the calculated optimal`() {
+        val unknown = VideoCompressorConfigFactory.create(
+            metadata = VideoFileMetadata(width = 1280, height = 720, bitrate = -1, frameRate = 30, rotation = 0),
+            preset = VideoCompressionPreset.STANDARD,
+        )
+        val zero = VideoCompressorConfigFactory.create(
+            metadata = VideoFileMetadata(width = 1280, height = 720, bitrate = 0, frameRate = 30, rotation = 0),
+            preset = VideoCompressionPreset.STANDARD,
+        )
+
+        assertThat(unknown.newBitRate).isGreaterThan(0)
+        assertThat(zero.newBitRate).isEqualTo(unknown.newBitRate)
+    }
+
     @Test
     fun `if the video should be compressed and is larger than 720p it will be resized`() {
         // Given