Skip to content

Update BodyPix to version 2.0#40

Merged
gohai merged 3 commits intomainfrom
model-bodysegmentation-rebased
Sep 7, 2023
Merged

Update BodyPix to version 2.0#40
gohai merged 3 commits intomainfrom
model-bodysegmentation-rebased

Conversation

@gohai
Copy link
Member

@gohai gohai commented Aug 24, 2023

Note: all this work was done by @VivianChenyc5519 and @ch3926. 👏 @gohai is shamelessly opening this PR after merely doing just a little bit of cleanup.

TODOs (hope those can be tackled after merging):

  • does the new result enable users to do everything they were able to in the old library? (maybe @MOQN knows?)
  • rework to match the the latest API pattern? (detect(), detectStart(), detectStop(), etc) à la @ziyuan-linn
  • rework examples to use preload?
  • add examples for imported image and video later
  • add Mediapipe support

VivianChenyc5519 and others added 3 commits August 24, 2023 16:45
Major changes in the code are as follows:
- Rewrite the code to match with the style of the current branches; Import eventemitter
- Update bodypix in accordance with tfjs repo - change model configuration and result structure, etc.
- Add three examples for bodypix (only supports webcam for now)
This follows the pattern of Handpose
For some reason, 1.7 doesn't refresh the webcam image in absence of a draw() function. Adding an empty draw fixes it. Note: the previous code worked fine in 1.6. Perhaps the better pattern is to only assign a variable in the callback, and only paint in draw?
@gohai gohai force-pushed the model-bodysegmentation-rebased branch from e3f6e50 to cd37683 Compare August 24, 2023 08:45
Copy link
Contributor

@VivianChenyc5519 VivianChenyc5519 left a comment

Choose a reason for hiding this comment

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

Assigning variable in the callback and only painting in draw would not work for body-segmentation in this case. I received an error trying to do so -- segmentation is not defined in draw(). And the main reason, I suppose, is that by the time I accessed segmentation, the assignment is not complete yet. And according to this stack overflow post, the only solution is to put everything under the callback function.

So I will stick with your work-around for now!

@gohai
Copy link
Member Author

gohai commented Aug 27, 2023

Assigning variable in the callback and only painting in draw would not work for body-segmentation in this case. I received an error trying to do so -- segmentation is not defined in draw(). And the main reason, I suppose, is that by the time I accessed segmentation, the assignment is not complete yet.

@VivianChenyc5519 You're right, because draw() will be called before the first gotResults(), we'd need to take some extra care not to access properties of the object before its ready. The most common way to do that is to test with an if (undefined will be "falsy", which makes this work):

if (segmentation) {
  image(segmentation.backgroundMask, 0, 0, width, height);
}

Copy link
Member

@shiffman shiffman left a comment

Choose a reason for hiding this comment

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

This is wonderful! @gohai I think you can merge when you feel it is ready. I would love @MOQN's thoughts also as I don't have much experience teaching with this model.

I agree we should emulate the detectStart() API model, but we could also consider segmentStart()? I'm not sure.

There is also the question as to whether this should be named ml5.bodypix() or if we should use a generic term like ml5.imageSegmentation("bodypix") with the name of the model passed in. The latter matches the ml5.bodyPose() and ml5.handPose() convention. However, BodyPix is perhaps developed in a unique/custom way that trying to make a class work with other image segmentation models might be too much trouble to maintain.

let video;
let segmentation;

let options = {
Copy link
Member

Choose a reason for hiding this comment

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

Just noting we might want to include a "hello world" version of this that just uses defaults. The options can sometimes overwhelm / confuse a beginner.

outputStride: 16, //adjust the output stride and see which one works best!
multiSegmentation: false,
segmentBodyParts: true,
flipHorizontal: true,
Copy link
Member

Choose a reason for hiding this comment

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

Is this something that the BodyPix model does natively or something you all added? I wonder if we should include an option like this for other models as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi Dan! If your question meant "why these four paramters", that's the latter three (multiSegmentation, segmentBodyParts, flipHorizontal) actually depends case by case. For example, if the user wants to detect multiple people and output different segmentation, then multiSegmentation needs to be turned on. As for outputStride, that's because I noticed this parameter could affect model performance, in terms of accuracy, so I thought this might be a good way to familiarize our users to machine learning.
I have set a default values for all these in index.js. We can create a default input for different cases like webcam or image for parameters like flipHorizontal and segmentBodyParts, but I think it might be a good idea to leave multiSegmentation (one segmentation for multiple people or multiple segmentation) to the user?
Lemme know if there's still something I didn't explain clearly or understand wrongly! Be happy to elaborate/correct :)

// Save the latest part mask from the model in global variable "segmentation"
segmentation = result;
//Draw the video
image(video, 0, 0, width, height);
Copy link
Member

Choose a reason for hiding this comment

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

For students to work from, in my experience it's best to have the drawing in draw() and use a if as @gohai suggests. Things go haywire quickly when students try to build on the code when there is drawing in a separate callback.

@gohai
Copy link
Member Author

gohai commented Sep 2, 2023

@VivianChenyc5519 @ch3926 Dear Vivian & Connie - pinging to make sure you see Dan's feedback (above). If you don't find the time to make any changes to this before our DURF wrap-up, then we can also do this at some later point.

@VivianChenyc5519
Copy link
Contributor

VivianChenyc5519 commented Sep 2, 2023 via email

@ch3926
Copy link
Collaborator

ch3926 commented Sep 2, 2023 via email

@shiffman
Copy link
Member

shiffman commented Sep 3, 2023

Another reason to keep it as ml5.imageSegmentation("bodypix") is to keep parallel with how tensorflow moved away from 'body-pix' towards "body-segmentation".

Oh! Thanks for pointing this out @ch3926! Body segmentation is a special case so maybe it's worth having a specific bodySegmentation() function instead of imageSegmentation("body")? I'm not sure! Here are some ideas:

// If we had different body segmentation models then an argument could be passed in
let bodySegmenter = ml5.bodySegmentation();
// If the function were just "image" you would have to specify body (and maybe a specific model?)
let bodySegmenter = ml5.imageSegmentation("body");

bodySegmentation() probably makes more sense b/c general image segmentation wouldn't know about body parts and other things specific to this kind of model that we may want to include (multiple people, "selfie" background removal, etc.)

@VivianChenyc5519
Copy link
Contributor

This is wonderful! @gohai I think you can merge when you feel it is ready. I would love @MOQN's thoughts also as I don't have much experience teaching with this model.

I agree we should emulate the detectStart() API model, but we could also consider segmentStart()? I'm not sure.

There is also the question as to whether this should be named ml5.bodypix() or if we should use a generic term like ml5.imageSegmentation("bodypix") with the name of the model passed in. The latter matches the ml5.bodyPose() and ml5.handPose() convention. However, BodyPix is perhaps developed in a unique/custom way that trying to make a class work with other image segmentation models might be too much trouble to maintain.

I think segmentStart would fit better in the context of body-segmentation. However, detectStart would make it more consistent. I am also a bit not sure about this. @ch3926 Just wondering what's Connie's view on this!

I would vote for
// If we had different body segmentation models then an argument could be passed in let bodySegmenter = ml5.bodySegmentation();

I will first integrate the detectStart() (or segmentStart() maybe) API model to body-segmentation and then work on the rest of the issues mentioned here.

@gohai gohai merged commit 86fcf80 into main Sep 7, 2023
@gohai gohai deleted the model-bodysegmentation-rebased branch September 16, 2023 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants