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

Fix issues #1224 & #1213: Don't call this.predict() unless there is a video. #1321

Merged
merged 1 commit into from
Apr 2, 2022

Conversation

lindapaiste
Copy link
Contributor

Issue

Reported in #1224 and #1213.

The Facemesh and Handpose models are designed such that they user can instantiated it a with a video and use a facemesh.on("face") callback to process the result of every frame. This chain of continuous prediction begins with a call to this.predict() inside this.loadModel().

This creates an error when no video is provided, as this.predict() is called without any input image. In Facemesh, this logs an error Uncaught (in promise) TypeError: Cannot read property 'height' of undefined. In Handpose the error is avoided by returning [] from this.predict().

Changes

  • Added a conditional statement such that this.predict() is only called when there is a video.
  • Throw an error in both classes if predict() is called with no image or video.
  • JSDoc
    • Fix the incorrect return types on predict().
    • Make all three constructor arguments optional.
    • Add a type for this.model.
  • Removed eslint disabling for rules which aren't actually broken.

Copy link
Contributor

@joeyklee joeyklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much @lindapaiste ! Another really helpful error handling + friendly error message. (sorry it has taken me so long to get to this!)

@joeyklee joeyklee added the SEMVER/patch a version tag for a patch release change label Apr 2, 2022
@joeyklee joeyklee merged commit 2371388 into ml5js:main Apr 2, 2022
@lindapaiste lindapaiste deleted the fix/facemesh-missing-input branch May 9, 2022 01:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
SEMVER/patch a version tag for a patch release change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants