Skip to content
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -437,13 +437,24 @@ fun ensureVideoDDExtensionForSVC(mediaDesc: MediaDescription) {
}
}

/* The svc codec (av1/vp9) would use a very low bitrate at the beginning and
increase slowly by the bandwidth estimator until it reach the target bitrate. The
process commonly cost more than 10 seconds cause subscriber will get blur video at
the first few seconds. So we use a 70% of target bitrate here as the start bitrate to
eliminate this issue.
*/
private const val startBitrateForSVC = 0.7
/*
* Video codecs use a very low bitrate at the beginning and increase slowly by
* the bandwidth estimator until they reach the target bitrate. The process commonly
* costs more than 10 seconds causing subscribers to get blurry video at the first
* few seconds. We use x-google-start-bitrate to hint the BWE to start higher.
*
* Why 90%: Gives ~10% headroom for bandwidth estimation while starting close to target.
* Why same for all codecs: Target bitrate already accounts for codec efficiency
* (e.g., users set lower targets for VP9/AV1 knowing they're more efficient).
* Why cap at 1 Mbps: Prevents BWE from starting too aggressively on high bitrate tracks.
*/
private const val startBitrateMultiplier = 0.9

/** Maximum x-google-start-bitrate in kbps. 1 Mbps prevents BWE from starting too aggressively. */
private const val maxStartBitrateKbps = 1000L

/** Minimum target bitrate in kbps to apply start bitrate hint. Below this, the hint hurts more than it helps. */
private const val minTargetBitrateKbps = 300L

