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

extend video reader to support fast video probing #1437

Merged
merged 4 commits into from
Oct 12, 2019

Conversation

stephenyan1231
Copy link
Contributor

Changes

Extend video reader to support probing the video quickly for getting video meta information

  • In io module, add new methods _probe_video_from_file and _probe_video_from_memory

unit test

  • python test/test_video_reader.py
  • python test/test_io.py

Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

This is ok for now, but I think we should probably refactor in the future the way we expose those attributes properties.

Also, C++ linter is failing in https://travis-ci.org/pytorch/vision/jobs/595456375, could you fix it?

The osx_wheel failures are unrelated to this PR.

Once linter is fixed this is good to merge.

@codecov-io
Copy link

codecov-io commented Oct 9, 2019

Codecov Report

Merging #1437 into master will increase coverage by 0.09%.
The diff coverage is 53.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1437      +/-   ##
==========================================
+ Coverage   64.01%   64.11%   +0.09%     
==========================================
  Files          80       80              
  Lines        6308     6325      +17     
  Branches      968      970       +2     
==========================================
+ Hits         4038     4055      +17     
+ Misses       1989     1984       -5     
- Partials      281      286       +5
Impacted Files Coverage Δ
torchvision/io/__init__.py 100% <ø> (ø) ⬆️
torchvision/io/_video_opt.py 40.65% <53.84%> (+13.63%) ⬆️

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 79daca1...f5cf884. Read the comment docs.

@stephenyan1231
Copy link
Contributor Author

@fmassa Make sense. Let's do it in the next refactoring PR.

@stephenyan1231 stephenyan1231 force-pushed the video_reader_video_probing branch from f5cf884 to f4708cf Compare October 12, 2019 16:06
Copy link
Member

@fmassa fmassa left a comment

Choose a reason for hiding this comment

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

Thanks a lot Zhicheng!

@fmassa fmassa merged commit ed5b2dc into pytorch:master Oct 12, 2019
facebook-github-bot pushed a commit to facebookresearch/ClassyVision that referenced this pull request Oct 16, 2019
Summary:
Pull Request resolved: #62

Current dependency torchvision 0.4.0 was released in August.
It missed quite a few PRs that are merged after that, and that are needed for video classification, such as

- pytorch/vision#1437
- pytorch/vision#1431
- pytorch/vision#1423
- pytorch/vision#1418
- pytorch/vision#1408
- pytorch/vision#1376
- pytorch/vision#1363
- pytorch/vision#1353
- pytorch/vision#1303

This will fail the CI test when a diff uses changes made in those PRs.
Before a new official version of TorchVision is released, we can temporarily use the nightly torchvision to get all the recent PRs, and unblock the PR merging.
We plan to use a fixed version of TorchVision later.

Reviewed By: vreis

Differential Revision: D17944239

fbshipit-source-id: 86ff540e3fc4f08ef767e84ef103525db5158201
@fmassa fmassa mentioned this pull request Oct 31, 2019
fmassa pushed a commit that referenced this pull request Oct 31, 2019
* extend video reader to support fast video probing

* fix c++ lint

* small fix

* allow to accept input video of type torch.Tensor
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.

3 participants