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

Make tileWillBePausedByDownlinkPolicy synchronous and remove setTimeout #1806

Merged
merged 1 commit into from Nov 18, 2021

Conversation

devalevenkatesh
Copy link
Contributor

Issue #:
#1790

Description of changes:

  • Make tileWillBePausedByDownlinkPolicy observer update synchronous without setTimeout

Testing:

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

  • Yes using the meetingV2 demo.
  1. Open the meeting in Chrome
  2. Joined the same meeting with 6 attendees.
  3. Added logs under pause and unpause APIs on the DefaultVideoTileController and observed the logs.
  4. Started video for all attendees and on one of the local attendee observed the logs.
  5. Checked the "will be paused/unpaused observers" are triggered synchronously and then the actual "pause/un-pause VideoTile" API is triggered.

Output from my test:

[DEMO] Tile 2 will be paused due to insufficient bandwidth
VM18:2 2021-11-18T16:55:40.078Z [INFO] SDK - bwe: unpausing attendee c2b19aaa-865f-5e10-a8af-a84ffeb883ec due to bandwidth
VM18:2 [DEMO] Tile 2 will be resumed due to sufficient bandwidth
VM18:2 due to 2 tile.unpause() called in DefaultVideoTileController
VM18:2 [DEMO] Tile 3 will be paused due to insufficient bandwidth
VM18:2 2021-11-18T16:55:43.858Z [INFO] SDK - bwe: unpausing attendee ad423fd4-c77f-6a2d-3e40-aeb1020a158b due to bandwidth
VM18:2 [DEMO] Tile 3 will be resumed due to sufficient bandwidth
VM18:2 due to 3 tile.unpause() called in DefaultVideoTileController
VM18:2 [DEMO] Tile 4 will be paused due to insufficient bandwidth
VM18:2 [DEMO] Tile 6 will be paused due to insufficient bandwidth
VM18:2 2021-11-18T16:55:48.232Z [INFO] SDK - bwe: unpausing attendee ef3560bb-1ee9-6254-55f6-1bed3ba985e8 due to bandwidth
VM18:2 [DEMO] Tile 6 will be resumed due to sufficient bandwidth
VM18:2 due to 6 tile.unpause() called in DefaultVideoTileController
VM18:2 2021-11-18T16:59:36.151Z [INFO] SDK - bwe: pausing streamId 13 attendee d1bfedd7-5a48-e56c-02b9-761cc752ef94 due to bandwidth
VM18:2 [DEMO] Tile 5 will be paused due to insufficient bandwidth
VM18:2 due to 5 tile.pause() called in DefaultVideoTileController
VM18:2 2021-11-18T16:59:36.152Z [INFO] SDK - bwe: pausing streamId 18 attendee ef3560bb-1ee9-6254-55f6-1bed3ba985e8 due to bandwidth
VM18:2 [DEMO] Tile 6 will be paused due to insufficient bandwidth
VM18:2 due to 6 tile.pause() called in DefaultVideoTileController
VM18:2 2021-11-18T16:59:43.163Z [INFO] SDK - bwe: unpausing attendee d1bfedd7-5a48-e56c-02b9-761cc752ef94 due to bandwidth
VM18:2 [DEMO] Tile 5 will be resumed due to sufficient bandwidth
VM18:2 due to 5 tile.unpause() called in DefaultVideoTileController
VM18:2 2021-11-18T16:59:43.165Z [INFO] SDK - bwe: unpausing attendee ef3560bb-1ee9-6254-55f6-1bed3ba985e8 due to bandwidth
VM18:2 [DEMO] Tile 6 will be resumed due to sufficient bandwidth
VM18:2 due to 6 tile.unpause() called in DefaultVideoTileController

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?
    NA

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

@devalevenkatesh devalevenkatesh merged commit c6fd2e3 into master Nov 18, 2021
@devalevenkatesh devalevenkatesh deleted the make-observer-sync branch November 18, 2021 17:32
dimitarz pushed a commit to dimitarz/amazon-chime-sdk-js that referenced this pull request Nov 20, 2021
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.

None yet

2 participants