ml5js / ml5-library

Friendly machine learning for the web! πŸ€–
https://ml5js.org
Other
6.45k stars 905 forks source link

Adding flipHorizontal option on Facemesh removes annotations and other key data #1198

Open lisajamhoury opened 3 years ago

lisajamhoury commented 3 years ago

Dear ml5 community,

I'm submitting a new issue. Please see the details below.

β†’ Step 1: Describe the issue πŸ“

Did you find a bug? Want to suggest an idea for feature?

If I add option to flipHorizontal on Facemesh annotations and other key data are no longer available.

β†’ Step 2: Screenshots or Relevant Documentation πŸ–Ό

Here's some helpful screenshots and/or documentation of the new feature

See this example in the p5 editor. https://editor.p5js.org/lisajamhoury/sketches/_f_-BmXL2

I copied the provided facemesh example. I've only added options at line 11, and added them to facemesh initialization at line 15.

const options = { flipHorizontal: true, };

facemesh = ml5.facemesh(video, options, modelReady);

flipHorizontal: false works as expected. flipHorizontal: true returns no annotations and draws nothing to canvas in this sketch.

β†’ Step 3: Share an example of the issue πŸ¦„

Here's some example code or a demonstration of my feature in this issue, separate GitHub repo, or in the https://editor.p5js.org or codepen/jsfiddle/Glitch/etc...

See above

Other relevant information, if applicable

β†’ Describe your setup πŸ¦„

Here's some helpful information about my setup...

Thank you! I'm so excited to have Facemesh in ml5, and I'm excited to teach with it this fall. <3

bomanimc commented 3 years ago

Thanks for highlighting this issue.

From what I can tell, it looks like the function that handles returning flipped data in Tensorflow.js's facial-landmark-detection library is here: https://github.com/tensorflow/tfjs-models/blob/1db6ba191bec70d8953bd3c6e274213dbbbbd778/face-landmarks-detection/src/mediapipe-facemesh/index.ts#L190.

Printing out the predictions we're getting in ml5.js shows what seems to be evidence of tensor disposal (a concept I don't know very much about): Screen Shot 2021-08-29 at 2 37 55 PM.

@shiffman, would you happen to have some ideas about why this would occur? It'd probably also be worthwhile to see if a demo straight from Tensorflow with the flipHorizontal: true configuration would have similar results.

tlsaeger commented 2 years ago

Hey, @lisajamhoury we where pinged by Aike.A on our Discord about this issue and I tried to look into this and give it a new go. I could not fix it at this point. What I found out is that the error that gets thrown has been Β»popularΒ« in the 2018. So this might be fixed in the meantime, what I'm thinking is that our Tensorflow implementation is not really up to date? Pinging @shiffman @joeyklee Maybe we just need to run an update. I will try what @bomanimc suggested and try the TF-Demo as well.

tlsaeger commented 2 years ago

@lisajamhoury I also added a workaround by just flipping the canvas before drawing it:

function draw() {
  //flipped Video before drawing it
  translate(width , 0)
  scale(-1,1)
  image(video, 0, 0, 640, 480);

https://editor.p5js.org/tlsaeger/sketches/UwlZIAaJa

lisajamhoury commented 2 years ago

Thanks @tlsaeger for taking another look at this and thinking of a workaround!

joeyklee commented 2 years ago

@tlsaeger -- Hi! Thanks for hopping on this! It has been a while since i've had space to work on ml5, so apologies for being absent from the latest discussions πŸ™ˆ . I haven't been following the developments on the tensorflow side, but I reckon that this is something like would likely have been implemented? 😬 I remember seeing in the Google Teachable Machine library specifically they'd implemented a flip on the webcam utility functions https://github.com/googlecreativelab/teachablemachine-community/blob/master/libraries/pose/src/utils/canvas.ts#L37 // maybe this can provide some inspiration for us?

tlsaeger commented 2 years ago

@joeyklee thanks for your input. You always had very good insights into the models, that's why i pinged you ;) I will take a look at this. I will try to find some time to dig into this flipp thing. @shiffman also gave some insights in the issue #1217