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

Update NScale uplink initial encoder params #1862

Merged
merged 4 commits into from
Dec 9, 2021
Merged

Update NScale uplink initial encoder params #1862

merged 4 commits into from
Dec 9, 2021

Conversation

predeekc-amazon
Copy link
Contributor

Issue #: No Issue

Description of changes:
While testing I found the default uplink bitrate was exceeding the default max bitrate for the stream when using the NScaleVideoUplinkBandwidthPolicy. While monitoring the calls to RTCRtpSender.setParameters I noticed they weren't being set initially.

A mismatch in the constructor seems to be the root cause. the optimalParameters value has a max bitrate of 0 but the encodingParamMap has a maxBitrate of 1400 (the default of idealMaxBandwidthKbps).

constructor(
    private selfAttendeeId: string,
    private scaleResolution: boolean = true,
    private logger: Logger | undefined = undefined
  ) {
    this.optimalParameters = new DefaultVideoAndEncodeParameter(0, 0, 0, 0, false);
    this.parametersInEffect = new DefaultVideoAndEncodeParameter(0, 0, 0, 0, false);
    this.encodingParamMap.set(NScaleVideoUplinkBandwidthPolicy.encodingMapKey, {
      scaleResolutionDownBy: 1,
      maxBitrate: this.idealMaxBandwidthKbps * 1000,
    });
  }

When the initial update for the stream was received, no call was made to WebRTC since the uplink policy believed it was no changes have been made.

NScaleVideoUplinkBandwidthPolicy.ts:172

  this.encodingParamMap.set(NScaleVideoUplinkBandwidthPolicy.encodingMapKey, {
    scaleResolutionDownBy: 1,
    maxBitrate: this.idealMaxBandwidthKbps * 1000,
  });

This update changes the constructor to bring the bitrate values in optimalParameters and encodingParamMap into alignment.

Testing:

I verified the change brought the externally measured uplink bitrate to the expected 1400kbps. I then added a unit test to ensure setEncodingParameters is called on the first update when the default ideal bitrate is used.

Can these tested using a demo application? Please provide reproducible step-by-step instructions.

  1. Create a client that never sets a new ideal bitrate using `NScaleVideoUplinkBandwidth.setIdealMaxBandwidthKbps'
  2. Send only video from the client and monitor the output bitrate of the client (I used WireShark)
  3. The bitrate was over the default ideal max bitrate of 1400kbps

Checklist:

  1. Have you successfully run npm run build:release locally?
    Yes

  2. Do you add, modify, or delete public API definitions? If yes, has that been reviewed and approved?
    No

  3. Do you change the wire protocol, e.g. the request method? If yes, has that been reviewed and approved?
    No

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@predeekc-amazon predeekc-amazon requested a review from a team as a code owner December 8, 2021 19:42
@ltrung ltrung merged commit 92d2508 into master Dec 9, 2021
@ltrung ltrung deleted the fix-params branch December 9, 2021 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants