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

burnin crosshair detection #1458

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hdefazio
Copy link
Member

No description provided.

@hdefazio
Copy link
Member Author

@mleotta @daniel-riehm

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@mleotta
Copy link
Member

mleotta commented Nov 11, 2021

There is nothing too special in this code that ties it to OpenCV or VXL. This could all be written fairly easily using only VXL or only OpenCV. For that matter, it could even be written without either OpenCV or VXL (KWIVER vital only) with just a little more effort. However, there will be a bit of work either way to implement some simple image processing utilities.

This class is called moving_burnin_detector, but it seems to handle only the cross-hair case. Is there another class that handle the corners or the moving "N"? We might want to consider the broader set of functionality that needs to be ported before determining which way to go with this.

@hdefazio
Copy link
Member Author

There is nothing too special in this code that ties it to OpenCV or VXL. This could all be written fairly easily using only VXL or only OpenCV. For that matter, it could even be written without either OpenCV or VXL (KWIVER vital only) with just a little more effort. However, there will be a bit of work either way to implement some simple image processing utilities.

This class is called moving_burnin_detector, but it seems to handle only the cross-hair case. Is there another class that handle the corners or the moving "N"? We might want to consider the broader set of functionality that needs to be ported before determining which way to go with this.

Good, I can start with removing opencv and we can go from there. The original code also has brackets and text detectors, I am just waiting to get the crosshairs working before adding those in.

@mleotta
Copy link
Member

mleotta commented Nov 12, 2021

@hdefazio Can you point me at the original code? I'm worried that some of the other code has stronger dependencies on OpenCV. We don't want to port part of this if we can't easily port the rest later in the same way.

I'm also concerned that it's not worth porting this code in its current form to use VXL instead of OpenCV. This code is way more complicated than it needs to be. It's probably better to just rewrite a much simpler version of it. Maybe we keep this version as is as a baseline for comparison. We should probably get this working in it's current form, using both VXL and OpenCV, first. So we can do that in the burnout arrow for now.

Longer term I think we should do all of this (cross-hairs, corners, and text) with template matching. I believe at least the text matching does this. There is no reason for this custom code to match the cross-hair when you can just render a cross-hair template and use it in the same template matching code.

@hdefazio
Copy link
Member Author

@mleotta The original code lives here: https://kwgitlab.kitware.com/computer-vision/vidtk/-/blob/master/library/object_detectors/moving_burnin_detector_opencv.cxx I was under the impression that this was all template matching. But I may have misunderstood.

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@hdefazio hdefazio force-pushed the dev/burnout_pattern_match branch from be1a05a to 3398d2f Compare December 9, 2021 21:22
@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@kwcvrobot
Copy link
Collaborator

@hdefazio hdefazio changed the title WIP: burnout pattern matching burnin crosshair detection Dec 15, 2021
// convert image to cv
cv::Mat cv_image = ocv::image_container::vital_to_ocv(
input_image->get_image(),
ocv::image_container::RGB_COLOR );
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this ocv::image_container::RGB_COLOR and a bit later the ocv::image_container::BGR_COLOR might be giving you problems. First, they don't match, which is probably intentional but I figure I'd mention it anyway. Second, if you look at the implementation of these functions in arrows/ocv/image_container.cxx, ocv_to_vital doesn't use the ColorMode parameter at all, and vital_to_ocv only explicitly handles the BGR_COLOR case and lumps everything else into a bucket, not really doing anything with it. It looks to me whoever wrote these functions only really worked with BGR images, so I might investigate if they are actually doing what you hope they would be doing. Perhaps a cause of the pixel weirdness you were seeing?

@mleotta mleotta added this to the v1.8 milestone Apr 12, 2022
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