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

[Jim-Bar]

Hi,

We think there is a mistake in DefaultVideoTileController 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() 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() 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 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(), 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.

[dingyishen-amazon]

Thanks for reporting this. We will investigate and update in the support ticket.

[dingyishen-amazon]

Hi @Jim-Bar,
I think the logic is already been handled here.
If renderViewToBoundVideoTileMap[videoView] is not empty, we will first call removeRenderViewFromBoundVideoTileMap() to remove it then rebind.
Can you try to take a look at your app logic to see whether it is same as ours?
Thanks!

[Jim-Bar]

Hi @dingyishen-amazon,

Yes removeRenderViewFromBoundVideoTileMap() is indeed called, but in the failing cases we observe that this call removes the wrong render view.

The key fact is that sometimes renderViewToBoundVideoTileMap contains two render views bound to the same tile id. This because when onRemoveVideoTile(videoId) is called from here, the old render view is not removed.

This is why I suggested adding a call to removeRenderViewFromBoundVideoTileMap(tileId) line 201 (line 57 would also solve it). Note that replacing firstOrNull{…}.let by filter{…}.let in removeRenderViewFromBoundVideoTileMap() also solves the issue (it removes both render views).

Can you try to take a look at your app logic to see whether it is same as ours?

Do you mean the way we call the Chime API? It most likely differs in many ways.

[hokyungh]

Hi @Jim-Bar , when onVideoTileRemoved is invoked, do you call audiovideo.unbindVideoView? I am trying to figure out the flow little better.

[Jim-Bar]

Hi @hokyungh, no we don't call audiovideo.unbindVideoView. I figured out that the issue would probably not occur if we did call it, however:

• it's a bit complex to implement in our app due to the frameworks we use
• we felt like bringing it up to you was the best thing to do as the problem may arise in other circumstances

Thanks for taking the time to look into this.

[hokyungh]

Thank you. This looks like we need to redesign DefaultVideoTileController to make it easier to use. Let me add feature request label and discuss with the team.

[Jim-Bar]

Okay, thanks for the feedback.

[Jim-Bar]

Hi, just letting you know that we reproduce this issue on iOS too (0.22.2).

A possible fix seems similar to the one for Android (adding removeRenderViewFromBoundVideoTileMap(tileState.tileId) line 157 in DefaultVideoTileController::onRemoveTrack()). However, replacing the DefaultVideoTileController by a custom implementation is impossible due to dependencies in AmazonChimeSDK/internal, so we cannot confirm this actually works (although looking at the code it should).

Side note: is it normal that we cannot use a custom implementation of VideoTileController on iOS even though it is possible on Android?

[ziyiz-amzn]

Thank you @Jim-Bar for the detailed explanation. As stated in the use case 14 and api overview, it is suggested that bindVideoView() and unbindVideoView should be always called in pairs for a single video view. Can you try this and let us know if you still have issue.

[Jim-Bar]

Hi, as I said previously due to the framework with use in our app this is no simple task. I managed to do it though, I think it solves most of our cases but I'm not 100% sure yet.

Nonetheless I would recommend to add the lines I described previously in the SDK because I really think this can show up as soon as one fails to properly call unbind (even once), and the fix seems safe.

[Jim-Bar]

Hi, this is still a problem for us. Did you make progress on what you want to do with this issue?

If you are willing to accept PRs I could make one that fixes this for both Android and iOS.