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

Three problems with 'frame-skip' parameter #70

Closed
ocram opened this issue Dec 13, 2021 · 9 comments
Closed

Three problems with 'frame-skip' parameter #70

ocram opened this issue Dec 13, 2021 · 9 comments
Labels
Milestone

Comments

@ocram
Copy link

ocram commented Dec 13, 2021

While the docs somewhat discourage from its use, the --frame-skip parameter is a great way to speed up processing significantly – by as much as expected, i.e. in some cases 2× or 3×.

But I think there are three bugs/problems with the implementation of the parameter:

  • The value of --min-event-length is not adjusted if you start using --frame-skip:
    • You need 1/2 of your actual --min-event-length, i.e. reduce the value, for similar results with --frame-skip 1.
    • You need 1/3 of your actual --min-event-length, i.e. reduce the value, for similar results with --frame-skip 2.
  • The CLI text output (list of motion events) adds another --time-post-event to the event duration for --frame-skip 1, and yet another for --frame-skip 2.
  • Exported video clips contain 1/2 of the frames for --frame-skip 1 and 1/3 of the frames for --frame-skip 2, but they still run at the original frame rate (e.g. 30 FPS), thus the exported clip appears sped up. But the total duration of the exported clips is still always the same, i.e. more (non-motion) time after the event is included to compensate.
@Breakthrough
Copy link
Owner

Breakthrough commented Dec 13, 2021

Regarding the exported videos appearing sped up, this is a limitation of the current implementation, and will be alleviated in v2.0 when #61 is fixed, as that will change how events are exported. For the next release I can look into scaling the framerate down in the meantime to help with this.

In v2.0, videos will be split by using ffmpeg instead of just re-encoding each frame as is done now.

For the first two points though indeed those will need to be fixed, thanks for the report. Note that I may need to break this item down into separate issues for tracking purposes.

@ocram
Copy link
Author

ocram commented Dec 13, 2021

That makes sense. Thank you!

@Breakthrough
Copy link
Owner

As discussed, have adjusted framerate so videos have correct playback speed when using -fs, and also adjusted values for -l/-tb/-tp so that they are relative to the original video framerate (and thus are compensated for when using -fs). For example, if we use -fs 2 and set -l to 6, it will only require L / (FS + 1) = 6 / (2 + 1) = 2 frames to trigger an event, instead of 6. The same logic is applied internally for time pre/post event, which combined with the framerate fix, means those values are also now correct when using frame skip.

Will be included in DVR-Scan v1.4, thanks for the report!

@ocram
Copy link
Author

ocram commented Feb 10, 2022

I can confirm that v1.4 fixes at least parts two (--frame-skip n affecting text output by adding n times --time-post-event to the durations) and three (--frame-skip n speeding up exported video clips by a factor of n + 1) from the first comment in this issue above.

With --frame-skip n, the output videos in v1.4 still contain only 1 / (n + 1) of the frames of the original, but the frame rate is now correctly reduced as well so that it doesn’t appear sped up. I understand the ffmpeg solution in v2.0 will change it so that all the frames are contained in the output again and the frame rate can be kept at the original value.

@Breakthrough
Copy link
Owner

One thing I've realized since then is that the ffmpeg solution doesn't work if people want to use bounding boxes or timecodes, so I need to actually rethink how that works. That being said, I was worried about addressing that for now due to the complexity of adding it in, but it is certainly possible - I'll file an issue to track that work so it doesn't get lost. Many thanks again for your feedback on the project.

@Breakthrough
Copy link
Owner

Reopening due to issue identified in #68 where the calculated start times still do not account for -tb / --time-before-event correctly.

@Breakthrough Breakthrough reopened this Feb 12, 2022
Breakthrough added a commit that referenced this issue Feb 12, 2022
Add tests to validate behaviour of frame skip with respect to
pre/post event times. (#68, #70)
@Breakthrough
Copy link
Owner

Added tests in 0dab20b to validate pre-event and post-event shifts when using various values of frame skip.

Note that as a natural consequence of frame skipping, it's still possible for the resulting start/end times to be off by the number of skipped frames (e.g. if frame skip is 2, the start time may differ by up to 2 frames from it's true position when frame skip is 0; same for the end time). This can be fixed for the end times when #82 is resolved, but there is no easy fix for the start time shift.

One solution that comes to mind would be to seek backwards and restart the background subtractor, but this may lead to erroneous results or other false positives, especially if there is not enough frames for the true background to be calculated correctly. In general frame skip should be avoided if frame-accurate results are required, but values of 1-2 shouldn't be that noticeable for most use cases.

Fix will be included in v1.5.

@Breakthrough Breakthrough added this to the v1.4.1 milestone Feb 12, 2022
@ocram
Copy link
Author

ocram commented Feb 12, 2022

Note that as a natural consequence of frame skipping, it's still possible for the resulting start/end times to be off by the number of skipped frames (e.g. if frame skip is 2, the start time may differ by up to 2 frames from it's true position when frame skip is 0; same for the end time). This can be fixed for the end times when #82 is resolved, but there is no easy fix for the start time shift.

I don’t think that’s a problem at all. If you have an input video with 50 FPS, for example, and use --frame-skip 4, which is a relatively large value already, your time codes could be off by 0.08 seconds at a maximum. That doesn’t matter in most use cases. In my example, with the current bug, times were off by three full seconds, or whatever --time-before-event was.

In general frame skip should be avoided if frame-accurate results are required, but values of 1-2 shouldn't be that noticeable for most use cases.

Agreed. I think it should be obvious to users that you shouldn’t use --frame-skip if you want frame-accurate results.

As an alternative, you can quickly scan source videos with --frame-skip enabled, and if motion is found, you can re-scan with --frame-skip omitted for frame-accurate results, depending on how likely motion is to be found.

@Breakthrough
Copy link
Owner

Breakthrough commented Feb 12, 2022

For clarity and a bit of a workaround, the issue arose from calculating the new start times. In v1.4, we calculate the buffer size as (roughly):

# Length to shift start time once enough frames have motion
shift_amount = (min_scene_len + time_before_event) / (frame_skip + 1)

# Shifted event time - incorrect, should be shift_amount * (frame_skip + 1)
event_start = curr_pos + shift_amount

This value was directly used to shift the starting time codes once min_scene_len frames were found to have motion. Instead, it should have been multiplied again by frame_skip + 1 to reverse the transformation. As a workaround for v1.4 you can get fairly close results by multiplying time_before_event by frame_skip + 1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants