mirror of
https://github.com/Novaloop-AG/element-x-android.git
synced 2026-10-31 16:22:49 +00:00
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
This commit is contained in:
+10
-2
@@ -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,
|
||||
)
|
||||
}
|
||||
|
||||
+42
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user