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

UnbindVideoElement #627

Closed
76kennyboss opened this issue Aug 15, 2020 · 12 comments
Closed

UnbindVideoElement #627

76kennyboss opened this issue Aug 15, 2020 · 12 comments
Labels
Bug Something isn't working Triaged

Comments

@76kennyboss
Copy link

Hello,

Am trying to get the following effect.

  1. Video streams are already "bind" successfully to small thumb nails
  2. When user press on the thumb nails, video stream is first "unbind" from the smaller thumb nail and then "bind" to a larger sized element.

Am unable to get both unbindVideoElement and bindVideoElement working in stage (2).

@RidipDe
Copy link
Contributor

RidipDe commented Aug 18, 2020

Hello,
can you please let us know what you are trying to achieve and what is actually happening?
Can you please provide us with SDK based API minimal repro that you are performing at your end.

@RidipDe RidipDe added Triaged Needs More Information Waiting for more information from builders labels Aug 18, 2020
@vidya-mohan
Copy link
Contributor

@76kennyboss please help the team by keeping the original bug template and provide the details with sufficient information to make progress

@stale stale bot removed the Needs More Information Waiting for more information from builders label Aug 19, 2020
@vidya-mohan vidya-mohan added the Needs More Information Waiting for more information from builders label Aug 19, 2020
@simmkyu
Copy link
Contributor

simmkyu commented Aug 25, 2020

audioVideo.unbindVideoElement() API seems to be broken.

Repro steps in the Chime SDK demo

  1. Join a meeting and turn on your camera. Open the browser console and take a note of the video tile ID, e.g. 1.
  2. In the browser console, run the following command with the tile ID.
    app.audioVideo.unbindVideoElement(__tileId__)
    
  3. The video element keeps displaying the video.

As a workaround, you might resize the same <video> element, e.g. using CSS or HTML attributes, when the user clicks on it.

@stale stale bot removed the Needs More Information Waiting for more information from builders label Aug 25, 2020
@simmkyu simmkyu added the Bug Something isn't working label Aug 25, 2020
@michhyun1
Copy link
Contributor

A fix has been pushed for this, the video stream should be removed from the video element when you unbind the video tile from the video element now.

@michhyun1
Copy link
Contributor

This commit was recently reverted, we need to push another fix for this issue

@ileccese
Copy link

Hi, have you pushed another fix for this issue? I am also facing problems from this bug

@michhyun1
Copy link
Contributor

@ileccese could you please elaborate? The unbindVideoElement should alter the tileState of the videoTile, which you can then reference in videoTileDidUpdate.

@ileccese
Copy link

@michhyun1 I'm seeing the same behavior that Kyu described above - the video tile still displays the video stream after unbindVideoElement has been called

@michhyun1
Copy link
Contributor

michhyun1 commented Nov 30, 2020

@ileccese If you monitor the VideoTile's tileState, you'll notice the boundVideoElement of the tileState is null after calling unbindVideoElement, which is what unbindVideoElement is supposed to do. It won't clean up the srcObject on the videoElement that it unbinds from before the tileState de-references the videoElement in its boundVideoElement attribute.

In your VideoTileDidUpdate implementation, I'd suggest checking to see if the updated tileState's boundVideoElement is null, and handling that case by either hiding the actual dom videoElement or removing the srcObject from the videoElement if you want to get rid of the stream that is playing on the videoElement that you just unbinded the videoTile from.

@ileccese
Copy link

Interesting, so you're saying all unbindVideoElement is supposed to do is trigger a videoTileDidUpdate with boundVideoElement = null and that if I want to remove the video stream from that video element I need to do that myself?

I have video tiles setup very similar to the components library's RemoteTile, where each video tile takes a tileId as props and has a useEffect to unbind/bind to that tileId when it's updated:

useEffect(() => {
    if (meetingSession && props.tileId) {
      if (meetingSession.audioVideo.getVideoTile(props.tileId)) {
        logger.info(`App: binding tileId ${props.tileId} to ${props.tileName}`);
        meetingSession.audioVideo.bindVideoElement(props.tileId, ref.current);
      }
    }
    return () => {
      if (meetingSession && props.tileId) {
        const tile = meetingSession.audioVideo.getVideoTile(props.tileId);
        if (tile) {
          logger.info(`App: unbinding tileId ${props.tileId} from ${props.tileName}`);
          meetingSession.audioVideo.unbindVideoElement(props.tileId);
        }
      }
    };
  }, [props.tileId, props.tileName, meetingSession, logger, ref]);

The problem I was running into is that we are implementing a "pin user to top tile" feature and expected that we could simply swap the order of the tileIds in state and when each video tile received a new tileId, it would unbind the old tileId and bind the new tileId. this worked fine when the tiles that were being swapped both had active video streams, but if one stream was inactive (the participant had their camera off), the video element which previously had the active stream would still show it. Based on your explanation, this is because unbindVideoElement does not remove the old srcObject and the new inactive tileId didn't have a new srcObject to replace the old srcObject
Replacing unbindVideoElement with removing the srcObject seems work as expected:

useEffect(() => {
    if (meetingSession && props.tileId) {
      if (meetingSession.audioVideo.getVideoTile(props.tileId)) {
        logger.info(`App: binding tileId ${props.tileId} to ${props.tileName}`);
        meetingSession.audioVideo.bindVideoElement(props.tileId, ref.current);
      }
    }
    return () => {
      if (meetingSession && props.tileId) {
        const tile = meetingSession.audioVideo.getVideoTile(props.tileId);
        if (tile) {
          logger.info(`App: unbinding tileId ${props.tileId} from ${props.tileName}`);
          // meetingSession.audioVideo.unbindVideoElement(props.tileId);
          ref.current.srcObject = null;
        }
      }
    };
  }, [props.tileId, props.tileName, meetingSession, logger, ref]);

@michhyun1
Copy link
Contributor

You may want to still call unbindVideoElement in that case, because you probably want the VideoTile's tileState boundVideoElement to be null. Without calling unbindVideoElement, the boundVideoElement attribute will still point to that videoElement.

@ileccese
Copy link

cool, thanks for your help.

I think it would be helpful to have unbindVideoElement remove the srcObject, or at least to make it clear in the documentation that unbindVideoElement won't remove the video stream from the video element, because I think most people would assume it just does the opposite of bindVideoElement

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Triaged
Projects
None yet
Development

No branches or pull requests

6 participants