aws / aws/amazon-chime-sdk-android

Video binding failing in some cases due to a bug in `DefaultVideoTileController`

Open
#499 11 comments 0 reactions 0 assignees View on GitHub
feature-request Triaged
Dominant language
Kotlin
Stars
113
Forks
60
Avg merge
40m
Merged PRs (30d)
2

Description

Hi,

We think there is a mistake in [`DefaultVideoTileController`](https://github.com/aws/amazon-chime-sdk-android/blob/v0.17.4/amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt) which causes video bindings to fail under certain circumstances. This occurs when two participants in a video meeting swap their videos.

**Description**

Initial situation:

1. `VideoRenderView` **A** shows `tileId` **1**
2. `VideoRenderView` **B** shows `tileId` **2**

Final situation (expected):

1. `VideoRenderView` **A** shows `tileId` **2**
2. `VideoRenderView` **B** shows `tileId` **1**

Final situation (actual):

1. `VideoRenderView` **A** shows frozen `tileId` **1**
2. `VideoRenderView` **B** shows `tileId` **1**

**To Reproduce**

Here are the steps to reproduce:

1. Have two participants in a meeting
2. Disable the video of the first participant then re-enable it
3. Now proceed to swapping the tiles: bind the `VideoRenderView` of the second participant to the first `tileId`
4. Bind the `VideoRenderView` of the first participant to the second `tileId`

The video of the second participant shows the old video and is frozen (binding didn't occur).

We did not try to reproduce it on the demo app because it means a bit of work for triggering the swap of videos. However I can upload a video recording of the bug occurring in our application if needed.

**Explanation and probable solution**

The problem is caused by step 2. When the video is disabled, [`DefaultVideoTileController::onRemoveVideoTile()`](https://github.com/aws/amazon-chime-sdk-android/blob/v0.17.4/amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt#L196) is invoked but does not remove the `VideoRenderView` (let's call it **X**) from `renderViewToBoundVideoTileMap`. Most likely a call to `removeRenderViewFromBoundVideoTileMap(tileId)` is missing line 201.

Then when later re-enabling the video, a new `VideoRenderView` is created (let's call it **Y**). Then, when swapping the videos, the following occurs:
1. [`DefaultVideoTileController::bindVideoView()`](https://github.com/aws/amazon-chime-sdk-android/blob/v0.17.4/amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt#L150) is invoked
2. The block checking whether a `VideoRenderView` is associated with a video tile is entered ("_Override the binding from (…)_")
3. A call is made to `removeRenderViewFromBoundVideoTileMap()` to remove the `VideoRenderView` which is going to be replaced
4. [`renderViewToBoundVideoTileMap.entries.firstOrNull`](https://github.com/aws/amazon-chime-sdk-android/blob/v0.17.4/amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt#L177) finds the `VideoRenderView` **X** (which no more exists and should have been removed in `onRemoveVideoTile()`) and removes it from the map instead of finding and removing **Y** (both have the save `tileId`)
5. First video replaced the other, now `DefaultVideoTileController::bindVideoView()` is invoked a second time to swap the second video
6. The same block as in **2** is entered **althought it should not** because **Y** should have been removed from the map there but was not
7. Again, call to `removeRenderViewFromBoundVideoTileMap()` which in turn calls [`videoTile.unbind()`](https://github.com/aws/amazon-chime-sdk-android/blob/v0.17.4/amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt#L180), **unbinding the first video we swapped previously**
8. Hence the video being frozen

We confirmed that by adding the missing call line 201 described above the bug goes away. This seems to be the right way of solving the issue, however we are not that familiar with the internals of Chime so maybe there is a better way.

**Test environment Info**

- Version amazon-chime-sdk: 0.17.4
- Version amazon-chime-sdk-media: 0.17.5

**Additional info**

This was not tested on iOS.

Contributor guide

Open the contributing guide

Research direction

Start in amazon-chime-sdk/src/main/java/com/amazonaws/services/chime/sdk/meetings/audiovideo/video/DefaultVideoTileController.kt, focusing on onRemoveVideoTile(), bindVideoView(), and removeRenderViewFromBoundVideoTileMap(). Trace the render-view map through disabling, re-enabling, and swapping two video tiles. Done means the swap no longer leaves a frozen or duplicated tile binding; the issue provides no test file to run.

Written by the indexing model from the issue text.

Assessment

Tech stack
android, kotlin
Domain
audio-video-rtc, mobile
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.