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

Adding getters and setters in VideoCaptureWrap #559

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

Conversation

fakob
Copy link
Contributor

@fakob fakob commented Oct 15, 2017

Hi,

I have added getPosition, getPositionMS, getFPS, getFourCC and setPositionMS.
setPositionMS is a duplicate of getFrameAt, but as it sets the position in MS I find getFrameAt confusing and would deprecated that in the future.

What do you think?

@codecov-io
Copy link

Codecov Report

Merging #559 into master will decrease coverage by 0.28%.
The diff coverage is 16.12%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #559      +/-   ##
=========================================
- Coverage   51.19%   50.9%   -0.29%     
=========================================
  Files          34      34              
  Lines        3899    3929      +30     
  Branches       27      27              
=========================================
+ Hits         1996    2000       +4     
- Misses       1903    1929      +26
Impacted Files Coverage Δ
src/VideoCaptureWrap.h 0% <ø> (ø) ⬆️
src/VideoCaptureWrap.cc 43.28% <16.12%> (-5.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52333d4...6855c81. Read the comment docs.

@peterbraden
Copy link
Owner

Looks great, any chance you could add an example file or some tests? Thanks!

@fakob
Copy link
Contributor Author

fakob commented Nov 1, 2017

I have never done that before as I am not a professional :-), but I will look into it.

@btsimonh
Copy link
Collaborator

hi fakob - in terms of a test, copy an existing example (e.g. examples/motion-track.js), and make it access each of the getters/setters in VideoCapture - this should be enough to ensure that they get tested regularly, and should contribute to coverage percentage. I just did that for examples/bgsubtractor.js ; if you want another video-reading sample to look at.

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