aws / amazon-chime-sdk-js

A JavaScript client library for integrating multi-party communications powered by the Amazon Chime service.
Apache License 2.0
699 stars 472 forks source link

Fix unnecessary simulcast cropping, vastly simplify simulcast uplink policy #2845

Closed hensmi-amazon closed 4 months ago

hensmi-amazon commented 4 months ago

Issue #: https://github.com/aws/amazon-chime-sdk-js/issues/2844, but after I fixed that by removing code, it somehow reduced coverage in all these corners related to our 'pausedByWebRTC' logic, which at this point is no longer needed. So I just nuked it all.

Description of changes: So back in the dark ages, we had this 'issue' where when simulcast was enabled, Chromium didn't really have a mechanism for indicating that layers would be paused if there wasn't enough bandwidth for them. So what we did was stare at simulcast.cc in libwebrtc and try to guess in JS when the layers would be dropped, and then use this information to resubscribe without the dropped stream being published. This was clunky but it did the job.

However when https://github.com/aws/amazon-chime-sdk-js/commit/2f8c523fd708837be32ef86dbd1ba407099d4b2e was released (the second time :) ) it gave us a mechanism to indicate to backend when a stream was paused. So all the aforemention logic is now completely unnecessary. Note that I'm pretty sure we don't need to resubscribe on layers switches anymore but I will do that another day.

Testing:

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

Checklist:

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

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

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

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

BrandonMathis commented 4 months ago

I was dealing with camera resolution issues with simulcast enabled in chromium based browsers (camera was coming back from chime looking very zoomed) and this PR fixed my problem. Thankyou 🙏🏼

Any possible updates on a release or beta release timeline for this fix to npmjs? Installing and building modules directly from GitHub is a bit challenging in my cloud environment.

Edit: Apologies, i see release target is set for 3/21/24 - I will simply disable simulcast until this fix is published

hensmi-amazon commented 4 months ago

@BrandonMathis you can use the mitigation in https://github.com/aws/amazon-chime-sdk-js/issues/2844#issuecomment-1954981303 until then if you want