/**
* @suppress
Expand All @@ -469,14 +480,29 @@ fun ensureCodecBitrates(
?: continue
val codecPayload = rtp.payload

// Skip start bitrate hint for very low bitrate tracks - the hint hurts more than it helps
if (trackBr.maxBitrate < minTargetBitrateKbps) {
continue

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This continue skips more than the start hint: the x-google-max-bitrate write below is bypassed too, so a track with a sub-300 kbps target loses the codec-level cap it got before this PR. An SVC publish registered at 250 kbps previously produced x-google-start-bitrate=175;x-google-max-bitrate=250; now it gets neither. In Rust the equivalent early return is harmless because that munger never writes x-google-max-bitrate, but here it does. I'd gate only the x-google-start-bitrate append on the threshold (in both the existing-fmtp branch and the !fmtpFound one) and keep the max-bitrate write unconditional.

}

val fmtps = media.getFmtps()
// Use 90% of target bitrate, capped at 1 Mbps for camera to prevent BWE from starting too aggressively
// Screen share is not capped since text/UI clarity requires high bitrate from the start
// TODO: dynamically adjust start bitrate based on network conditions (e.g., use previous BWE estimate)
val calculatedStartBitrate = (trackBr.maxBitrate * startBitrateMultiplier).roundToLong()
val startBitrate = if (trackBr.isScreenShare) {

@adrian-niculescu adrian-niculescu Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There is a structural problem with choosing the cap per track: libwebrtc consumes these fmtp params per connection, not per m-section. In the m144 line this SDK ships (144.7559.05), WebRtcVideoSendChannel::ApplyChangedParams reads x-google-start-bitrate/x-google-max-bitrate via GetBitrateConfigForCodec and pushes the result into the shared Call config, where RtpBitrateConfigurator::UpdateWithSdpParameters replaces the stored config unconditionally, last writer wins. ApplyChangedParams even carries a TODO noting codec max bitrate probably should not affect the global call max.

Two consequences:

  1. The camera/screenshare split cannot work as written. With both published, the capped camera value and the uncapped screenshare value land in the same SDP, and whichever m-section applies its send parameters last seeds the single BWE. Depending on order, the uncapped screenshare start overrides the camera cap for the whole call, or the camera value clobbers the screenshare hint and the exemption silently does nothing.

  2. The max-bitrate write is the sharper edge. A default camera publish now emits x-google-max-bitrate=2310, and that value becomes the Call's max_data_rate, a ceiling on total send bandwidth across all tracks. If the camera m-section applies last, a concurrent 3 Mbps screenshare is starved under a 2.3 Mbps connection ceiling. Before this PR the conflict required at least one SVC publication (a lone VP9 screenshare's max could already ceiling a concurrent camera); this change brings it to the default VP8 camera plus screenshare case.

Since the knob is per connection, the fix that actually holds is one connection-level value applied consistently to every video m-section, or dropping the global hints entirely; a uniform cap merely bounds the damage, since tracks with different targets still produce different values and the last-writer variance remains. For the max specifically, consider not writing x-google-max-bitrate for simulcast at all: the per-encoding maxBitrateBps already caps each layer, so the fmtp value only acts as the call-wide ceiling described above. The merged JS change (livekit/client-sdk-js#1987) shares the start-bitrate interaction, though not the max one (the JS munger never writes x-google-max-bitrate), so this is probably a cross-SDK decision rather than an Android-only fix.

calculatedStartBitrate
} else {
minOf(calculatedStartBitrate, maxStartBitrateKbps)

@adrian-niculescu adrian-niculescu Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This cap regresses the configured start for high-bitrate SVC cameras: previously 0.7x with no ceiling, so a 3 Mbps VP9 camera publish started at 2100 kbps; now it starts at 1000. Above roughly 1.4 Mbps targets the new start is lower than the old one, which can lengthen the ramp on links that can carry the target (on constrained links the lower seed is arguably safer). Screen shares are exempt and go up (2100 to 2700), so it only affects camera SVC. JS ships the same values, so nothing may need to change here, but it is worth stating as a known tradeoff in the PR description, or reconsidering the cap for SVC.

}

var fmtpFound = false
for ((attribute, fmtp) in fmtps) {
if (fmtp.payload == codecPayload) {
fmtpFound = true
var newFmtpConfig = fmtp.config
if (!fmtp.config.contains("x-google-start-bitrate")) {
newFmtpConfig = "$newFmtpConfig;x-google-start-bitrate=${(trackBr.maxBitrate * startBitrateForSVC).roundToLong()}"
newFmtpConfig = "$newFmtpConfig;x-google-start-bitrate=$startBitrate"
}
if (!fmtp.config.contains("x-google-max-bitrate")) {
newFmtpConfig = "$newFmtpConfig;x-google-max-bitrate=${trackBr.maxBitrate}"
Expand All @@ -492,7 +518,7 @@ fun ensureCodecBitrates(
media.addAttribute(
SdpFmtp(
payload = codecPayload,
config = "x-google-start-bitrate=${trackBr.maxBitrate * startBitrateForSVC};" +
config = "x-google-start-bitrate=$startBitrate;" +
"x-google-max-bitrate=${trackBr.maxBitrate}",
).toAttributeField(),
)
Expand All @@ -512,6 +538,7 @@ internal fun isSVCCodec(codec: String?): Boolean {
data class TrackBitrateInfo(
val codec: String,
val maxBitrate: Long,
val isScreenShare: Boolean = false,

@adrian-niculescu adrian-niculescu Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TrackBitrateInfo.maxBitrate has always carried kbps, but nothing in the name or type says so, and neighboring fields like RtpParameters' maxBitrateBps set a bps expectation. The old test demonstrated the ambiguity: it passed 1000000 into the kbps field and asserted a roughly 1 Gbps fmtp value. Renaming to maxBitrateKbps closes the trap. While here: isScreenShare could use a KDoc, and the camera/screenshare/other defaults mapping is now written out in five places (both data class comments, the base KDoc, the publish-site comment, and getDefaultDegradationPreference); one authoritative copy in the public KDoc would keep them from drifting.

One consideration for both the new isScreenShare parameter and the rename: TrackBitrateInfo is a public JVM type in the shipped 2.27.0 AAR (@suppress only hides it from docs), and changing the primary constructor drops the compiled (String, long) constructor and copy descriptors. Exposure is low since PeerConnectionTransport is internal and the only public entry point taking the type is the @VisibleForTesting ensureCodecBitrates, but if its ABI is meant to be stable that should be a deliberate call rather than an accidental side effect.

)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -697,14 +697,22 @@ internal constructor(
track.statsGetter = engine.createStatsGetter(transceiver.sender)

val finalOptions = options
// Handle trackBitrates
if (encodings.isNotEmpty()) {
if (finalOptions is VideoTrackPublishOptions && isSVCCodec(finalOptions.videoCodec) && encodings.firstOrNull()?.maxBitrateBps != null) {
// Handle trackBitrates - apply start bitrate for all video codecs to prevent initial blurriness.
// - SVC codecs: use first encoding's bitrate (single stream with built-in layers)
// - Simulcast: sum all encoding bitrates (independent streams, BWE needs total)
if (encodings.isNotEmpty() && finalOptions is VideoTrackPublishOptions) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A lifecycle gap this expansion widens: trackBitrates in PeerConnectionTransport is insert-only, and neither unpublishTrack nor RTCEngine.removeTrack removes the entry, so registrations outlive their publications for the transport's lifetime. Mostly that is just dead map entries scanned on every offer, but it becomes wrong munging on republish: cid is the RTC track id, so republishing the same track hits the same key, and if the new publish computes no encodings (videoEncoding == null with simulcast = false returns an empty list from computeVideoEncodings) this block is skipped and ensureCodecBitrates injects the previous publish's start/max values into the new offer. This existed for SVC registrations before, but the all-video path makes it reachable for every codec. An unregister call when the track is removed would close it.

val targetBitrateBps: Long = if (isSVCCodec(finalOptions.videoCodec)) {
(encodings.firstOrNull()?.maxBitrateBps ?: 0).toLong()
} else {
encodings.sumOf { (it.maxBitrateBps ?: 0).toLong() }
}
if (targetBitrateBps > 0) {
engine.registerTrackBitrateInfo(
cid = cid,
TrackBitrateInfo(
codec = finalOptions.videoCodec,
maxBitrate = (encodings.first().maxBitrateBps?.div(1000) ?: 0).toLong(),
maxBitrate = targetBitrateBps / 1000,
isScreenShare = trackSource == Track.Source.SCREEN_SHARE,
),
)
}
Expand All @@ -716,7 +724,12 @@ internal constructor(
(track as LocalVideoTrack).codec = finalOptions.videoCodec

val rtpParameters = transceiver.sender.parameters
// Use provided degradation preference, or default based on track source:
// - Camera: MAINTAIN_FRAMERATE (smoother video for real-time communication)
// - Screen share: MAINTAIN_RESOLUTION (clarity is critical for text/UI)
// - Other/unknown: BALANCED
rtpParameters.degradationPreference = finalOptions.degradationPreference
?: getDefaultDegradationPreference(trackSource)

@adrian-niculescu adrian-niculescu Jul 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Revised, the first version of this comment overstated the default case. The backup transceiver reuses the same RTC track and source, and with no explicit preference libwebrtc's GetDegradationPreference resolves is_screencast sources to MAINTAIN_RESOLUTION and camera content to MAINTAIN_FRAMERATE, so for camera and screen share the backup sender lands on the same preference implicitly and the new defaults stay consistent. What does not carry over to the backup sender: an explicitly supplied degradationPreference (a gap that predates this PR), and for sources outside CAMERA/SCREEN_SHARE the new BALANCED fallback applies only to the primary, while the backup retains libwebrtc's source-derived default (MAINTAIN_FRAMERATE for camera content, MAINTAIN_RESOLUTION for screencast content). If publishAdditionalCodecForTrack gets touched anyway, setting the resolved preference on the backup sender would close both.

transceiver.sender.parameters = rtpParameters
}

Expand Down Expand Up @@ -1456,10 +1469,17 @@ abstract class BaseVideoTrackPublishOptions {
abstract val backupCodec: BackupVideoCodec?

/**
* When bandwidth is constrained, this preference indicates which is preferred
* between degrading resolution vs. framerate.
* Controls how the encoder trades off between resolution and framerate
* when bandwidth is constrained.
*
* - MAINTAIN_FRAMERATE: Prioritizes framerate, reduces resolution if needed
* - MAINTAIN_RESOLUTION: Prioritizes resolution, drops frames if needed
* - BALANCED: Balances between both
*
* null value indicates default value (maintain framerate).
* If not set (null), the SDK uses defaults based on track source:
* - Camera: MAINTAIN_FRAMERATE (smoother video for real-time communication)
* - Screen share: MAINTAIN_RESOLUTION (clarity is critical for text/UI)
* - Other/unknown: BALANCED
*/
abstract val degradationPreference: RtpParameters.DegradationPreference?

