Modify --min-duration arguments to accept a timecode instead of frame numbers

[tonycpsu]

• Add new -m/--min-duration option for specifying a minimum number
  of seconds for each scene.
• Scenes shorter than this value will be merged with subsequent
  scenes, or if -M/--min-duration-action is set to drop, dropped
  from the scene list entirely.

[Breakthrough]

I like where this PR is going in terms of maintainability, but this essentially depreciates the --min-scene-len option for both detect-content and detect-threshold. I'm open to that for sure to make adding detectors easier in the future, but would like to make sure the existing detectors are modified accordingly to remove the redundant option.

(Or was there a reason you kept -m for the detectors as well that I might be missing? Just want to discuss this briefly before making any decisions, thanks for the PR!)

[tonycpsu]

Er, I wish I had a better answer than "I completely forgot about --min-scene-len, but I don't... 🙈

I do think there's merit in using seconds rather than frames, as FPS varies, and I kind of feel like most people would want to specify a number of seconds. But that would break backwards compatibility for not much gain...

I also think the option to have short scenes prepended/appended to neighbors might be valuable to some, but it wasn't really my primary motivation for this, so maybe this was just a poorly-conceived idea.

Will check out if --min-scene-len works for my purposes.

[Breakthrough]

One other option which might work is modifying --min-scene-len to accept a timecode instead of number of frames. I completely agree that there is merit in using a unit other than frames for that purpose; for transparency, the --min-scene-len option was added before PySceneDetect actually supported timecodes.

Now that the parsing is done for them though, modifying --min-scene-len to accept any of the timecode formats would be trivial (all the parsing work is done if you look at the time command).

I would be happy to accept a PR that modifies that, or to have you submit a feature request for it. Thoughts?

---

The more I think about it, while I like not having to reimplement --min-scene-len for each detector, it would change the output if it was a global option for the whole program versus each detector. I'm honestly just not sure what the most appropriate use-case is for the end user; having it as a global option would be more maintainable, but allow less flexibility. Any feedback you have in that regard would be most welcome.

[tonycpsu]

For now, I've just reverted my implementation and added timecode parsing for the existing option as you suggested.

I do think there's merit to allowing users to drop short scenes (it appears the current implementation just prepends them to the next scene?), and maybe some simplicity to doing it after the scene list is generated instead of in the detection code, but this'll do for now.

[Breakthrough]

Thanks so much for this, @tonycpsu! Sorry I don't have a Travis.CI setup for the project yet, so still need to run all the unit tests on this before merging, but I don't expect any issues (again thank you for submitting very small/targeted code changes, it really helps!)

Will do my best to get this tested & merged by end of the week. Thank you again for your submission, really appreciate it!

[Breakthrough]

Tested manually for now; still requires documentation update. Will try to get that pushed to this PR so it can go in for this weekend.

Sorry for the delay, and thanks again for your submission!

[Breakthrough]

Documentation updated in latest ( https://pyscenedetect-manual.readthedocs.io/en/latest/cli/detectors.html ). Default value for both -m options changed from 15 frames to 0.6 seconds. Feel free to provide any comments if you feel this new default isn't a good choice, or if you notice anything in the new documentation. All unit tests passing, and the existing implementation allowed for drop-in replacement of the frame # with a timecode (very nice!).

Thanks again for the pull request!