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

Object Detector Abstraction #3656

Merged
merged 12 commits into from
Nov 4, 2022

Conversation

NateMeyer
Copy link
Contributor

This is an initial change to abstract the edgtpu module to a more generic Object Detector. The idea is to be able to add different inference runtimes into the Detectors folder, add a new DetectorTypeEnum, and an else-if in the LocalObjectDetector init function to setup the new detector type.

The object_detection module keeps the multi-process logic to run the detector asynchronously and apply the detection thresholds. All the detector modules are required to do is setup the runtime, load the model, and process a tensor (detect_raw).

Future work to build on this:

  • Add an OpenVino detector module
  • Add a RKNPU/RKNN detector module
  • Possibly add a config for a "plugin" folder that will be scanned at runtime for user-supplied detector implementations?

frigate/object_detection.py Outdated Show resolved Hide resolved
frigate/object_detection.py Outdated Show resolved Hide resolved
frigate/object_detection.py Outdated Show resolved Hide resolved
@NickM-27
Copy link
Sponsor Collaborator

Thanks, this will be a good basis for #3626

@NickM-27
Copy link
Sponsor Collaborator

It would be nice to add some tests for this new module. Might be difficult / not a good idea to test actual detection, but at least testing that the correct detector is used when sending config metadata would be good.

@NateMeyer
Copy link
Contributor Author

It would be nice to add some tests for this new module. Might be difficult / not a good idea to test actual detection, but at least testing that the correct detector is used when sending config metadata would be good.

Shouldn't be too hard if we can mock the edgetpu detector inside a unit test? I'll take a look at that.

@NickM-27
Copy link
Sponsor Collaborator

Yeah that's what I was thinking

@NateMeyer
Copy link
Contributor Author

As I am working on an OpenVino detector using this branch, I'm finding it would be helpful if the configuration for a model were more descriptive. Has there been any work to adjust the shape of the input/output tensors for different models?

@NateMeyer
Copy link
Contributor Author

I see some work in #1022, #3099, and #2548. Was there any preference on how any of these handled different models?

@blakeblackshear
Copy link
Owner

I would prefer that each model implementation defines it's own function to prepare the input data in shared memory and then translates the outputs into a common format. Keep in mind that the detection process is extremely sensitive to small increases in processing time. When using a Coral, an extra copy of a frame in memory is enough to increase the total inference times by a significant percentage.

@NateMeyer
Copy link
Contributor Author

Looks like the input tensor shape can be transformed without a memory copy. This can be handled by the detector, so I changed the "model_shape" to be the whole model config in most places. The one place I needed to use the model config in video.py is to set the correct colorspace when we convert from YUV. We'll only want to do this conversion once, so we need to know the correct colorspace for the model input at that time.

Copy link
Sponsor Collaborator

@NickM-27 NickM-27 left a comment

Choose a reason for hiding this comment

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

Generally looks good to me, will defer to Blake on the model specifics

@blakeblackshear
Copy link
Owner

I haven't had time to review this in detail yet, but Frigate+ models will be trained on the YUV colorspace, so don't make any assumptions about colorspace here.

@NateMeyer
Copy link
Contributor Author

The default colorspace should remain RGB as was used for the current default model and there is an option for YUV that will only crop/resize and skip the colorspace transformation call. I was finding a mix of RGB and BGR in the Intel Open Model Zoo.

@NateMeyer NateMeyer mentioned this pull request Sep 5, 2022
5 tasks
@NickM-27
Copy link
Sponsor Collaborator

NickM-27 commented Oct 2, 2022

Please rebase this PR and to point it to dev branch

@NateMeyer NateMeyer changed the base branch from release-0.11.0 to dev October 2, 2022 19:38
@NateMeyer
Copy link
Contributor Author

I rebased and pushed one more commit for the model input config. I switched it to an enum to specify NCHW vs NHWC tensor shapes instead of an array of arbitrary format. I thought that other shapes might be out there, but I haven't come across any besides these two yet. Best to keep it easier to configure for now.

@blakeblackshear blakeblackshear merged commit 4383b88 into blakeblackshear:dev Nov 4, 2022
herostrat pushed a commit to herostrat/frigate that referenced this pull request Nov 24, 2022
…kshear#3656)

* Refactor EdgeTPU and CPU model handling to detector submodules.

* Fix selecting the correct detection device type from the config

* Remove detector type check when creating ObjectDetectProcess

* Fixes after rebasing to 0.11

* Add init file to detector folder

* Rename to detect_api

Co-authored-by: Nicolas Mowen <[email protected]>

* Add unit test for LocalObjectDetector class

* Add configuration for model inputs
Support transforming detection regions to RGB or BGR.
Support specifying the input tensor shape.  The tensor shape has a standard format ["BHWC"] when handed to the detector, but can be transformed in the detector to match the model shape using the model  input_tensor config.

* Add documentation for new model config parameters

* Add input tensor transpose to LocalObjectDetector

* Change the model input tensor config to use an enumeration

* Updates for model config documentation

Co-authored-by: Nicolas Mowen <[email protected]>
@triptec
Copy link

triptec commented Nov 26, 2022

@NateMeyer this looks great! May I ask if you have any example config/model where you use this new functionality?

@NateMeyer
Copy link
Contributor Author

@NateMeyer this looks great! May I ask if you have any example config/model where you use this new functionality?

Hey. The config changes should be included in the documentation updates. I implemented an OpenVino detector using this abstraction in #3768

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants