Breakthrough / PySceneDetect

:movie_camera: Python and OpenCV-based scene cut/transition detection program & library.
https://www.scenedetect.com/
BSD 3-Clause "New" or "Revised" License
2.97k stars 374 forks source link

Incorrect FrameTimecode conversion to string #354

Closed Breakthrough closed 4 months ago

Breakthrough commented 8 months ago

There's a report of a timestamp being converted to string for video splitting with a seconds value of 60, which ffmpeg promptly rejects. This was raised in another project: Breakthrough/DVR-Scan#145

In PR #269 and the associated issue #268, some math was changed to use flooring/truncation instead of rounding to ensure correct conversions when doing round trips from frame numbers to FrameTimecodes. However, there still seems to be some use of rounding for converting the time into a string (e.g. in HH:MM:SS.nnn format).

Notably, rounding is still used by default for this purpose. This math should probably be revisited so the integral and fractional components can be handled correctly: https://github.com/Breakthrough/PySceneDetect/blob/main/scenedetect/frame_timecode.py#L192

Breakthrough commented 4 months ago

Previously it was possible that in some rare cases, rounding might cause an incorrect result that could explain https://github.com/Breakthrough/DVR-Scan/issues/145. I've re-implemented this correctly now, and have added a new test case to ensure this behavior remains consistent.