Expand All @@ -1481,6 +1501,8 @@ data class VideoTrackPublishDefaults(
override val videoCodec: String = VideoCodec.VP8.codecName,
override val scalabilityMode: String? = null,
override val backupCodec: BackupVideoCodec? = null,
// Default is null - SDK applies source-based defaults at runtime:
// Camera: MAINTAIN_FRAMERATE, Screen share: MAINTAIN_RESOLUTION, Other: BALANCED
override val degradationPreference: RtpParameters.DegradationPreference? = null,
override val simulcastLayers: List<VideoPreset>? = null,
) : BaseVideoTrackPublishOptions()
Expand All @@ -1494,6 +1516,8 @@ data class VideoTrackPublishOptions(
override val backupCodec: BackupVideoCodec? = null,
override val source: Track.Source? = null,
override val stream: String? = null,
// Default is null - SDK applies source-based defaults at runtime:
// Camera: MAINTAIN_FRAMERATE, Screen share: MAINTAIN_RESOLUTION, Other: BALANCED
override val degradationPreference: RtpParameters.DegradationPreference? = null,
override val simulcastLayers: List<VideoPreset>? = null,
) : BaseVideoTrackPublishOptions(), TrackPublishOptions {
Expand Down Expand Up @@ -1661,6 +1685,21 @@ internal fun VideoTrackPublishOptions.hasBackupCodec(): Boolean {
private val backupCodecs = listOf(VideoCodec.VP8.codecName, VideoCodec.H264.codecName)
private fun isBackupCodec(codecName: String) = backupCodecs.contains(codecName)

/**
* Returns the appropriate degradation preference for a video track based on its source.
*
* - Camera: MAINTAIN_FRAMERATE (smoother video for real-time communication)
* - Screen share: MAINTAIN_RESOLUTION (clarity is critical for reading text/UI)
* - Other/unknown: BALANCED
*/
private fun getDefaultDegradationPreference(source: Track.Source): RtpParameters.DegradationPreference {
return when (source) {
Track.Source.CAMERA -> RtpParameters.DegradationPreference.MAINTAIN_FRAMERATE
Track.Source.SCREEN_SHARE -> RtpParameters.DegradationPreference.MAINTAIN_RESOLUTION
else -> RtpParameters.DegradationPreference.BALANCED
}
}

/**
* A handler that processes an RPC request and returns a string
* that will be sent back to the requester. The payload must
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,15 @@ class SdpMungingTest {
val sdp = SdpFactory.getInstance().createSessionDescription(JainSdpUtilsTest.DESCRIPTION)
val mediaDescription = sdp.getMediaDescriptions(true).filterIsInstance<MediaDescription>()[1]

// Use realistic bitrate: 1000 kbps (1 Mbps)
// With 0.9 multiplier: startBitrate = 900 kbps (below 1 Mbps cap)
ensureCodecBitrates(
mediaDescription,
mapOf(
TrackBitrateInfoKey.Cid("PA_Qwqk4y9fcD3G") to
TrackBitrateInfo(
"VP9",
1000000L,
1000L,
),
),
)
Expand All @@ -71,7 +73,7 @@ class SdpMungingTest {
.filter { (_, fmtp) -> fmtp.payload == 98L }
.first()

assertEquals("profile-id=0;x-google-start-bitrate=700000;x-google-max-bitrate=1000000", vp9fmtp.config)
assertEquals("profile-id=0;x-google-start-bitrate=900;x-google-max-bitrate=1000", vp9fmtp.config)
}

companion object {
Expand Down
2 changes: 1 addition & 1 deletion protocol

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This appears to be unrelated to the bitrate/SDP change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, yes, it should removed, it got checked in by mistake

Submodule protocol updated 236 files
Loading