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

Update calculations to assume undistorted space #4

Open
wants to merge 9 commits into
base: master
from

Conversation

@romanroibu
Copy link
Member

romanroibu commented Mar 16, 2020

Addresses #3

romanroibu added 7 commits Mar 16, 2020
…atrices from location calculation
@romanroibu romanroibu requested review from marc-tonsen and s-westphal Mar 16, 2020
@romanroibu

This comment has been minimized.

Copy link
Member Author

romanroibu commented Mar 16, 2020

hey @s-westphal,

I've updated the library to only use undistorted coordinates. Please try it out and let me know if there are any issues that you find. Thanks!

@s-westphal

This comment has been minimized.

Copy link

s-westphal commented Mar 17, 2020

hi @romanroibu
looks good!
Two more questions:

  • Would it be possible to add a surface element as an optional initialization parameter for SurfaceTracker, such that define_surface does not need to be called? Or a method set_surface(surface) that can be called instead of define_surface. This would allow us to save the surface, e.g. in a database, and then later get a SurfaceTracker instance and only locate or modify the surface.
  • Can we allow to call define_surface with an empty list of markers? Markers are then iteratively added with an add_markers_to surface function, and a locate_surface can only be called if there are any markers added to the surface
    Thank you!
@romanroibu

This comment has been minimized.

Copy link
Member Author

romanroibu commented Mar 17, 2020

Answering your questions:

  • You can do this now, there is no need for a set_surface method. You can serialize the surface definition, store it somewhere, then deserialize the stored definition and use it with an instance of surface tracker.
  • Well, the constraint was that the surface is defined by 1+ markers. I'd have to check what I need to change to relax this constraint. As a workaround, with the iterative approach you suggested, you could define the surface on the first iteration, and then add markers. Also, could you please describe why you need this iterative process?
@s-westphal

This comment has been minimized.

Copy link

s-westphal commented Mar 17, 2020

for (1) that's perfect, thanks!
for (2) it would be great if it's possible, I didn't know of this constraint, if not, I will have to come up with another solution for our problem
We need it for the enrichments, when you create a surface_tracker enrichment, we would like to intitialize it 'empty' then the user can select a recording and a frame to define the surface.
There will also be a 'reset'-button that deletes the current surface, so set the makers to an empty list

@romanroibu

This comment has been minimized.

Copy link
Member Author

romanroibu commented Apr 3, 2020

Pending review of #6


@marc-tonsen, @euryalus Could you please review the changes in #6 when you have time, and let me and @s-westphal know what you think about it? Thank you!

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.