Bugfixes for upcoming v1.7 release #1157
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
papr
commented
Apr 17, 2018
papr
commented
- Fix Add blink detection plugin to Pupil Service #1137
- Use correct timing for offline video sources
- Fix jumping to fixations in Player Pupil Player 1.6.13 | Offline Fixation Detector Crash | Previous & Next Fixation Button click #1151
mkassner
reviewed
Apr 18, 2018
| """ | ||
| Undistorts points according to the camera model. | ||
| cv2.fisheye.undistortPoints does *NOT* perform the same unprojection step the original cv2.undistortPoints does. | ||
| cv2.fisheye.unprojectPoints does *NOT* perform the same unprojection step the original cv2.unprojectPoints does. |
There was a problem hiding this comment.
I think copy replace was applied a bit to generously in this case.
mkassner
reviewed
Apr 18, 2018
| # f = np.array((self.K[0, 0], self.K[1, 1])).reshape(1, 2) | ||
| # c = np.array((self.K[0, 2], self.K[1, 2])).reshape(1, 2) | ||
| # xy_undist = xy_undist * f + c | ||
| # xy_undist = cv2.fisheye.undistortPoints(xy, self.K, self.D, P=self.K) | ||
| # xy_undist = cv2.fisheye.unprojectPoints(xy, self.K, self.D, P=self.K) |
There was a problem hiding this comment.
please make sure this change is correct. Seems fishy.
mkassner
reviewed
Apr 18, 2018
| if xy.ndim == 2: | ||
| xy= np.expand_dims(xy, 0) | ||
|
|
||
| xy_undist = cv2.fisheye.undistortPoints( | ||
| xy_undist = cv2.fisheye.unprojectPoints( |
mkassner
requested changes
Apr 18, 2018
mkassner
reviewed
Apr 18, 2018
| @@ -83,7 +83,10 @@ def fixation_from_data(dispersion, method, base_data, timestamps=None): | |||
|
|
|||
| def vector_dispersion(vectors): | |||
| distances = pdist(vectors, metric='cosine') | |||
| return np.arccos(1. - distances.max()) | |||
| distances.sort() # sort by distance | |||
There was a problem hiding this comment.
reference to lit or a small explanation?
mkassner
reviewed
Apr 18, 2018
| @@ -1,126 +0,0 @@ | |||
| # Pupil Time Sync Protocol | |||
There was a problem hiding this comment.
Maybe we misunderstood each other, I thought we leave this here but link from Pupil helpers to here. (have the spec stay with implementation.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.