VIDEO-6368 | wait to load image before removing processor

[gabriel-jt]

Contributing to Twilio

All third party contributors acknowledge that any contributions they provide will be made under the same open source license that the open source project is provided under.

• I acknowledge that all my contributions will be made under the project's license.

Pull Request Details

JIRA link(s):

• VIDEO-6368

Description

Wait to load images for virtual background before removing the processor

Burndown

Before review

• Updated CHANGELOG.md if necessary
• Added unit tests if necessary
• Updated affected documentation
• Verified locally with npm test
• Manually sanity tested running locally
• Included screenshot as PR comment (if needed)
• Ready for review

Before merge

• Got one or more +1s
• Re-tested if necessary

[charliesantos]

Your changes works! However, it's not immediately obvious what the code is trying to do. What do you think of my suggestion below? Basically, I created addProcessor and removeProcessor helper functions and just use those. It reduces the nested if statements, and also prevents removing and adding processors if the processor hasn't changed.

Helper functions outside of your useEffect hook

const removeProcessor = () => {
  if (videoTrack && videoTrack.processor) {
    videoTrack.removeProcessor(videoTrack.processor);
  }
};

const addProcessor = (processor: GaussianBlurBackgroundProcessor | VirtualBackgroundProcessor) => {
  if (!videoTrack || videoTrack.processor === processor) {
    return;
  }
  removeProcessor();
  videoTrack.addProcessor(processor);
};

Handler now looks like this

const handleProcessorChange = async () => {
  if (!blurProcessor) {
    blurProcessor = new GaussianBlurBackgroundProcessor({
      assetsPath: virtualBackgroundAssets,
    });
    await blurProcessor.loadModel();
  }
  if (!virtualBackgroundProcessor) {
    virtualBackgroundProcessor = new VirtualBackgroundProcessor({
      assetsPath: virtualBackgroundAssets,
      backgroundImage: await getImage(0),
      fitType: ImageFit.Cover,
    });
    await virtualBackgroundProcessor.loadModel();
  }

  if (!room?.localParticipant) {
    return;
  }

  if (backgroundSettings.type === 'blur') {
    addProcessor(blurProcessor);
  } else if (backgroundSettings.type === 'image' && typeof backgroundSettings.index === 'number') {
    virtualBackgroundProcessor.backgroundImage = await getImage(backgroundSettings.index);
    addProcessor(virtualBackgroundProcessor);
  } else {
    removeProcessor();
  }
};

[gabriel-jt]

Your changes works! However, it's not immediately obvious what the code is trying to do. What do you think of my suggestion below? Basically, I created addProcessor and removeProcessor helper functions and just use those. It reduces the nested if statements, and also prevents removing and adding processors if the processor hasn't changed.

Helper functions outside of your useEffect hook

const removeProcessor = () => {
  if (videoTrack && videoTrack.processor) {
    videoTrack.removeProcessor(videoTrack.processor);
  }
};

const addProcessor = (processor: GaussianBlurBackgroundProcessor | VirtualBackgroundProcessor) => {
  if (!videoTrack || videoTrack.processor === processor) {
    return;
  }
  removeProcessor();
  videoTrack.addProcessor(processor);
};

Handler now looks like this

const handleProcessorChange = async () => {
  if (!blurProcessor) {
    blurProcessor = new GaussianBlurBackgroundProcessor({
      assetsPath: virtualBackgroundAssets,
    });
    await blurProcessor.loadModel();
  }
  if (!virtualBackgroundProcessor) {
    virtualBackgroundProcessor = new VirtualBackgroundProcessor({
      assetsPath: virtualBackgroundAssets,
      backgroundImage: await getImage(0),
      fitType: ImageFit.Cover,
    });
    await virtualBackgroundProcessor.loadModel();
  }

  if (!room?.localParticipant) {
    return;
  }

  if (backgroundSettings.type === 'blur') {
    addProcessor(blurProcessor);
  } else if (backgroundSettings.type === 'image' && typeof backgroundSettings.index === 'number') {
    virtualBackgroundProcessor.backgroundImage = await getImage(backgroundSettings.index);
    addProcessor(virtualBackgroundProcessor);
  } else {
    removeProcessor();
  }
};

Thanks for the suggestions @charliesantos ! I wrapped the helper functions inside a useCallback hook to prevent warnings about missing dependencies in the useEffect hook

[timmydoza]

Nice work! I used Chrome's dev tools to throttle my network connection so the background images loaded slowly, and the changes here worked well. 👍