-
Notifications
You must be signed in to change notification settings - Fork 7.2k
[torchvision video reader]inception commit #1279
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
[torchvision video reader]inception commit #1279
Conversation
| video_reader_src, | ||
| include_dirs=[ | ||
| video_reader_src_dir, | ||
| '/home/zyan3/local/anaconda3/envs/pytorch_py3/include', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fmassa , I will remove this line.
For header files from ffmpeg, we need to ensure they are installed at default header file search path.
4eec101 to
b3f2a6e
Compare
…vision into torchvision_video_reader
…vision into torchvision_video_reader
…vision into torchvision_video_reader
* fixed typo * fixed some more typos and grammer
* make shufflenet scriptable * make resnet18 scriptable * set downsample to identity instead of __constants__ api * use __constants__ for downsample instead of identity * import tensor to fix flake * use torch.Tensor type annotation instead of import
|
Thanks a lot for the PR Zhicheng! The first thing I need to figure out before we can merge this is how we will be adding ffmpeg as a dependency for torchvision, and if it will be a soft or hard dependency. A few options:
Also, what is the version of FFMpeg that we will be relying upon? Another thing I need to do is to get CI working for Windows and OSX in torchvision, so that we can make sure that this PR compiles and works nicely in the other OS that torchvision supports. I'll be looking into both the CI and ffmpeg dependency from an OSS perspective. |
|
i think it might be a good start to start with (1), i.e. the ffmpeg from conda-source or system package manager (brew install ffmpeg / apt install ffmpeg). Also, by ffmpeg I presume you mean For binaries, we will figure out how to ship ffmpeg the right way ourselves. Just building ffmpeg from source is not sufficient btw, because you need to build it with codec support, and there are tons of codecs we need to build it with. |
sounds good, I'll be looking into option (1) first (once I get full CI running).
We need the underlying libraries that are composed of
Sounds good |
| } | ||
| } | ||
|
|
||
| <<<<<<< Updated upstream |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that you forgot some merge conflicts in this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey Lowik, it is fixed in the replacement PR (#1303)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few things to clean up but this looks very promising! Exciting stuff!!!
|
|
||
| class BasicBlock(nn.Module): | ||
| expansion = 1 | ||
| __constants__ = ['downsample'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you separate model changes in a separate PR to track it more easily?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct. I mess up this PR. I will abandon this PR and create a more clean PR.
| audio_timebase = Fraction(0, 1) | ||
| if "audio_timebase" in info: | ||
| audio_timebase = info["audio_timebase"] | ||
| audio_start_pts = pts_convert( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it makes sense to keep the global pts as opposed to doing this conversion?
In case we have more than two streams, we'd have to add more of these if clauses at every iteration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I think it might be better to use a global metric, like seconds for example
| import collections | ||
| from common_utils import get_tmp_dir | ||
| from fractions import Fraction | ||
| import logging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't seen logging used in torchvision in general. What is the best prActice for this @fmassa?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't currently use logging in torchvision, only emit some deprecation warnings at some places.
I'm not yet sure we want to add logging as of now, it might deserve a larger discussion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Those logging are mostly used for my dev. I will remove them now.
|
Abandon this PR. |
Implement a C++ video decoder, and refer to it as TorchVision (TV) video reader in the following.
Main features
AV_PIX_FMT_RGB24)AVSampleFormat(default:AV_SAMPLE_FMT_FLT)APIs
The main API includes
FfmpegDecoder::decodeFile(....): decode frames from a given video file. This is useful for both OOS and FB research projects, where videos reside in file folder.FfmpegDecoder::decodeMemory(....): decode frames from a given compressed video byte array. This is useful for decoding everstore videos.Sanity check
Benchmark
We use several videos from HMDB-51, UCF-101 and Kinetics-400 for benchmarking and unit test. Test videos are listed below.
RATRACE_wave_f_nm_np1_fr_goo_37.avi
SchoolRulesHowTheyHelpUs_wave_f_nm_np1_ba_med_0.avi
TrumanShow_wave_f_nm_np1_fr_med_26.avi
v_SoccerJuggling_g23_c01.avi
v_SoccerJuggling_g24_c01.avi
R6llTwEh07w.mp4
SOX5yA1l24A.mp4
WUzgd7C1pWA.mp4
Unit test
Results of unit test are attached.
[torchvision video reader unit test.log]
torchvision.video.reader.unit.test.log
Comparison with PyAv