VL.OpenCV icon indicating copy to clipboard operation
VL.OpenCV copied to clipboard

Added Non-Maximum Suppression (NMS) in YOLOv3 and example

Open dani217s opened this issue 11 months ago • 3 comments

This proposal add a new node S+H(OpenCv) that works with OpenCv Image as S+H works with generic data.

There are also changes that implements Non-max suppression (NMS) on Yolo3 Detection Node and a new Node that implents Face Detection based on caffe model as discussed by Aman Preet Gulati on https://www.analyticsvidhya.com/blog/2022/04/face-detection-using-the-caffe-model/

Almost all changes on VL are added in VL.OpenCv.AddOn.vl file.

This is the list of changes done on VL.OpenCv.vl:

VL-OpenCV-vl_changes

VL-OpenCV-vl_changes advanced

VL-OpenCV-dependency

VL-OpenCV-dependency_forwarded

dani217s avatar Mar 08 '24 14:03 dani217s

Wow, thanks for this! I will have a look and get back to you but in general, it looks good.

For now though, could you please stick with the naming convention and change OpenCv to OpenCV wherever needed?

Sorry for being picky on this matter but I think consistency is important.

Thanks again!

ravazquez avatar Mar 08 '24 21:03 ravazquez

Yes, sure, I agree with you about consistency. I've fixed wrong file name. Thanks!

dani217s avatar Mar 09 '24 18:03 dani217s

Hello again @dani217s ,

Apologies for the delayed response, as you can imagine the recent release had people busy on other things for a bit.

First of all, thanks for the great work. From a technical perspective everything is as it should be.

There is one point of concern around redistributing the model files as part of the nuget/git repo, both because of copyright issues and file-size/traffic. For these reasons we would rather remove the files and have just a clean set of instructions in the help patch, with a link to the needed files, which the end user will have to download on their first run (like we have for the original YOLO help patch).

At this point, even reverting the addition of the caffemodel file would leave it in the git history, so we cannot accept this PR as is.

Since a new PR is needed, and in a way this one covers multiple things at once (S+H, NMS change on the YOLO node and a new node for face recognition), it would also be best to make a separate PR for each change/feature instead of merging them all in one, it is easier to track, manage and understand things in the future that way.

There are some minor cosmetic issues as well that I hope are easy to fix:

  • YOLO3Detector node has a left over/forgotten "builder" Pad
  • Caffe Model help patch: - Default camera is selected in IOBox and shows up yellow for me, remove IOBox to avoid confusion - typo : Usefoul - typo: OpenCv - typo: opencv's - Lower caps use on start of paragraph
  • Yolo 3 label detection help patch - IOBoxes for links are cut off, make them wider - Patch could use a general tidy up - Lowering threshold to .2 or less gives better result

So, if you can make new individual PRs with the described changes we should be good to go.

Sorry if it feels like a hassle, we do very much appreciate your effort and help.

Thanks again!

Cheers.

ravazquez avatar Apr 10 '24 19:04 ravazquez

Hello @ravazquez ,

I've been busy due to work. I'll be doing separate PRs, so I'm done with this one. Thanks for your suggestions!

dani217s avatar Jul 12 '24 09:07 dani217s