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

Roi plugin #1788

Open
wants to merge 33 commits into
base: develop
from
Open

Roi plugin #1788

wants to merge 33 commits into from

Conversation

@pfaion
Copy link
Contributor

pfaion commented Jan 9, 2020

This PR cleans up the detector ROI code and moves it to a plugin.

While moving the stuff over, I found a ton (!) of bugs, unhandled cornercases and UI inconsistencies that were not handled correctly. The new ROI should be a lot more stable and provide better UX.

Rationale

  1. Get rid of duplicate (plugin) logic for roi in eye.py. I also found bugs in this.
  2. Allow correct ordering of the ROI processing (after backend, before detectors). This is important because we had some frame drops and hack because the old ROI was processed after the detectors (which were plugins).
  3. Simplify confusing roi properties. The roi grew over time and just had added properties. In the end there were (for x direction): lx, ux, nx, minx, maxx, frame_width where you only need minx, maxx, frame_width.

API Changes
I changed the network format for setting the ROI from
(minx, maxx, miny, maxy) to
(minx, MINY, MAXX, maxy)
This is the same format that the old roi used internally, the new roi uses internally and the detectors use internally. This will make the code more consistent, but we need to inform users that the API changed! I added improved error logging for setting the roi over the network to provide better feedback when this causes problems.

pfaion added 30 commits Jan 7, 2020
Since we don't do comparison based on index anymore, we can just use a
regular Enum.
Also add stricter format checking here and better feedback.
We don't even need the ROI for that, since this only applies when the frame resolution changes, not when the ROI changes.
pfaion added 3 commits Jan 10, 2020
@pfaion pfaion requested review from papr and romanroibu Jan 10, 2020
@pfaion pfaion added the API changes label Jan 10, 2020
@pfaion pfaion marked this pull request as ready for review Jan 10, 2020
Copy link
Member

romanroibu left a comment

great job! 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.