From 44122a2e1b402a781073bd075f523e6ead2b6f1f Mon Sep 17 00:00:00 2001 From: Henry Smith Date: Mon, 29 Nov 2021 11:58:41 -0800 Subject: [PATCH 1/2] Clarify comment on previous fix --- .../classes/defaultsimulcastuplinkpolicy.html | 24 +++++++++---------- .../DefaultSimulcastUplinkPolicy.ts | 9 ++++++- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/docs/classes/defaultsimulcastuplinkpolicy.html b/docs/classes/defaultsimulcastuplinkpolicy.html index 44f1ba6403..a0ec116b1a 100644 --- a/docs/classes/defaultsimulcastuplinkpolicy.html +++ b/docs/classes/defaultsimulcastuplinkpolicy.html @@ -235,7 +235,7 @@

addObserver

Parameters

@@ -259,7 +259,7 @@

chooseCaptureAndEncodeParameters

Returns DefaultVideoAndEncodeParameter

@@ -277,7 +277,7 @@

chooseEncodingParameters

Returns Map<string, RTCRtpEncodingParameters>

@@ -295,7 +295,7 @@

chooseMediaTrackConstraints

Returns MediaTrackConstraints

@@ -312,7 +312,7 @@

forEachObserver

  • Parameters

    @@ -353,7 +353,7 @@

    getEncodingSimulcastLayer

  • Parameters

    @@ -377,7 +377,7 @@

    maxBandwidthKbps

    Returns number

    @@ -395,7 +395,7 @@

    removeObserver

    Parameters

    @@ -419,7 +419,7 @@

    setHasBandwidthPriority

    Parameters

    @@ -443,7 +443,7 @@

    setIdealMaxBandwidthKbps

    Parameters

    @@ -495,7 +495,7 @@

    updateIndex

    Parameters

    @@ -519,7 +519,7 @@

    wantsResubscribe

    Returns boolean

    diff --git a/src/videouplinkbandwidthpolicy/DefaultSimulcastUplinkPolicy.ts b/src/videouplinkbandwidthpolicy/DefaultSimulcastUplinkPolicy.ts index 8f52e0f4cd..9f38656d99 100644 --- a/src/videouplinkbandwidthpolicy/DefaultSimulcastUplinkPolicy.ts +++ b/src/videouplinkbandwidthpolicy/DefaultSimulcastUplinkPolicy.ts @@ -136,7 +136,7 @@ export default class DefaultSimulcastUplinkPolicy implements SimulcastUplinkPoli // The value of `newActiveStreams` is somewhat irrelevant since this single // stream will adapt based on both sender and receiver network conditions. // - // We don't use top layer (middle vs. low doesn't matter) here to work around a bug in Chromium where + // We use middle layer here to work around a bug in Chromium where // it seems when a transceiver is created when BWE is low (e.g. on a reconnection), // it will never reset the encoder even when `setParameters` is called. WebRTC bug // #12788 seems to call a similar issue out as fixed for VP8, it's not clear if this @@ -144,6 +144,13 @@ export default class DefaultSimulcastUplinkPolicy implements SimulcastUplinkPoli // request from the backend since it will only be sending padding (which also // don't have MID due to #10822). Since we don't scale when simulcast is disabled // this doesn't have any end-user effect. + // + // Note that this still relies on a little bit (5-6 packets) of padding on reconnect + // and that technically the browser will still eventually try to send all 3 streams. + // + // Also note that due to some uninvestigated logic in bitrate allocation, Chromium + // will skip the bottom layer if we try setting it to 1200 kbps instead so it will still take a while to + // recover (as it needs to send padding until it reaches around 1000 kbps). this.newActiveStreams = ActiveStreams.kHi; newBitrates[0].maxBitrateKbps = 0; newBitrates[1].maxBitrateKbps = 1200; From 635cdf9fe9029e4f74c3318377dea9feb0e1321a Mon Sep 17 00:00:00 2001 From: Henry Smith Date: Mon, 29 Nov 2021 12:09:38 -0800 Subject: [PATCH 2/2] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 610a1dc33c..fccd871e3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed ### Changed +- Clarified comment in `DefaultSimulcastUplinkPolicy`. ## [2.23.0] - 2021-11-22 ### Added