Improve heuristic for variable frame rate detection #1242
Conversation
Description: Previous heuristic would fail for videos with a fractional fps, such as 29.97(30000/1001). It would classify such videos as vfr even if they might be cfr. This change will compare the frame rate reported by the container to an estimate based off on the first video frame. While this check is not exhaustive but in my testing it was accurate. Signed-off-by: Abhishek Sansanwal <asansanwal@nvidia.com>
Description: This test case tests new heuristic that correctly identifies cfr videos with fractional fps as cfr instead of vfr. Depends upon NVIDIA/DALI_extra#25 Signed-off-by: Abhishek Sansanwal <asansanwal@nvidia.com>
| @@ -217,9 +230,18 @@ OpenFile& VideoLoader::get_or_open_file(const std::string &filename) { | |||
| stream->avg_frame_rate.num}; | |||
|
|
|||
| // This check is based on heuristic FFMPEG API | |||
| AVPacket pkt = AVPacket{}; | |||
| int ret; | |||
| while ((ret = av_read_frame(file.fmt_ctx_.get(), &pkt)) >= 0) { | |||
There was a problem hiding this comment.
I started to wonder, how costly this check can get? Maybe we should provide it as an option? Something like allow_fractional_frame_rate? And normally just omit it and print an error with info that in fact it might be fractional frame rate and user should try that? Or maybe the performance penalty is not that high? Just a loose thought.
There was a problem hiding this comment.
For all the videos that I tested with the video frame is either the first frame or the second frame. So this while loop will probably only run once or twice in majority of videos. We break when we find the first video frame.
Something like allow_fractional_frame_rate?
The reason i dont want to hide this check behind a flag is because for example NTSC uses 29.97 fps which is common. The ucf-101 dataset has videos with this fps.
I also ran benchmark on about 1200 files which showed no significant differences between the two heuristics. But this could be biased.
-------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations UserCounters...
-------------------------------------------------------------------------------------------------------------------
VideoReaderBench/VideoPipe/2/1/4/9/iterations:100/real_time 62 ms 0 ms 100 FPS=81.5763/s
-------------------------------------------------------------------------------------------------------------------
Benchmark Time CPU Iterations UserCounters...
-------------------------------------------------------------------------------------------------------------------
VideoReaderBench/VideoPipe/2/1/4/9/iterations:100/real_time 62 ms 0 ms 100 FPS=81.545/s
An alternative I can imagine is to combine it with the previous heuristic, When previous heuristic thinks that the video is VFR then we can try to verify again with this new heuristic.
But I can also imagine a video which looks like constant frame rate to the previous heuristic but is actually vfr.
What's your opinion ?
There was a problem hiding this comment.
I think we should leave it as it is now. You are right. It's more robust to errors and issues as it is now.
There was a problem hiding this comment.
To run the CI we need to update DALI_EXTRA_VERSION
All necessary files are already merged to the master there.
Signed-off-by: Abhishek Sansanwal <asansanwal@nvidia.com>
a-sansanwal
force-pushed
the
videoreader_improve_vfr_detection
branch
from
f8d942f
to
7b90026
Compare
|
!build |
|
CI MESSAGE: [897160]: BUILD STARTED |
awolant
requested review from
klecki,
banasraf,
jantonguirao,
szalpal and
mzient
|
CI MESSAGE: [897160]: BUILD PASSED |
Description: Previous heuristic would fail for videos with a fractional fps, such as 29.97(30000/1001). It would classify such videos as vfr even if they might be cfr. This change will compare the frame rate reported by the container to an estimate based off on the first video frame. While this check is not exhaustive, but in my testing it was accurate. Why we need this PR? Refactoring to improve what Improves vfr detection heuristic. What happened in this PR? Explain solution of the problem, new feature added. Compare the frame rate reported by the container to an estimate based off on the first video frame What was changed, added, removed? Replaced old check with new check for vfr. Was this PR tested? How? Tested with constant frame rate video with 29.97 fps Were docs and examples updated, if necessary? No Signed-off-by: Abhishek Sansanwal <asansanwal@nvidia.com> Signed-off-by: Jianjun Liu <00liujj@163.com>
Description: Previous heuristic would fail for videos with
a fractional fps, such as 29.97(30000/1001). It would classify
such videos as vfr even if they might be cfr.
This change will compare the frame rate reported by the container
to an estimate based off on the first video frame.
While this check is not exhaustive, but in my testing it was accurate.
Signed-off-by: Abhishek Sansanwal asansanwal@nvidia.com
Why we need this PR?
Improves vfr detection heuristic.
What happened in this PR?
Compare the frame rate reported by the container
to an estimate based off on the first video frame
Replaced old check with new check for vfr.
Tested with constant frame rate video with 29.97 fps
No