-
Notifications
You must be signed in to change notification settings - Fork 40
add legacy object detection cocossd code and example from p5js #257
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
base: main
Are you sure you want to change the base?
Conversation
Yay! Great job @yiyujin! As discussed, if it's helpful here's a quick list of things that we changed with ml5.js 1.0.
|
…urn result not error
…ct (instance.model), added mediaReady utils so that detection waits for video to be ready before detecting, updated tf.backend to webgl to avoid webgpu warning
Thank you @shiffman for the neat instructions!
|
Based on today's meetup with @shiffman cc.@nasif-co After all, final output for the project would be the p5js example code. Example code (in order of priority) : bcd1788
I guess next will be the docs, which I can draft out in parallel with the code ! |
…de to call detectStart in setup()
I made the 3 changes to code so it works with ml5.js 1.0 💪 This week, I want to digest code better and draft a docs that explains the common pattern (functions and its roles) used across models : start, stop etc. I need it for myself anyways + could be a useful onboarding docs for future dev team. Some notes on naming conventions : (speaking of which.. I think my branch should have been object-detector, not object-detection..? 😭)
|
After a brief code review from @pearmini 🙏 (wow I mindlessly copied the code to begin with then didn't think through!)
@pearmini can you take a glance at this please? |
Just dropping notes for future tasks :
|
@shiffman how does the example code for webcam look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @yiyujin fantastic work! I left a few small comments in the basic example!
examples/objectDetection/sketch.js
Outdated
ml5 Example | ||
Object Detection using COCOSSD | ||
This example uses a callback pattern to create the classifier | ||
=== */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tiny thing, but sometime this year we adopted a "friendlier" pattern for comments at the top of an example sketch. I don't think we need the copyright and technically the license should be the ml5.js one. Here is what it looks like from a hand pose example:
/*
* 👋 Hello! This is an ml5.js example made and shared with ❤️.
* Learn more about the ml5.js project: https://ml5js.org/
* ml5.js license and Code of Conduct: https://github.com/ml5js/ml5-next-gen/blob/main/LICENSE.md
*
* This example demonstrates face tracking on live video through ml5.faceMesh.
*/
(This also reminds me that we discussed moving the Code of Conduct to the website, I forgot where we are with that though maybe @MOQN remembers?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll include it for sure!
examples/objectDetection/sketch.js
Outdated
image(video, 0, 0); | ||
|
||
for (let i = 0; i < detections.length; i += 1) { | ||
const detection = detections[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const detection = detections[i]; | |
let detection = detections[i]; |
We're adopting p5.js style of using let
even where const
would be more typical JS.
|
||
function gotDetections(results) { | ||
detections = results; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't over do it, but a few concise explanatory comments might be good to add.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shiffman thank you for the comments! I will update it for review.
switch (this.modelName) { | ||
case "cocossd": | ||
this.modelToUse = cocoSsd; | ||
break; | ||
case "yolo": | ||
this.modelToUse = yolo; | ||
break; | ||
// more models... currently only cocossd is supported | ||
default: | ||
console.warn(`Unknown model: ${this.modelName}, defaulting to CocoSsd`); | ||
this.modelToUse = cocoSsd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Where is variable yolo and coco defined?
- You can move the switch logic into the wrapper function, so you pass the model directly instead a string.
…ng transparent canvas on top of video)
I updated the three example codes -> bcd1788 - webcam (examples/objectDetection-webcam/sketch.js) I'm esp. not sure if the video file example is at its best, since I'm adding a bunch of video eventListeners to check if video is loadedmetadata/play/pause/ended. I don't think p5 has a method on video to do that if I'm correct.. |
canvasElement = createCanvas(640, 480); | ||
canvasElement.position(0, 0); | ||
|
||
// Create video element (paused by default) | ||
video = createVideo('test.mov'); | ||
video.position(0, 0); | ||
video.volume(0); | ||
video.showControls(); | ||
|
||
// Make canvas transparent and on top | ||
canvasElement.style('z-index', '1'); | ||
canvasElement.style('pointer-events', 'none'); // Allow clicks to pass through to video | ||
video.style('z-index', '-1'); | ||
|
||
// Set up video event listeners | ||
video.elt.addEventListener('loadedmetadata', () => { | ||
console.log('Video metadata loaded'); | ||
|
||
// Resize canvas to match video size | ||
resizeCanvas(video.elt.videoWidth, video.elt.videoHeight); | ||
}); | ||
|
||
video.elt.addEventListener('play', () => { | ||
console.log('Video started playing'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I woudl avoid all this extra code if you can! You can treat the video just like the webcam, have it auto-start and loop. Here's an example from BlazePose you can use as a model:
https://editor.p5js.org/codingtrain/sketches/ftALPDieT
But yours can be simpler b/c you can hide the video element and draw it on the canvas.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh okay! I will forget about showControls and try video.hide + video.loop(). Thank you 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A couple last small comments on the video example and then some discoveries about video resizing! Great work @yiyujin!
|
||
// Load and loop the video for object detection | ||
video = createVideo('test.mov'); | ||
video.size(width, height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting our discussion to use a video we have permission for. Also, I would suggest sizing the video to 640x480 so it doesn't appear squashed or skewed. Or just make the canvas size identical to the video. You can add a code comment that explains that if the video does not match the canvas size, you may need to adjust the code to scale.
let scaleX = width / video.elt.videoWidth; | ||
let scaleY = height / video.elt.videoHeight; | ||
|
||
for (let i = 0; i < detections.length; i++) { | ||
let detection = detections[i]; | ||
|
||
let x = detection.x * scaleX; | ||
let y = detection.y * scaleY; | ||
let w = detection.width * scaleX; | ||
let h = detection.height * scaleY; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noting that if the video is sized the same as the canvas then this is unnecessary which is preferable for a basic example. However, if we do have to use this we should use the p5.js video.width
and video.height
property directly.
I would have thought if you call video.size(640, 480)
then the x, y, width, height returned would match the new size. Could this be related to the bug @nasif-co is fixing in #264??
createCanvas(640, 480); | ||
|
||
// Using webcam feed as video input, hiding html element to avoid duplicate with canvas | ||
video = createCapture(VIDEO); | ||
video.size(width, height); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I just tested and confirm that the #264 applies here too! If I change the webcam to 320,240
then the bounding boxes are all off. @yiyujin I don't think this has to be fixed in this PR, it can be done separately after the fact or along with #264. @nasif-co maybe you can help track this in the issue thread or a new issue so that we can merge at least the SelfieSegmentation fix?
https://github.com/ml5js/ml5-next-gen/tree/main/examples#p5js-20-examples -> will be handled in another PR |
937ba8b
to
02bdb6d
Compare
Bring back object detection in ml5.js
To start, I added legacy object detection code) and p5js example code. Example code is from github reference doc
History
Make code compatible with ml5 1.0Make code scalable to support more models
Next Steps
- p5 2.0 examples : Having this coco-ssd work with p5.js 2.0 is priority over supporting Transformers.js.